From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: Date: Tue, 21 Feb 2006 12:54:24 +0200 From: "Dmitry Adamushko" Subject: Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge) In-Reply-To: <43FAD322.4060001@domain.hid> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_12485_13732239.1140519264889" References: <43F48832.7060804@domain.hid> <43F4E2D3.4010705@domain.hid> <43F793AF.4080805@domain.hid> <43F9C9C1.60101@domain.hid> <43FAD322.4060001@domain.hid> List-Id: "Xenomai life and development \(bug reports, patches, discussions\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anders Blomdell Cc: xenomai@xenomai.org ------=_Part_12485_13732239.1140519264889 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On 21/02/06, Anders Blomdell wrote: > > Dmitry Adamushko wrote: > > > > N.B. Amongst other things, some thoughts about CHAINED with shared > > interrupts. > > > > > > On 20/02/06, *Anders Blomdell* > > 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 |=3D 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 =3D 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 interrup= t > 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 onc= e > 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 the= m > > 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 hav= e > 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 lea= d to problems on some archs. --- > ... I'll take a look at the rest of the message a bit later. -- Best regards, Dmitry Adamushko ------=_Part_12485_13732239.1140519264889 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline
On 21/02/06, Anders Blomdell <anders.blomdell@domain.hid> wrote:
Dmitry Adamushko wrote:
>
> N.B. Amongst other things, some tho= ughts about CHAINED with shared
> interrupts.
>
>
>= On 20/02/06, *Anders Blomdell* < anders.blomdell@domain.hid
> <mailto:anders.blomdell@domain.hid>> wro= te:
>
>
>
>     A number of que= stions arise:
>
>     1. What happens if one of the shared handl= ers leaves the interrupt
>     asserted,
>&= nbsp;    returns NOENABLE|HANDLED and another return only HA= NDLED?
>
>     2. What happens if one retur= ns PROPAGATE and another returns HANDLED?
>
>
> Yep, each ISR may return a different value and all= of them are
> accumulated in the "s" variable ( s |=3D int= r->isr(intr); ).
>
> So the loop may end up with "s&quo= t; 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 =3D HANDLED | ENABLE | CHAINED;
>
> Then CHAINED will be ignored because of the following code :
>> +    if (s & XN_ISR_ENABLE)
> + &= nbsp;     xnarch_end_irq(irq);
> +  &nb= sp; else if (s & XN_ISR_CHAINED)    (*)
>= ; +       xnarch_chain_irq(irq);
Which is the worst way possible of prioritizing them, if a Linux interr= upt is
active when we get there with ENABLE|CHAINED, interrupts will be = enabled with
the Linux interrupt still asserted -> the IRQ-handlers w= ill be called once more,
probably returning ENABLE|CHAINED again -> infinite loop...

&= gt;
> the current code in the CVS doen not contain "else" i= n (*), so that
> ENABLE | CHAINED is possible, though it's a wrong co= mbination.
>
> 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 an= d take
> into account all possible return values of all the ISRs, as each o= f them
> may affect others (e.g. if one returns CHAINED when another = - HANDLED |
> ENABLE).
Which is somewhat contrary to the concept o= f shared interrupts, if we have to
take care of the global picture, why make them shared in the first plac= e?
(I like the concept of shared interrupts, but it is important that th= e 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 lea= d to problems on some archs.

---
> ...

I'll take a look at the rest of the message a bit later.
 

--
Best regards,
Dmitry Adamushko ------=_Part_12485_13732239.1140519264889--