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

Linus Torvalds (torvalds@transmeta.com)
Sat, 25 Apr 1998 18:15:22 -0700 (PDT)


On 25 Apr 1998, Claus-Justus Heine wrote:
>
> Nice to see that the above was implemented in 2.1.98
>
> I have one further remark, talking about 2.1.98:

I've made some other changes in my tree already, I always found the
repeating and counting to be of dubious value - if we have nested
interrupts there is no point in trying to count them up. 2.1.98 makes one
counter be a simple flag, and I think the other counters should be simple
flags too.

Would some io-apic people try out this patch to irq.c (patch against
2.1.98)? It simplifies the irq handling quite a bit, I'd like to hear
whether it is stable for you..

Linus

-----
--- v2.1.98/linux/arch/i386/kernel/irq.c Sat Apr 25 18:13:10 1998
+++ linux/arch/i386/kernel/irq.c Sat Apr 25 18:12:37 1998
@@ -673,16 +673,6 @@
set_8259A_irq_mask(irq);
}

-#ifdef __SMP__
-static void disable_ioapic_irq(unsigned int irq)
-{
- disabled_irq[irq] = 1;
- /*
- * We do not disable IO-APIC irqs in hardware ...
- */
-}
-#endif
-
void enable_8259A_irq (unsigned int irq)
{
unsigned long flags;
@@ -693,30 +683,50 @@
}

#ifdef __SMP__
-void enable_ioapic_irq (unsigned int irq)
-{
- unsigned long flags, should_handle_irq;
- int cpu = smp_processor_id();

- spin_lock_irqsave(&irq_controller_lock, flags);
- disabled_irq[irq] = 0;
-
- /*
- * In the SMP+IOAPIC case it might happen that there are an unspecified
- * number of pending IRQ events unhandled. These cases are very rare,
- * so we 'resend' these IRQs via IPIs, to the same CPU. It's much
- * better to do it this way as thus we dont have to be aware of
- * 'pending' interrupts in the IRQ path, except at this point.
- */
+/*
+ * In the SMP+IOAPIC case it might happen that there are an unspecified
+ * number of pending IRQ events unhandled. These cases are very rare,
+ * so we 'resend' these IRQs via IPIs, to the same CPU. It's much
+ * better to do it this way as thus we dont have to be aware of
+ * 'pending' interrupts in the IRQ path, except at this point.
+ *
+ * This code appears still badly broken on some machines, and is thus
+ * disabled.
+ */
+static inline void trigger_pending_irqs(unsigned int irq)
+{
+#if 0
if (irq_events[irq]) {
if (!ipi_pending[irq]) {
ipi_pending[irq] = 1;
--irq_events[irq];
- send_IPI(cpu,IO_APIC_VECTOR(irq));
+ send_IPI(smp_processor_id(), IO_APIC_VECTOR(irq));
}
}
+#endif
+}
+
+void enable_ioapic_irq (unsigned int irq)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&irq_controller_lock, flags);
+ disabled_irq[irq] = 0;
+
+ trigger_pending_irqs(irq);
+
spin_unlock_irqrestore(&irq_controller_lock, flags);
}
+
+static void disable_ioapic_irq(unsigned int irq)
+{
+ disabled_irq[irq] = 1;
+ /*
+ * We do not disable IO-APIC irqs in hardware ...
+ */
+}
+
#endif

void enable_irq(unsigned int irq)
@@ -773,32 +783,25 @@
#ifdef __SMP__
static void do_ioapic_IRQ(unsigned int irq, int cpu, struct pt_regs * regs)
{
- int should_handle_irq = 0;
+ int should_handle_irq;

ack_APIC_irq();

spin_lock(&irq_controller_lock);
- if (ipi_pending[irq])
- ipi_pending[irq] = 0;
-
- if (!irq_events[irq]++ && !disabled_irq[irq])
- should_handle_irq = 1;
+ ipi_pending[irq] = 0;
+ irq_events[irq] = 0;
+ should_handle_irq = 1;
+ if (disabled_irq[irq]) {
+ should_handle_irq = 0;
+ irq_events[irq] = 1;
+ }
hardirq_enter(cpu);
spin_unlock(&irq_controller_lock);

if (should_handle_irq) {
while (test_bit(0,&global_irq_lock)) mb();
-again:
- handle_IRQ_event(irq, regs);

- spin_lock(&irq_controller_lock);
- should_handle_irq=0;
- if (--irq_events[irq] && !disabled_irq[irq])
- should_handle_irq=1;
- spin_unlock(&irq_controller_lock);
-
- if (should_handle_irq)
- goto again;
+ handle_IRQ_event(irq, regs);
}

hardirq_exit(cpu);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu