[PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM

[Takashi Iwai <tiwai@suse.de>]

Hi,

while the recent investation of latency issues, Thomas Charbonne
suggested that there is a long-standing bug in the irq handler.
When the irq is shared, SA_INTERRUPT flag is checked only for the
first registered handler.  When it's without SA_INTERRUPT, always
local_irq_enable() is called even if the second or later handler has
SA_INTERRUPT.

Also, handle_IRQ_event() always calls add_interrupt_randomness()
even if the irq is not for the handler with SA_SAMPLE_RANDOM.
This is a performance loss.

The patch below fixes these problems by adding the SA_INTERRUPT
handler always to the head of the irq list, and by checking the return
value of each handler.

The patch is for i386 and x86-64 only.  Similar patches will be needed
for other architectures, too (or more better, making an
arch-independent generic handler as in voluntary-preemptive patch).


--
Takashi Iwai <tiwai@suse.de>		ALSA Developer - www.alsa-project.org


--- linux/arch/i386/kernel/irq.c	2004-08-18 15:15:18.000000000 +0200
+++ linux/arch/i386/kernel/irq.c	2004-08-20 14:54:14.000000000 +0200
@@ -221,13 +221,21 @@ asmlinkage int handle_IRQ_event(unsigned
 {
 	int status = 1;	/* Force the "do bottom halves" bit */
 	int retval = 0;
-
-	if (!(action->flags & SA_INTERRUPT))
-		local_irq_enable();
+	int irq_off = 1;
 
 	do {
-		status |= action->flags;
-		retval |= action->handler(irq, action->dev_id, regs);
+		int ret;
+		/* Assume that all SA_INTERRUPT handlers are at the head
+		 * of the irq queue
+		 */
+		if (irq_off && ! (action->flags & SA_INTERRUPT)) {
+			local_irq_enable();
+			irq_off = 0;
+		}
+		ret = action->handler(irq, action->dev_id, regs);
+		if (ret)
+			status |= action->flags;
+		retval |= ret;
 		action = action->next;
 	} while (action);
 	if (status & SA_SAMPLE_RANDOM)
@@ -955,11 +963,16 @@ int setup_irq(unsigned int irq, struct i
 			return -EBUSY;
 		}
 
-		/* add new interrupt at end of irq queue */
-		do {
-			p = &old->next;
-			old = *p;
-		} while (old);
+		if (new->flags & SA_INTERRUPT)
+			/* add interrupt at the start of the queue */
+			new->next = old;
+		else
+			/* add new interrupt at end of irq queue */
+			do {
+				p = &old->next;
+				old = *p;
+			} while (old);
+
 		shared = 1;
 	}
 
--- linux/arch/x86_64/kernel/irq.c	2004-08-20 15:04:26.000000000 +0200
+++ linux/arch/x86_64/kernel/irq.c	2004-08-20 15:07:06.000000000 +0200
@@ -213,13 +213,20 @@ inline void synchronize_irq(unsigned int
 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 irq_off = 1;
 
 	do {
-		status |= action->flags;
-		action->handler(irq, action->dev_id, regs);
+		int ret;
+		/* Assume that all SA_INTERRUPT handlers are at the head
+		 * of the irq queue
+		 */
+		if (irq_off && ! (action->flags & SA_INTERRUPT)) {
+			local_irq_enable();
+			irq_off = 0;
+		}
+		ret = action->handler(irq, action->dev_id, regs);
+		if (ret)
+			status |= action->flags;
 		action = action->next;
 	} while (action);
 	if (status & SA_SAMPLE_RANDOM)
@@ -794,11 +801,16 @@ int setup_irq(unsigned int irq, struct i
 			return -EBUSY;
 		}
 
-		/* add new interrupt at end of irq queue */
-		do {
-			p = &old->next;
-			old = *p;
-		} while (old);
+		if (new->flags & SA_INTERRUPT)
+			/* add interrupt at the start of the queue */
+			new->next = old;
+		else
+			/* add new interrupt at end of irq queue */
+			do {
+				p = &old->next;
+				old = *p;
+			} while (old);
+
 		shared = 1;
 	}