Re: [Xenomai-help] xeno-2.3.1 shared interrups BUG?

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 4098 bytes --]

Dmitry Adamushko wrote:
> On 19/06/07, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>> Dmitry Adamushko wrote:
>> > On 19/06/07, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>> >> apittaluga@domain.hid wrote:
>> >> > Hi,
>> >> > running a simple test application which spawns a periodic task
>> >> writing on a
>> >> > serial interface
>> >> > the system hangs performing the rt_dev_close.
>> >> > The test program ran fine with xeno 2.2.6 with "Shared Interrupts"
>> >> enabled,
>> >> > so as with
>> >> > xeno 2.3.1 with "Shared Interrupts" disabled. It fails with xeno
>> >> 2.3.1 with
>> >> > "Shared Interrupts" enabled, so the problem seems to be in the
>> shared
>> >> > interrupts handling area.
>> >> > kernel is 2.6.20 adeos patched
>> >> >
>> >> > Any suggestion?
>> >
>> > Does the fix below eliminate the problem?
>> >
>> > The problem (allegedly) is cause by the following reinitialization at
>> > the end of the loop:
>> >
>> > ...
>> >                if (!(intr = intr->next))
>> >                        intr = shirq->handlers;
>> > ...
>> >
>> > 'end' may point to some of the elements ... and shirq->handlers may
>> > become NULL (all elements have been deleted)..
>>
>> Good catch.
>>
>> >
>> > (white-space damaged version.. enclosed a normal one)
>> >
>> > --- ksrc/nucleus/intr.c-orig    2007-06-19 13:44:55.090623404 +0200
>> > +++ ksrc/nucleus/intr.c 2007-06-19 13:45:53.867440067 +0200
>> > @@ -273,7 +273,7 @@ static void xnintr_edge_shirq_handler(un
>> >        xnintr_shirq_lock(shirq);
>> >        intr = shirq->handlers;
>> >
>> > -       while (intr != end) {
>> > +       while (intr && intr != end) {
>> >                int ret, code;
>> >
>> >                xnstat_runtime_switch(sched,
>> >
>> >
>>
>> But your patch looks incomplete: What if someone removes "end" but
>> leaves other handlers behind while we are looping? Neither intr would
>> then become NULL nor would we hit the end again. This seems to be more
>> tricky...
> 
> Yeah.. what's about smth like this? (quick approach: if not ok, will
> have to elaborate it thoroughly :-)
> 
> 
> --- ksrc/nucleus/intr.c-orig    2007-06-19 13:44:55.090623404 +0200
> +++ ksrc/nucleus/intr.c 2007-06-19 14:38:36.073535255 +0200
> @@ -259,7 +259,7 @@ static void xnintr_edge_shirq_handler(un
>        xnstat_runtime_t *prev;
>        xnticks_t start;
>        xnintr_shirq_t *shirq = &xnshirqs[irq];
> -       xnintr_t *intr, *end = NULL;
> +       xnintr_t *intr, *end = NULL, *old_end = NULL;
>        int s = 0, counter = 0;
> 
>        xnarch_memory_barrier();
> @@ -273,7 +273,7 @@ static void xnintr_edge_shirq_handler(un
>        xnintr_shirq_lock(shirq);
>        intr = shirq->handlers;
> 
> -       while (intr != end) {
> +       while (intr && intr != end) {
>                int ret, code;
> 
>                xnstat_runtime_switch(sched,
> @@ -297,8 +297,14 @@ static void xnintr_edge_shirq_handler(un
>                if (counter++ > MAX_EDGEIRQ_COUNTER)
>                        break;
> 
> -               if (!(intr = intr->next))
> +               if (!(intr = intr->next)) {
>                        intr = shirq->handlers;
> +
> +                       /* 'end' has been removed in the mean time. */
> +                       if (end && old_end == end)
> +                               intr = NULL;
> +                       old_end = end;
> +               }

"end" may still remain stuck on an xnintr object that was removed.
Unless some other element becomes "end" while continuing with the chain,
I don't see a way out of this loop.

I currently have a new approach in mind:

- work with two chains, one is active and remains so as long as we
  iterate over it in the hander, the other gets modified and then marked
  active for succeeding handler entries

- turn the shared-edge chains into rings (make the last point to the
  first) so that we can drop "if (!(intr = intr->next))"

The latter is actually an unrelated optimisation, I just wanted to save
the idea. :)

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]