Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1321169

Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code

From Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in the debug code
Date 2016-01-29 01:30 +0100
Message-ID <qW56P-2Vm-3@gated-at.bofh.it> (permalink)
References (5 earlier) <qVQ7P-FX-67@gated-at.bofh.it> <qVSjg-28z-25@gated-at.bofh.it> <qVSsW-2cx-19@gated-at.bofh.it> <qVWZA-5wF-17@gated-at.bofh.it> <qW3Ro-28Y-31@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On (01/28/16 15:08), Peter Hurley wrote:
[..]
> > even if at some level of recursion (nested printk calls)
> > spin_dump()->__spin_lock_debug()->arch_spin_trylock() acquires the
> > lock, it returns back with the spin lock unlocked anyway.
> > 
> > vprintk_emit()
> >  console_trylock()
> >   spin_lock()
> >    spin_dump()
> >     vprintk_emit()
> >      console_trylock()
> >       spin_lock()
> >        spin_dump()
> >         vprintk_emit()
> >          console_trylock()
> >           spin_lock()     << OK, got the lock finally
> 
> The problem is you have postulated a very shallow recursion.
> This looks much worse if this happens 1000 times, and
> probably won't recover to output anything.

well, the stack is surely limited, but on every
spin_dump()->spin_lock() recursive call it does another
round of

	u64 loops = loops_per_jiffy * HZ;

	for (i = 0; i < loops; i++) {
		if (arch_spin_trylock(&lock->raw_lock))
			return;
		__delay(1);
	}

so if you have 1000 spin_dump()->spin_lock() then, well,
something has been holding the lock for '1000 * loops_per_jiffy * HZ'.

and in particularly this case that somethign was holding the
spin lock doing trivial operations like

	count = sem->count - 1;
	if (likely(count >= 0))
		sem->count = count;

(or a bit more if it was in down()). but still.

and it's kinda hard to imagine console_sem lock being soooooooo
congested and unfair. on each given point of time in the worst
case there are `num_online_cpus() - 1' cpus spinning on that spin_lock
and 1 cpu holding that spinlock. which in Byungchul's case is, what,
3 spinning cpus, or 7 spinnign cpus?...


> Additionally, what if the console_sem is simply corrupted?
> A livelock with no output ever is not very helpful.

if it's corrupted then this is not a spinlock debug problem.
at all.


> As I wrote earlier, I don't think this is the way to fix
> recursion problems with printk() [by eliding output].
> 
> Rather, a way to effectively determine a recursion is in progress,
> and _at a minimum_ guaranteeing that the recursive output will
> eventually be output should be the goal.
> 
> Including dumb recursion like a console driver printing
> an error :/

this is not a case of printk recursion and it should be handled
just fine. console drivers are called under console_sem only.
logbuf lock is unlocked. vprintk_emit() adds message to the logbuf,
calls console_trylock() (which of course does not lock anything)
and returns back to console_driver code.

the only case when we really have a printk recursion is when
someone calls printk() from within the vprintk_emit() logbuf_lock
area.

print()
 spin_lock logbuf
   printk()
     spin_lock logbuf <<< recursion
 spin_unlock logbuf


	-ss

> Then, lockdep could remain enabled while calling console drivers.
> 
> Regards,
> Peter Hurley
> 
> >            sem->count--
> >           spin_unlock()   << unlock, return
> >        arch_spin_lock()   << got the lock, return
> >       sem->count--
> >       spin_unlock() << unlock, return
> >    arch_spin_lock() << got the lock, return
> >   sem->count--
> >   spin_unlock() << unlock, return
> > 
> > 
> > ...um
> > 
> > 
> >> But I found there's a possiblity in the debug code *itself* to cause a
> >> lockup.
> > 
> > please explain.
> > 
> > 	-ss
> > 
> 

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

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