Messages in this thread Patch in this message | | | Date | Mon, 23 Aug 2004 19:10:11 +0200 | From | Takashi Iwai <> | Subject | [PATCH] Fix shared interrupt handling of SA_INTERRUPT and SA_SAMPLE_RANDOM |
| |
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; } - 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/
| |