Hi!
> This is my first real introduction to the IRQ handling code in Linux,
> so please forgive any little errors. I'm fairly sure the big picture
> is right, partly because the patch helps so much.
>
> To explain what I think is happening, let me start with a very simple
> case. A number of PCI devices (this one included) have a number of
> events which can trigger an interrupt. The events which are current
> are presented as bits in a register, and are ORed together (and
> possibly masked by another register) to make the IRQ line.
> When 1's are written to any bits in this register, it acknowledges
> the event and clears the bit.
> A typical code fragment is
> events = read_register(INTERRUPTS);
> write_register(INTERRUPTS, events);
> ... handle each 1 bits in events ....
>
> This would normally clear all pending events and cause the interrupt
> line to go low (or at least to not be asserted).
>
> However there is room for a race here. If an event occurs between
> the read and the write, then this will NOT de-assert the IRQ line.
> It will remain asserted throughout.
>
> Now if the IRQ is handled as an edge-triggered line (which I believe
> they are in Linux), then losing this race will mean that we don't see
> any more interrupts on this line.
I believe that
a) any shared interrupts should be level-triggered. It is not okay to
share edge-triggered interrupt
b) your patch does not fix that issue. It only makes race window
smaller.
> if (!(action->flags & SA_INTERRUPT))
> local_irq_enable();
>
> do {
> ret = action->handler(irq, action->dev_id, regs);
> - if (ret == IRQ_HANDLED)
> + if (ret == IRQ_HANDLED) {
> status |= action->flags;
> + repeat = 1;
> + }
> retval |= ret;
> action = action->next;
> + if (!action &&
> + repeat &&
> + safeirq &&
> + (actionlist->flags & SA_SHIRQ)) {
> + /* at least one handler on the list did something,
> + * and the interrupt is sharable, so give
> + * every handler another chance, incase a new event
> + * came in and is holding the irq line asserted.
> + */
> + action = actionlist;
> + repeat = 0;
> + }
> } while (action);
I think it is still racy. What if another interrupt comes here?
> if (status & SA_SAMPLE_RANDOM)
Pavel
--
Thanks for all the (sleeping) penguins.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
