Messages in this thread Patch in this message | | | Date | Fri, 3 Dec 2004 00:29:25 -0700 (MST) | From | Zwane Mwaikambo <> | Subject | Re: [PATCH][BUG] Badness in smp_call_function at arch/i386/kernel/smp.c:552 |
| |
On Thu, 2 Dec 2004, Andrew Morton wrote:
> Well, sort-of. > > If __handle_sysrq was really a normal IRQ handler then the correct thing to > do here is to replace spin_lock_irqsave() with spin_lock(). But > __handle_sysrq() can also be called via /proc/sysrq-trigger and via the > handlers of multiple interrupt sources. So we're stuck with using > spin_lock_irqsave(). > > However enabling interrupts as you've done menas that theoretically we > could deadlock on sysrq_key_table_lock if another sysrq happens at the > wrong time. > > Which deadlock opportunity would you prefer? ;)
Agreed, there is actually a higher chance of the smp_call_function deadlock occuring since the __handle_sysrq one relies on another sysrq event occuring via a different IRQ line interrupt handler, so we would have to do sysrq via serial and then sysrq via keyboard to cause the deadlock. Perhaps just make it a spin_trylock?
Index: linux-2.6.10-rc2-mm4/drivers/char/sysrq.c =================================================================== RCS file: /home/cvsroot/linux-2.6.10-rc2-mm4/drivers/char/sysrq.c,v retrieving revision 1.1.1.1 diff -u -p -B -r1.1.1.1 sysrq.c --- linux-2.6.10-rc2-mm4/drivers/char/sysrq.c 30 Nov 2004 18:52:31 -0000 1.1.1.1 +++ linux-2.6.10-rc2-mm4/drivers/char/sysrq.c 3 Dec 2004 07:21:04 -0000 @@ -370,7 +370,10 @@ void __handle_sysrq(int key, struct pt_r int i, j; unsigned long flags; - spin_lock_irqsave(&sysrq_key_table_lock, flags); + local_irq_save(flags); + if (!spin_trylock(&sysrq_key_table_lock)) + goto bail; + orig_log_level = console_loglevel; console_loglevel = 7; printk(KERN_INFO "SysRq : "); @@ -396,7 +399,9 @@ void __handle_sysrq(int key, struct pt_r printk ("\n"); console_loglevel = orig_log_level; } - spin_unlock_irqrestore(&sysrq_key_table_lock, flags); + spin_unlock(&sysrq_key_table_lock); +bail: + local_irq_restore(flags); } /* Index: linux-2.6.10-rc2-mm4/arch/i386/kernel/reboot.c =================================================================== RCS file: /home/cvsroot/linux-2.6.10-rc2-mm4/arch/i386/kernel/reboot.c,v retrieving revision 1.1.1.1 diff -u -p -B -r1.1.1.1 reboot.c --- linux-2.6.10-rc2-mm4/arch/i386/kernel/reboot.c 30 Nov 2004 18:52:19 -0000 1.1.1.1 +++ linux-2.6.10-rc2-mm4/arch/i386/kernel/reboot.c 3 Dec 2004 07:23:44 -0000 @@ -274,6 +274,8 @@ void machine_shutdown(void) #ifdef CONFIG_SMP int reboot_cpu_id; + local_irq_enable(); + /* The boot cpu is always logical cpu 0 */ reboot_cpu_id = 0; - 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/
| |