On Mon, May 14, 2007 at 04:05:15PM +0200, Bernhard Walle wrote: > * Vivek Goyal <[EMAIL PROTECTED]> [2007-05-08 19:18]: > > On Thu, May 03, 2007 at 12:19:32AM +0200, Bernhard Walle wrote: > > > * Vivek Goyal <[EMAIL PROTECTED]> [2007-04-30 10:48]: > > > > > > > > handle_edge_irq() already makes sure that desc->action is not null, > > > > still > > > > note_interrupt() is receiving desc->action as null, that's strange. On > > > > my > > > > system this is happening for irq 4 and /proc/interrupt shows that it is > > > > coming from "serial". > > > > > > Unfortunately, I couldn't reproduce this here. Vivek, do you have time > > > to take a look at this at your site? For the meanwhile, should I > > > create a patch that checks for desc->action in note_interrupt(), too? > > > > I can reproduce this problem only on one machine. I think there is some > > race condition and your code somehow just exposes it. > > thanks for finding that out. Could you try/review out the patch below? > As the lock is only aquired when irqfixup == 2 it shouldn't impact > performance of a 'normal' system. >
Hi Bernhard, It does fix up my problem. I have modified your patch a bit. I think new version is little more clear. What do you think? Thanks Vivek o System crashes if booted with irqpoll command line option. o Problem happens because Inside note_interrupt() we are accessing desc->action->flag without taking the desc->lock. While accessing it somebody goes ahead and unregisters the irq handler hence desc->action is NULL. By the time note_interrupt() checks it, it crashes. o In my system it is irq 4 seriving to serial driver. o Take the desc->lock before accessing desc->action->flag. Signed-off-by: Bernhard Walle <[EMAIL PROTECTED]> Signed-off-by: Vivek Goyal <[EMAIL PROTECTED]> --- linux-2.6.21-git12-root/kernel/irq/spurious.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff -puN kernel/irq/spurious.c~fix-irqpoll-crash kernel/irq/spurious.c --- linux-2.6.21-git12/kernel/irq/spurious.c~fix-irqpoll-crash 2007-05-17 17:36:50.000000000 +0530 +++ linux-2.6.21-git12-root/kernel/irq/spurious.c 2007-05-17 17:53:52.000000000 +0530 @@ -138,6 +138,8 @@ report_bad_irq(unsigned int irq, struct void note_interrupt(unsigned int irq, struct irq_desc *desc, irqreturn_t action_ret) { + int call_misrouted_irq = 0; + if (unlikely(action_ret != IRQ_HANDLED)) { desc->irqs_unhandled++; if (unlikely(action_ret != IRQ_NONE)) @@ -146,9 +148,24 @@ void note_interrupt(unsigned int irq, st if (unlikely(irqfixup)) { /* Don't punish working computers */ - if ((irqfixup == 2 && ((irq == 0) || - (desc->action->flags & IRQF_IRQPOLL))) || - action_ret == IRQ_NONE) { + if (action_ret == IRQ_NONE) + /* Nobody handled irq. Possibly a misrouted one. */ + call_misrouted_irq = 1; + else if (irqfixup == 2) { + /* irqpoll is enabled. Is this the irq driving + * polling. + */ + if (irq == 0) + call_misrouted_irq = 1; + else { + spin_lock(&desc->lock); + if (desc->action && + (desc->action->flags & IRQF_IRQPOLL)) + call_misrouted_irq = 1; + spin_unlock(&desc->lock); + } + } + if (call_misrouted_irq) { int ok = misrouted_irq(irq); if (action_ret == IRQ_NONE) desc->irqs_unhandled -= ok; _ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/