On 21/02/06, Anders Blomdell <[EMAIL PROTECTED]> wrote:
Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs.
As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs.
---
> ...
I'll take a look at the rest of the message a bit later.
Dmitry Adamushko wrote:
>
> N.B. Amongst other things, some thoughts about CHAINED with shared
> interrupts.
>
>
> On 20/02/06, *Anders Blomdell* < [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>> wrote:
>
>
>
> A number of questions arise:
>
> 1. What happens if one of the shared handlers leaves the interrupt
> asserted,
> returns NOENABLE|HANDLED and another return only HANDLED?
>
> 2. What happens if one returns PROPAGATE and another returns HANDLED?
>
>
> Yep, each ISR may return a different value and all of them are
> accumulated in the "s" variable ( s |= intr->isr(intr); ).
>
> So the loop may end up with "s" which contains all of the possible bits:
>
> (e.g.
>
> isr1 - HANDLED | ENABLE
> isr2 - HANDLED (don't want the irq to be enabled)
> isr3 - CHAINED
>
> )
>
> s = HANDLED | ENABLE | CHAINED;
>
> Then CHAINED will be ignored because of the following code :
>
> + if (s & XN_ISR_ENABLE)
> + xnarch_end_irq(irq);
> + else if (s & XN_ISR_CHAINED) (*)
> + xnarch_chain_irq(irq);
Which is the worst way possible of prioritizing them, if a Linux interrupt is
active when we get there with ENABLE|CHAINED, interrupts will be enabled with
the Linux interrupt still asserted -> the IRQ-handlers will be called once more,
probably returning ENABLE|CHAINED again -> infinite loop...
>
> the current code in the CVS doen not contain "else" in (*), so that
> ENABLE | CHAINED is possible, though it's a wrong combination.
>
> This said, we suppose that one knows what he is doing.
>
> In the case of a single ISR per line, it's not that difficult to
> achieve. But if there are a few ISRs, then one should analize and take
> into account all possible return values of all the ISRs, as each of them
> may affect others (e.g. if one returns CHAINED when another - HANDLED |
> ENABLE).
Which is somewhat contrary to the concept of shared interrupts, if we have to
take care of the global picture, why make them shared in the first place?
(I like the concept of shared interrupts, but it is important that the framework
gives a separation of concerns)
Unfortunately, it looks to me that the current picture (even with your scalar values) requires from the user who develops a given IRQ to take into account the possible return values of other ISRs.
As I pointed out, the situation when 2 ISRs return HANDLED_NOENABLE may lead to problems on some archs.
---
> ...
I'll take a look at the rest of the message a bit later.
--
Best regards,
Dmitry Adamushko
