Messages in this thread | | | Date | Thu, 21 Aug 2003 17:33:59 +0200 | From | Andrea Arcangeli <> | Subject | Re: Dumb question: BKL on reboot ? |
| |
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |