Messages in this thread Patch in this message | | | Date | Mon, 24 Feb 2003 21:38:09 +1100 | From | James Harper <> | Subject | PATCH to fix irq sharing and SA_INTERRUPT on x86. please review |
| |
further to my email yesterday (to which i've had no response :) i propose the attached patch to arch/i386/kernel/irq.c. it corrects what i see as a bug in interrupt handling.
currently if a driver requests SA_INTERRUPT in an interrupt handler, it is only called with interrupts disabled if it is the first handler in the list.
my patch modifies setup_irq to put any interrupt with SA_INTERRUPT in the front of the handler queue (eg before any handlers without the flag).
and also modifies handle_IRQ_event to only enable interrupts when it hits the first handler with SA_INTERRUPT not set.
the patch is against 2.5.62 and greatly improves stability on my SMP system.
james
--- linux/arch/i386/kernel/irq.c Wed Feb 12 21:36:34 2003 +++ linux.test/arch/i386/kernel/irq.c Mon Feb 24 21:01:04 2003 @@ -201,11 +201,13 @@ int handle_IRQ_event(unsigned int irq, struct pt_regs * regs, struct irqaction * action) { int status = 1; /* Force the "do bottom halves" bit */ - - if (!(action->flags & SA_INTERRUPT)) - local_irq_enable(); + int enabled = SA_INTERRUPT; do { + if (~action->flags & enabled) { + local_irq_enable(); + enabled = 0; + } status |= action->flags; action->handler(irq, action->dev_id, regs); action = action->next; @@ -761,23 +763,38 @@ * The following block of code has to be executed atomically */ spin_lock_irqsave(&desc->lock,flags); - p = &desc->action; - if ((old = *p) != NULL) { - /* Can't share interrupts unless both agree to */ - if (!(old->flags & new->flags & SA_SHIRQ)) { - spin_unlock_irqrestore(&desc->lock,flags); - return -EBUSY; + if (new->flags & SA_INTERRUPT) { + p = &desc->action; + old = *p; + /* Add SA_INTERRUPT interrupts to the front of the queue + * 'cos it's faster than adding them in the middle */ + if (old) { + if (!(old->flags & new->flags & SA_SHIRQ)) { + spin_unlock_irqrestore(&desc->lock,flags); + return -EBUSY; + } + new->next = old; + shared = 1; } + *p = new; + } else { + p = &desc->action; + if ((old = *p) != NULL) { + /* Can't share interrupts unless both agree to */ + if (!(old->flags & new->flags & SA_SHIRQ)) { + spin_unlock_irqrestore(&desc->lock,flags); + return -EBUSY; + } - /* add new interrupt at end of irq queue */ - do { - p = &old->next; - old = *p; - } while (old); - shared = 1; + /* add new interrupt at end of irq queue */ + do { + p = &old->next; + old = *p; + } while (old); + shared = 1; + } + *p = new; } - - *p = new; if (!shared) { desc->depth = 0; | |