Re: [Xenomai] [Xenomai-git] Jan Kiszka : switchtest: Account for invalid last_switch.from field

[Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>]

On 11/18/2013 03:18 PM, Jan Kiszka wrote:
> On 2013-11-18 14:26, Gilles Chanteperdrix wrote:
>> On 11/18/2013 01:41 PM, git repository hosting wrote:
>>> Module: xenomai-jki
>>> Branch: for-forge
>>> Commit: 3e6d8ff9a99262e78655329dc043aacc607eb158
>>> URL:
>>> http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=3e6d8ff9a99262e78655329dc043aacc607eb158
>>>
>>>
>>> Author: Jan Kiszka <jan.kiszka@siemens.com>
>>> Date:   Mon Nov 18 13:19:34 2013 +0100
>>>
>>> switchtest: Account for invalid last_switch.from field
>>>
>>> If we close a test device early, no switch may have yet taken place when
>>> the first call to rtswitch_to_rt/nrt happens. This can cause to_idx to
>>> become -1, and the system will crash. Handle this corner case
>>> gracefully.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> ---
>>>
>>>    kernel/drivers/testing/switchtest.c |   10 ++++++++--
>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/drivers/testing/switchtest.c
>>> b/kernel/drivers/testing/switchtest.c
>>> index 6f77ee9..7d17c5f 100644
>>> --- a/kernel/drivers/testing/switchtest.c
>>> +++ b/kernel/drivers/testing/switchtest.c
>>> @@ -147,8 +147,11 @@ static int rtswitch_to_rt(rtswitch_context_t *ctx,
>>>
>>>        /* to == from is a special case which means
>>>           "return to the previous task". */
>>> -    if (to_idx == from_idx)
>>> +    if (to_idx == from_idx) {
>>>            to_idx = ctx->error.last_switch.from;
>>> +        if (to_idx == -1)
>>> +            return -EINVAL;
>>> +    }
>>
>> I do not see how we can reach rtswitch_to_rt without having switched
>> context, since the first task to run is not an rt task.
>
> Counter question: What should enforce this ordering? And via which call
> stack should last_switch.from be first updated?
>
> I suspect that the RT tasks overtake the non-RT one here, but - granted
> - I didn't understand the control flow and synchronization of this
> driver yet.

At any time, there is at most one task running on each cpu. This task 
then switches to every other task, in turn. The first task to run is the 
"sleeper" task, which calls nanosleep in order to avoid the system to be 
completely paralized.

For instance, with 3 tasks you would get:
1(sleeper)
2
3
1(sleeper)
3
2
1(sleeper)
2
3

etc...


>
> So even if this is not the cause, just curing a symptom, I think it is a
> valid safety measure.

I do not think we need this safety measure: AFAIK it works on 2.6, so I 
would rather make it work on -forge.

-- 
					    Gilles.