[patch] IRQ_INPROGRESS bugfix

Andrea Arcangeli (andrea@e-mind.com)
Tue, 22 Dec 1998 22:37:47 +0100 (CET)


The i386 port had a bug in the IRQ_INPROGRESS flag handling. It was
setting the flag even if there was no action and so even if the irq
handler would be no executed and so even if the IRQ_INPROGRESS flag will
remain set forever.

The bug seen the light after the pre-2.1.132-4 irq.c changes in
enable_irq() that doesn't unset the INPROGRESS flag anymore. This is
obviously a right change since the only piece of code that make sense to
play with INPROGRESS is the code in the irq context path (and the irq probing
of course ;).

The bug was causing my ethernet card to lose the network doing simply a
ping flood. The reason of the network-lose was that the irq was been
disabled by the ethernet 8390 driver in a critical section of
ei_start_xmit(). While the irq was disabled the irq happened and so it
bogus setted the IRQ_INPROGRESS flag forever. This way no other ethernet
irq had a chance to run anymore.

The IRQ_INPROGRESS flag has to be set also when we are probing for irqs
and the patch takes care of that. There was an issue also about the IRQ_PENDING
flag, now I set it only if the irq is disabled or if the irq is in progress,
this is the right thing to do according to me.

Here the fix against pre-2.1.132-4, as usual I don' t know nothing about
Copyright issues...

Index: linux/arch/i386/kernel/irq.c
diff -u linux/arch/i386/kernel/irq.c:1.1.1.3 linux/arch/i386/kernel/irq.c:1.1.1.1.2.8
--- linux/arch/i386/kernel/irq.c:1.1.1.3 Sun Dec 20 16:26:03 1998
+++ linux/arch/i386/kernel/irq.c Tue Dec 22 22:53:40 1998
@@ -15,6 +15,13 @@
* Naturally it's not a 1:1 relation, but there are similarities.
*/

+/*
+ * Fixed the IRQ_INPROGRESS handling. IRQ_INPROGRESS has to be set only if
+ * the irq handler exists and if we are going to execute it or as special
+ * case if we are probing for irqs.
+ * Copyright (C) 1998 Andrea Arcangeli
+ */
+
#include <linux/ptrace.h>
#include <linux/errno.h>
#include <linux/kernel_stat.h>
@@ -659,8 +666,8 @@
status = desc->status & ~IRQ_REPLAY;
action = NULL;
if (!(status & (IRQ_DISABLED | IRQ_INPROGRESS)))
- action = desc->action;
- desc->status = status | IRQ_INPROGRESS;
+ if ((action = desc->action) || status & IRQ_AUTODETECT)
+ desc->status = status | IRQ_INPROGRESS;
}
spin_unlock(&irq_controller_lock);

@@ -891,7 +898,7 @@
for (i = NR_IRQS-1; i > 0; i--) {
if (!irq_desc[i].action) {
unsigned int status = irq_desc[i].status | IRQ_AUTODETECT;
- irq_desc[i].status = status & ~IRQ_INPROGRESS;
+ irq_desc[i].status = status & ~(IRQ_INPROGRESS|IRQ_DISABLED);
irq_desc[i].handler->startup(i);
}
}
@@ -915,7 +922,7 @@

/* It triggered already - consider it spurious. */
if (status & IRQ_INPROGRESS) {
- irq_desc[i].status = status & ~IRQ_AUTODETECT;
+ irq_desc[i].status = (status & ~IRQ_AUTODETECT) | IRQ_DISABLED;
irq_desc[i].handler->shutdown(i);
}
}
@@ -944,8 +951,9 @@
if (!nr_irqs)
irq_found = i;
nr_irqs++;
+ status &= ~IRQ_INPROGRESS;
}
- irq_desc[i].status = status & ~IRQ_AUTODETECT;
+ irq_desc[i].status = (status & ~IRQ_AUTODETECT) | IRQ_DISABLED;
irq_desc[i].handler->shutdown(i);
}
spin_unlock_irq(&irq_controller_lock);
Index: linux/arch/i386/kernel/io_apic.c
diff -u linux/arch/i386/kernel/io_apic.c:1.1.1.2 linux/arch/i386/kernel/io_apic.c:1.1.1.1.2.6
--- linux/arch/i386/kernel/io_apic.c:1.1.1.2 Sun Dec 20 16:26:03 1998
+++ linux/arch/i386/kernel/io_apic.c Tue Dec 22 23:23:48 1998
@@ -7,6 +7,13 @@
* patches and reporting/debugging problems patiently!
*/

+/*
+ * Fixed the IRQ_INPROGRESS handling. IRQ_INPROGRESS has to be set only if
+ * the irq handler exists and if we are going to execute it or as special
+ * case if we are probing for irqs.
+ * Copyright (C) 1998 Andrea Arcangeli
+ */
+
#include <linux/sched.h>
#include <linux/smp_lock.h>
#include <linux/init.h>
@@ -1018,7 +1025,6 @@
*/
ack_APIC_irq();
status = desc->status & ~IRQ_REPLAY;
- status |= IRQ_PENDING;

/*
* If the IRQ is disabled for whatever reason, we cannot
@@ -1026,10 +1032,11 @@
*/
action = NULL;
if (!(status & (IRQ_DISABLED | IRQ_INPROGRESS))) {
- action = desc->action;
- status &= ~IRQ_PENDING;
+ if ((action = desc->action) || status & IRQ_AUTODETECT)
+ desc->status = status | IRQ_INPROGRESS;
+ else
+ status |= IRQ_PENDING;
}
- desc->status = status | IRQ_INPROGRESS;
spin_unlock(&irq_controller_lock);

/*
@@ -1079,9 +1086,9 @@
*/
action = NULL;
if (!(status & (IRQ_DISABLED | IRQ_INPROGRESS))) {
- action = desc->action;
+ if ((action = desc->action) || status & IRQ_AUTODETECT)
+ desc->status = status | IRQ_INPROGRESS;
}
- desc->status = status | IRQ_INPROGRESS;

ack_APIC_irq();
spin_unlock(&irq_controller_lock);
Andrea Arcangeli

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/