Messages in this thread Patch in this message | | | Date | Wed, 27 Jan 2016 10:14:54 +0900 | From | Sergey Senozhatsky <> | Subject | Re: + lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump.patch added to -mm tree |
| |
On (01/26/16 16:12), akpm@linux-foundation.org wrote: [..] > There is an infinite recursive cycle when using CONFIG_DEBUG_SPINLOCK, in > spin_dump(). Backtrace prints printk() -> console_trylock() -> > do_raw_spin_lock() -> spin_bug() -> spin_dump() -> printk()... > infinitely.
is it even possible to lockup on a semaphore's spin_lock?
int down_trylock(struct semaphore *sem) { unsigned long flags; int count;
raw_spin_lock_irqsave(&sem->lock, flags); ^^^^ here? count = sem->count - 1; if (likely(count >= 0)) sem->count = count; raw_spin_unlock_irqrestore(&sem->lock, flags);
return (count < 0); }
under what circumstances and why it should be silenced? a memory corruption? or is it the 'logbuf_lock' spin_lock that was meant to be in the report? what if we lockup on `logbuf_lock`, it will generate the same call-chain...
> If the spin_bug() is called from a function like printk() which is trying > to obtain the console lock, we should prevent the debug spinlock code from > calling printk() again in that context.
even if it was the 'logbuf_lock' spin_lock then still, we take it for quite short periods of time with IRQs disabled:
in vprintk_emit(), when sprintf text and store it
local_irq_save() raw_spin_lock() vscnprintf() log_store() raw_spin_unlock() local_irq_restore()
and in console_unlock() when we read it back
for (;;) { raw_spin_lock_irqsave(&logbuf_lock, flags); msg_print_text raw_spin_unlock(&logbuf_lock) call_console_drivers() local_irq_restore }
so if the CPU that owns the spin_lock somehow managed to keep it forever (due to a memory corruption... or something has powered off the cpu core???) -- then _this is_ the problem, not the fact that other CPUs will not lock the spin_lock anymore.
so I don't think this patch does the right thing, sorry.
-ss
> Signed-off-by: Byungchul Park <byungchul.park@lge.com> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Jan Kara <jack@suse.cz> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > kernel/locking/spinlock_debug.c | 11 +++++++++++ > kernel/printk/printk.c | 5 +++++ > 2 files changed, 16 insertions(+) > > diff -puN kernel/locking/spinlock_debug.c~lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump kernel/locking/spinlock_debug.c > --- a/kernel/locking/spinlock_debug.c~lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump > +++ a/kernel/locking/spinlock_debug.c > @@ -67,11 +67,22 @@ static void spin_dump(raw_spinlock_t *lo > dump_stack(); > } > > +extern int is_console_lock(raw_spinlock_t *lock); > + > static void spin_bug(raw_spinlock_t *lock, const char *msg) > { > if (!debug_locks_off()) > return; > > + /* > + * If this function is called from a function like printk() > + * which is trying to obtain the console lock, then we should > + * not call printk() any more. Or it will cause an infinite > + * recursive cycle! > + */ > + if (unlikely(is_console_lock(lock))) > + return; > + > spin_dump(lock, msg); > } > > diff -puN kernel/printk/printk.c~lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump kernel/printk/printk.c > --- a/kernel/printk/printk.c~lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump > +++ a/kernel/printk/printk.c > @@ -120,6 +120,11 @@ static int __down_trylock_console_sem(un > up(&console_sem);\ > } while (0) > > +int is_console_lock(raw_spinlock_t *lock) > +{ > + return &console_sem.lock == lock; > +} > + > /* > * This is used for debugging the mess that is the VT code by > * keeping track if we have the console semaphore held. It's > _ > > Patches currently in -mm which might be from byungchul.park@lge.com are > > lib-spinlock_debugc-prevent-an-infinite-recursive-cycle-in-spin_dump.patch > > -- > To unsubscribe from this list: send the line "unsubscribe mm-commits" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
| |