Re: PATCH: 2.6.26-rc8: Fix IRQF_DISABLED for shared interrupts



On Sunday 29 June 2008, Stefan Becker wrote:
The problem is caused by this code in handle_IRQ_event():

        if (!(action->flags & IRQF_DISABLED))
                local_irq_enable_in_hardirq();

        do {
                ret = action->handler(irq, action->dev_id);
         ...
                action = action->next;
        } while (action);

For shared interrupts IRQF_DISABLED will only take effect if the first
registered handler sets it.

I was suspecting something rude like that ...

By the way, did you notice the oddness of IRQF_SAMPLE_RANDOM there?
For a shared IRQ, I would rather think that if any IRQ was flagged
as "too predictable for use as IRQ randomness" (by not having that
flag set) then the IRQ should never be sampled ... there's some odd
thought (or non-thought) involved in IRQ sharing.


The attached changes fix the problem for me.

And it looks plausible to me. Seems like this patch (or a variant)
should be merged for 2.6.26-final, yes? Disregarding IRQF_DISABLED
has -- as you noted -- significant potential for oopsage.

I suggest you provide a fully cleaned-up version of this patch with
your signed-off-by line and a proper patch description ... do this
ASAP, since RC8 is getting a bit late for patches to merge!


One technical comment:

--- a/kernel/irq/internals.h
+++ b/kernel/irq/internals.h
@@ -2,6 +2,13 @@
  * IRQ subsystem internal functions and variables:
  */
 
+/*
+ * Internal interrupt flags
+ *
+ * IRQF_DISABLED_CUMULATIVE - one handler in the chain has IRQF_DISABLED
set + */
+#define IRQF_DISABLED_CUMULATIVE       0x80000000
+
 extern int noirqdebug;
 
 /* Set default functions for irq_chip structures: */

I don't think you should need that flag; and if you did, it should be
declared in <linux/irq.h> to prevent anyone else from using that bit
for some other purpose.

Instead, I think you can set IRQF_DISABLED in irq_desc[irq].status
to achieve the same effect.

- Dave

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



Relevant Pages

  • Re: User mode drivers: part 1, interrupt handling (patch for 2.6.11)
    ... >> I can see there'd be problems if the code allowed shared interrupts, ... > write a small IRQ helper module for the kernel or extend the syntax I ... the supervisor so that the supervisor could cut off the IRQ if the ... 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/ ...
    (Linux-Kernel)
  • Re: [PATCH] uio: User IRQ Mode
    ... This patch adds a "User IRQ Mode" to UIO. ... This doesn't work even for some non shared interrupts. ... Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany ...
    (Linux-Kernel)
  • Re: [PATCH] uio: User IRQ Mode
    ... This patch adds a "User IRQ Mode" to UIO. ... This doesn't work even for some non shared interrupts. ... If I take a level triggered interrupt then the IRQ handler code must ...
    (Linux-Kernel)
  • [patch 0/7] Add IRQF_IRQPOLL_IRQ flag to allow irqpoll
    ... irqpoll is broken on some architectures that don't use the IRQ 0 for the timer ... This patch adds a IRQF_IRQPOLL_IRQ flag. ...
    (Linux-Kernel)
  • [patch 0/7] Add IRQF_IRQPOLL flag to allow irqpoll
    ... irqpoll is broken on some architectures that don't use the IRQ 0 for the timer ... This patch adds a IRQF_IRQPOLL flag. ...
    (Linux-Kernel)