[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160129002703.GA4820@swordfish>
Date: Fri, 29 Jan 2016 09:27:03 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: Peter Hurley <peter@...leysoftware.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Byungchul Park <byungchul.park@....com>,
akpm@...ux-foundation.org, mingo@...nel.org,
linux-kernel@...r.kernel.org, akinobu.mita@...il.com, jack@...e.cz,
torvalds@...ux-foundation.org,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [PATCH v4] lib/spinlock_debug.c: prevent a recursive cycle in
the debug code
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
> >
>
Powered by blists - more mailing lists