Re: [PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM

[Takashi Iwai <tiwai@suse.de>]

At Tue, 24 Aug 2004 20:45:08 -0700,
Andrew Morton wrote:
> 
> Takashi Iwai <tiwai@suse.de> wrote:
> >
> > 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.
> 
> That's because SA_INTERRUPT interrupts shouldn't be shared.  The grey cell
> which remembered why this is so seems to have died, but I've put the email
> thread here: http://www.zip.com.au/~akpm/linux/patches/stuff/x.txt

Oh thanks that helps to understand what happened.
(BTW, regarding the atomicity in that discussion: could the code like
 rtc.c (assuming it has also SA_SHIRQ) really cause deadlock?)

Anyway, suppressing the unnecessary call of add_interrupt_randomness()
should be still valid.  The reduced patch is below.


Takashi

--- linux-2.6.8.1/arch/i386/kernel/irq.c-dist	2004-08-25 13:13:05.153227112 +0200
+++ linux-2.6.8.1/arch/i386/kernel/irq.c	2004-08-25 13:13:34.760726088 +0200
@@ -220,14 +220,16 @@ asmlinkage int handle_IRQ_event(unsigned
 		struct pt_regs *regs, struct irqaction *action)
 {
 	int status = 1;	/* Force the "do bottom halves" bit */
-	int retval = 0;
+	int ret, retval = 0;
 
 	if (!(action->flags & SA_INTERRUPT))
 		local_irq_enable();
 
 	do {
-		status |= action->flags;
-		retval |= action->handler(irq, action->dev_id, regs);
+		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)