Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1320411
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code |
| Date | 2016-01-28 09:30 +0100 |
| Message-ID | <qVQ7P-FX-67@gated-at.bofh.it> (permalink) |
| References | <qVx58-3fC-7@gated-at.bofh.it> <qVK2m-4kg-7@gated-at.bofh.it> <qVKF3-4Q6-1@gated-at.bofh.it> <qVMQz-6q1-21@gated-at.bofh.it> <qVNWh-7D7-9@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, Jan 28, 2016 at 03:05:30PM +0900, Sergey Senozhatsky wrote:
> On (01/28/16 13:36), byungchul.park wrote:
> [..]
> > > the thing is, it's really-really hard to lockup in console_trylock()...
> > >
> > > int down_trylock(struct semaphore *sem)
> > > {
> > > unsigned long flags;
> > > int count;
> > >
> > > raw_spin_lock_irqsave(&sem->lock, flags); <<<<<< um...
> >
> > I also think it's hard, but a backtrace said the lockup happened here.
>
> what was the state of `struct semaphore *sem' and especially of `sem->lock'?
> what was the lock->owner_cpu doing? (addr2line of its pc registe, for example).
Unfortunately, it's not reproduced anymore.
If it's clearly a spinlock caller's bug as you said, modifying the
spinlock debug code does not help it at all. But I found there's a
possiblity in the debug code *itself* to cause a lockup. So I tried
to fix it. What do you think about it?
>
> > > count = sem->count - 1;
> > > if (likely(count >= 0))
> > > sem->count = count;
> > > raw_spin_unlock_irqrestore(&sem->lock, flags);
> > >
> > > return (count < 0);
> > > }
> > >
> > > was not able to dereference sem->count? `*sem' was corrupted? or because
> > > sem->lock was corrupted? in any of those cases you solve the problem from
> > > the wrong side. if you have a memory corruption then it can corrupt
> >
> > What I solved here is to make it possible to print what the problem is, by
> > the spinlock debug code instead of system lockup while printing a debug
> > message. I think it's not wrong.
>
> none of the CPUs will print anything anymore. it's done.
You are right if it's a real lockup situation. But it will print proper
debug messages if it's just a suspect case, which works with this patch.
I will also try to reproduce it and check if there's a bug on use of
spinlock. In this case, we should fix the root cause. But since the
possiblity I mentioned can *also* cause the lockup problem, I think it
must be fixed at first.
>
>
> your CPUa - CPUx are spinning in down_trylock()
>
> vprintk_emit()
> down_trylock()
> raw_spin_lock_irqsave() << spin here
>
> and they are spinnig because CPUz is keeping the sem->lock and is
> _not_ going to release it. and this is the root cause, not spin_dump().
If it's not going to release it then it's a problem. But IMHO,
arch_spin_trylock() in __spin_lock_debug() can fail even though the owner
of the spinlock releases it properly, if there's heavy use on printk() at
the moment. Is there something I missed here? If what I mention can happen,
then it's not a spinlock user's problem. It's just a debug code's problem.
>
>
> CPUz was expected to do a quick thing in down_trylock()
>
> raw_spin_lock_irqsave(&sem->lock, flags);
> count = sem->count - 1;
> if (likely(count >= 0))
> sem->count = count;
> raw_spin_unlock_irqrestore(&sem->lock, flags);
This may be done quickly, but if the use of printk() on the system is
heavy?
>
>
> or down()/down_common()
>
>
> raw_spin_lock_irqsave(&sem->lock, flags);
> if (likely(sem->count > 0))
> sem->count--;
> else
> down_common()
> {
> ...
> for (;;) {
> if (signal_pending_state(state, task))
> goto interrupted;
> if (unlikely(timeout <= 0))
> goto timed_out;
> __set_task_state(task, state);
> raw_spin_unlock_irq(&sem->lock);
> timeout = schedule_timeout(timeout);
> raw_spin_lock_irq(&sem->lock);
> if (waiter.up)
> return 0;
> }
> ...
> }
> raw_spin_unlock_irqrestore(&sem->lock, flags);
>
>
> I can't see how it's even possible to keep that spin_lock locked
> longer than `loops_per_jiffy * HZ'.
No need to keep that spinlock longer than it to cause the problem..
>
> and the fact that you saw N recursive
> printk()->down_trylock()->spin_lock()->spin_dump()->printk()->...
>
> sounds like a good prove of the fact the your CPUz was either dead,
> or gone crazy, and took the spin_lock with it. but this is not
My system esp. qemu might have been crazy because printk() business.
> spinlock_debug's business.
I think it could be exactly a spinlock debug's business.
>
> console_flush_on_panic() _probably_ would help in this particular
> case -- it does not care about console_sem state and goes to
> console_unlock() directly -- but it still locks the logbuf_lock.
> so if CPUz died with logbuf_lock being locked, then nothing will
> save your day.
>
>
> do you have any reproducer or you've seen it once?
Just once.
thanks,
byungchul
>
> -ss
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-27 13:10 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Peter Hurley <peter@hurleysoftware.com> - 2016-01-27 23:50 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-28 08:20 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 09:20 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-28 03:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-28 03:40 +0100
RE: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code "byungchul.park" <byungchul.park@lge.com> - 2016-01-28 06:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-28 07:10 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-28 09:30 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-28 11:50 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-28 12:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-01-28 16:50 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Peter Hurley <peter@hurleysoftware.com> - 2016-01-29 00:10 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 01:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 02:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 04:10 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 05:10 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 13:20 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 01:30 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Peter Hurley <peter@hurleysoftware.com> - 2016-01-29 05:40 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 06:30 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Peter Hurley <peter@hurleysoftware.com> - 2016-01-29 06:50 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 07:20 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 07:40 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-31 13:30 +0100
[PATCH 2/3] printk: introduce reset_console_drivers() Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-31 13:40 +0100
Re: [PATCH 2/3] printk: introduce reset_console_drivers() kbuild test robot <lkp@intel.com> - 2016-01-31 13:50 +0100
[PATCH 1/3] printk: introduce console_reset_on_panic() function Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-31 13:40 +0100
[PATCH 3/3] spinlock_debug: panic on recursive lock spin_dump() Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-31 13:40 +0100
Re: [PATCH 3/3] spinlock_debug: panic on recursive lock spin_dump() Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2016-02-01 17:20 +0100
Re: [PATCH 3/3] spinlock_debug: panic on recursive lock spin_dump() Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-02 09:00 +0100
Re: [PATCH 1/3] printk: introduce console_reset_on_panic() function kbuild test robot <lkp@intel.com> - 2016-01-31 13:50 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 08:00 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-01-29 08:20 +0100
Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code Byungchul Park <byungchul.park@lge.com> - 2016-01-29 09:20 +0100
csiph-web