On 21/02/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
Dmitry Adamushko wrote:
> N.B. Amongst other things, some thoughts about CHAINED with shared
> interrupts.
> On 20/02/06, Anders Blomdell < [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.
> isr2 - HANDLED (don't want the irq to be enabled)
> isr3 - 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 may actually be fatal: consider one ISR intentionally not handling
an IRQ but propagating it while another ISR thinks it has to re-enable
it - bang! Anders' as well as my original suggestion were to ignore the
ENABLE in this case.

> 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).
> So my feeling is that CHAINED should not be used by drivers which registered
> their ISRs as SHARED.

That's true for standard drivers, but we should not prevent special
corner-case designs which can be tuned to make even shared RT-IRQs +
propagation possible. Sharing does not necessarily mean that different
RT drivers are involved...

Yep.  But in this case the designer of the system should _care_ about all corners of the system, i.e. to make sure that all ISRs return values which are not contradictory to each other.


isr1 : HANDLED (keep disabled)
isr2 : CHAINED


isr1 : HANDLED | NOENABLE (will be enabled by rt_task1)
isr2 : HANDLED | NOENABLE (-//- by rt_task2)

1) and 2) are wrong! Both lead to the IRQ line being .end-ed (xnarch_end_irq()) twice! And on some archs, as we have seen before, that may lead to lost interrupts.

Of course, when designing the real-time system, one should take charge of all corners of the system, so that analyzing the possible return values of all other ISRs that may share the same IRQ line is not anything extravagant, but a requirement.

This said, I see no reason then why we should prevent users
from some cases (like CHAINED | ENABLE) and let him do the ones like 2).
2) is much more common scenario for irq sharing and looks sane  from the point of view of a separate ISR, when the global picture (and nucleus innards) is not taken into account.

my 2) is equal to 2 ISRs returning HANDLED_NOENABLE in terms of Anders. This case is one of the most common with shared interrupts which I would imagine. And it nevertheless leads to problems when the whole picture is not taken into account by the designer of a given ISR.

> Moreover, I actually see the only scenario of CHAINED (I provided it before)
> :
> all ISRs in the primary domain have reported UNHANDLED  =>  nucleus
> propagates the interrupt down the pipeline with xnacrh_chain_irq().
> This call actually returns 1 upon successful propagation (some domain down
> the pipeline was interested in this irq) and 0 otherwise.

>But this only means that some handler is registered down there,
>cannot predict if that handler will actually care about the event.

Linux (not only vanilla, but ipipe-aware too) always .end-s (calls desc->handler->end(irq)) on return from __do_IRQ(), no matter this interrupt was handled by some ISR or not. It also re-enables the IRQ line if it was not disabled by some ISR explicitly.
We don't care about anything else.


Best regards,
Dmitry Adamushko

Reply via email to