PATCH to fix irq sharing and SA_INTERRUPT on x86. please review

From: James Harper (james.harper@bigpond.com)
Date: Mon Feb 24 2003 - 05:38:09 EST


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;

-
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/



This archive was generated by hypermail 2b29 : Fri Feb 28 2003 - 22:00:18 EST