Re: enable_ioapic_irq broken in arch/i386/kernel/irq.c

Claus-Justus Heine (
25 Apr 1998 21:40:20 +0200

Claus-Justus Heine <> writes:

> MOLNAR Ingo <> writes:
> > On 20 Apr 1998, Claus-Justus Heine wrote:
> >
> > > I think I have found a bug in enable_ioapic_irq() in arch/i3886/kernel/irq.c
> >
> > nicely spotted, the current enable_ioapic_irq() code is badly broken. I've
> I didn't want to hurt anybody. It is generally a stupid idea to run
> interrupt service routines in a non-interrupt level the way that
> enable_ioapic_irq() did it.
> BTW, the enable_ioapic_irq only "enables" the interrupt if
> "disable_irq[irq]" is zero. The UP version of enable_irq()
> unconditionally enables the interrupt. I think these two should match
> -- i.e. behave as much the same as is possible. So, maybe the
> disable_irq() routines shouldn't increment that disable_irq[irq]
> counter, but simply set a flag?

Nice to see that the above was implemented in 2.1.98

I have one further remark, talking about 2.1.98:

If more than a single (i.e. 1) interrupt has accumulated during the
period that disable_ioapic_irq() was active, then do_ioapic_IRQ() will
NEVER AGAIN execute that interrupt.

This is because do_ioapic_IRQ() checks whether irq_events[irq] is zero:

if (!irq_events[irq]++ && !disabled_irq[irq])
should_handle_irq = 1;

otherwise it will not handle the irq but simply increment the variable
irq_events[irq]. Now, if, say, two interrupts have accumulated while
an IRQ was disabled, then irq_events[irq] will have the value
"2". Occasionally enable_ioapic_irq() is called. It then resends the
IRQ and decrements irq_events[irq]:

if (irq_events[irq]) {
if (!ipi_pending[irq]) {
ipi_pending[irq] = 1;

That means irq_events[irq] is still different from zero when
do_ioapic_IRQ() is called next time. do_ioapic_IRQ() then still will
not handle the IRQ, and even worse, it NEVER will handle the IRQ
except in the unlikely case that someone calls enable_ioapic_irq()
until irq_events[irq] finally is zero (and if that someone calls the
enable_ioapic_irq()s fast enough so that the interrupt count decreases
faster than the other CPU increases it by calling do_ioapic_IRQ() :-)

My suggestion is to unconditionally handle the IRQ if the
ipi_pending[irq] flag is set which means that somebody tried to enable
the IRQ again by calling enable_ioapic_IRQ(). Patch follows:

--- linux-2.1/arch/i386/kernel/irq.c~ Sat Apr 25 13:52:51 1998
+++ linux-2.1/arch/i386/kernel/irq.c Sat Apr 25 21:19:14 1998
@@ -779,11 +779,15 @@

- if (ipi_pending[irq])
- ipi_pending[irq] = 0;

if (!irq_events[irq]++ && !disabled_irq[irq])
should_handle_irq = 1;
+ if (ipi_pending[irq]) {
+ ipi_pending[irq] = 0;
+ should_handle_irq = !disabled_irq[irq];
+ }

Maybe not very elegant, but should work.



  Claus-Justus Heine      

PGP Public Key:

Ftape - the Linux Floppy Tape Project WWW : Mailing-list:

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to