Re: Dumb question: BKL on reboot ?

[Andrea Arcangeli <andrea@suse.de>]

On Wed, Aug 20, 2003 at 01:23:44PM -0700, Dave Hansen wrote:
> On Wed, 2003-08-20 at 11:35, David S. Miller wrote:
> > On Wed, 20 Aug 2003 11:29:18 -0700
> > Andrew Morton <akpm@osdl.org> wrote:
> > 
> > > Where exactly does the rebooting CPU get stuck in machine_restart()?  If
> > > someone has done lock_kernel() with local interrupts disabled then yes,
> > > it'll deadlock.  But that's unlikely?  Confused.
> 
> I thought it was legal to do that.  The normal interrupts-off spinlock
> deadlock happens because a thread does a spin_lock() with no irq
> disable, then gets interrupted.  The interrupt handler tries to take the
> lock, and gets stuck.  

that's the regular spinlock case but it has nothing to do with the big
kernel lock during smp_call_function. holding the big kernel lock while
broadcasting the IPI with smp_call_function is perfectly legal. Infact
if something it's safer (and it can't explain any deadlock), and since
it's a so low performance path, where scalability doesn't matter,
leaving the BKL around that code can be acceptable.

The only illegal thing is to call smp_call_function with _local_irqs_
disabled (as Andrew noted above). the big kernel lock doesn't matter for
the smp_call_function callers.

> If the BKL is put in that situation, the interrupt handler will see
> current->lock_depth > 0, and not actually take the spinlock; it will
> just increment the lock_depth.  It won't deadlock.
> 
> Or, are you saying that a CPU could have the BKL, and have
> stop_this_cpu() called on it?  I guess we could add
> release_kernel_lock() to stop_this_cpu().

the *real* bug IMHO is that machine_restart isn't shutting down the
other cpus properly in smp, it has nothing to do with lock_kernel in
kernel/, the bug is in arch/. Dropping the big kernel lock in kernel/
can hide the bug, but it will showup again later in any other spinlock.

The problem is that machine_restart in arch/s390 (and arch/i386 too) can
easily hang a cpu in the IPI handler while it had zillon of spinlocks
held, that may later interfere with the main "reboot" cpu.

So at the very least a better fix than the one that drops the big kernel
lock from kernel/ is to execute current->lock_depth = -1 before entering
the infinite loop in the arch/ code.

A real fix would be to use the unplug cpu framework to allow all cpus to
reach the quiescent point, in this case it's easily doable even w/o
using RCU and w/o the pluggable cpu framework in 2.4, by simply spawning
a kernel thread bind to interesting cpu, and having this kernel thread
setting the bitflag after it become runnable (cut and pasting the
spawn_ksoftirqd will do the trick). The kernel thread will only execute
the infinite loop then. Hanging a cpu forever in a IPI is simply
deadlock prone for all other cpus, and that's the real bug.

Andrea