[Xenomai-core] SCHED_RR

[Xenomai-core] SCHED_RR

[Jan Kiszka]

Hi Gilles,

here is a trivial test case on my desk that points to SCHED_RR issues of
the POSIX skin: simple pthread_create with an attribute block that has
SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.

The way the scheduling policy and parameters get to that user space
thread leads via __real_pthread_setschedparam after
__pse51_thread_create. This triggers do_setsched_event in shadow.c. But
here we filter out all policies except for SCHED_FIFO. So neither the
priority nor the required XNRRB flag make it to the target therefore.

Even worse, we are running aperiodic timer mode, so this would not have
worked anyway due to the scheduler limitations. Fortunately, it turned
out that the customer is fine with SCHED_FIFO as well, but the broken
priorities and/or lacking error codes on creation are annoying + the
fact that it likely wouldn't work even with periodic timer mode.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] SCHED_RR

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Hi Gilles,
> 
> here is a trivial test case on my desk that points to SCHED_RR issues of
> the POSIX skin: simple pthread_create with an attribute block that has
> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
> 
> The way the scheduling policy and parameters get to that user space
> thread leads via __real_pthread_setschedparam after
> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
> here we filter out all policies except for SCHED_FIFO. So neither the
> priority nor the required XNRRB flag make it to the target therefore.
> 
> Even worse, we are running aperiodic timer mode, so this would not have
> worked anyway due to the scheduler limitations. Fortunately, it turned
> out that the customer is fine with SCHED_FIFO as well, but the broken
> priorities and/or lacking error codes on creation are annoying + the
> fact that it likely wouldn't work even with periodic timer mode.

The fact that it does not work with aperiodic timer is documented.
However, this used to work and was tested with periodic timer (you can
find the tests under sim/skins/posix/testsuite), that is why there is no
reason to return any error code.

-- 
                                                 Gilles.

Re: [Xenomai-core] SCHED_RR

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Hi Gilles,
>>
>> here is a trivial test case on my desk that points to SCHED_RR issues of
>> the POSIX skin: simple pthread_create with an attribute block that has
>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>
>> The way the scheduling policy and parameters get to that user space
>> thread leads via __real_pthread_setschedparam after
>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>> here we filter out all policies except for SCHED_FIFO. So neither the
>> priority nor the required XNRRB flag make it to the target therefore.
>>
>> Even worse, we are running aperiodic timer mode, so this would not have
>> worked anyway due to the scheduler limitations. Fortunately, it turned
>> out that the customer is fine with SCHED_FIFO as well, but the broken
>> priorities and/or lacking error codes on creation are annoying + the
>> fact that it likely wouldn't work even with periodic timer mode.
> 
> The fact that it does not work with aperiodic timer is documented.
> However, this used to work and was tested with periodic timer (you can
> find the tests under sim/skins/posix/testsuite), that is why there is no
> reason to return any error code.

...which is bogus IMHO. Expecting to read the doc when something does
not work as expected is ok. But more or less silently failing even if it
is documented that it will somehow fail is not what Xenomai is usually
known for.

That said, I still don't see (looking at TRUNK) how SCHED_RR should work
even under periodic mode. Guess I have to enable and test it.

But I guess the best we can do is to finally remove this limitation by
allowing the use to create a periodic tick timer over aperiodic mode if
there is a need for RR scheduling.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] SCHED_RR

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Hi Gilles,
>>>
>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>> the POSIX skin: simple pthread_create with an attribute block that has
>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>
>>> The way the scheduling policy and parameters get to that user space
>>> thread leads via __real_pthread_setschedparam after
>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>> priority nor the required XNRRB flag make it to the target therefore.
>>>
>>> Even worse, we are running aperiodic timer mode, so this would not have
>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>> priorities and/or lacking error codes on creation are annoying + the
>>> fact that it likely wouldn't work even with periodic timer mode.
>> The fact that it does not work with aperiodic timer is documented.
>> However, this used to work and was tested with periodic timer (you can
>> find the tests under sim/skins/posix/testsuite), that is why there is no
>> reason to return any error code.
> 
> ...which is bogus IMHO.

What is bogus ? Not to return an error when something works ?

> Expecting to read the doc when something does
> not work as expected is ok. But more or less silently failing even if it
> is documented that it will somehow fail is not what Xenomai is usually
> known for.
> 
> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
> even under periodic mode. Guess I have to enable and test it.

It would work if __wrap_pthread_create called
__wrap_pthread_setschedparam which would issue a syscall passing the
policy and priority to kernel-space. I am almost sure that it used to
work, but it may happen that recent changes regarding the way priority
is handled broke it.

> 
> But I guess the best we can do is to finally remove this limitation by
> allowing the use to create a periodic tick timer over aperiodic mode if
> there is a need for RR scheduling.

Yes, a very low priority change on the todo list. Someone needing this
in a real-world application would boost the priority.

-- 
                                                 Gilles.

Re: [Xenomai-core] SCHED_RR

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Hi Gilles,
>>>
>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>> the POSIX skin: simple pthread_create with an attribute block that has
>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>
>>> The way the scheduling policy and parameters get to that user space
>>> thread leads via __real_pthread_setschedparam after
>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>> priority nor the required XNRRB flag make it to the target therefore.
>>>
>>> Even worse, we are running aperiodic timer mode, so this would not have
>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>> priorities and/or lacking error codes on creation are annoying + the
>>> fact that it likely wouldn't work even with periodic timer mode.
>> The fact that it does not work with aperiodic timer is documented.
>> However, this used to work and was tested with periodic timer (you can
>> find the tests under sim/skins/posix/testsuite), that is why there is no
>> reason to return any error code.
> 
> ...which is bogus IMHO. Expecting to read the doc when something does
> not work as expected is ok. But more or less silently failing even if it
> is documented that it will somehow fail is not what Xenomai is usually
> known for.
> 
> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
> even under periodic mode. Guess I have to enable and test it.
> 
> But I guess the best we can do is to finally remove this limitation by
> allowing the use to create a periodic tick timer over aperiodic mode if
> there is a need for RR scheduling.

In ksrc/skins/posix/syscall.c, function __pthread_create:

        pthread_attr_init(&attr);
        attr.policy = p->policy;
        param.sched_priority = p->rt_priority;
        attr.detachstate = PTHREAD_CREATE_DETACHED;

So, the priority and policy are taken from the underlying Linux thread.

-- 
                                                 Gilles.

Re: [Xenomai-core] SCHED_RR

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Hi Gilles,
>>>>
>>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>>> the POSIX skin: simple pthread_create with an attribute block that has
>>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>>
>>>> The way the scheduling policy and parameters get to that user space
>>>> thread leads via __real_pthread_setschedparam after
>>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>>> priority nor the required XNRRB flag make it to the target therefore.
>>>>
>>>> Even worse, we are running aperiodic timer mode, so this would not have
>>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>>> priorities and/or lacking error codes on creation are annoying + the
>>>> fact that it likely wouldn't work even with periodic timer mode.
>>> The fact that it does not work with aperiodic timer is documented.
>>> However, this used to work and was tested with periodic timer (you can
>>> find the tests under sim/skins/posix/testsuite), that is why there is no
>>> reason to return any error code.
>> ...which is bogus IMHO. Expecting to read the doc when something does
>> not work as expected is ok. But more or less silently failing even if it
>> is documented that it will somehow fail is not what Xenomai is usually
>> known for.
>>
>> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
>> even under periodic mode. Guess I have to enable and test it.
>>
>> But I guess the best we can do is to finally remove this limitation by
>> allowing the use to create a periodic tick timer over aperiodic mode if
>> there is a need for RR scheduling.
> 
> In ksrc/skins/posix/syscall.c, function __pthread_create:
> 
>         pthread_attr_init(&attr);
>         attr.policy = p->policy;
>         param.sched_priority = p->rt_priority;
>         attr.detachstate = PTHREAD_CREATE_DETACHED;
> 
> So, the priority and policy are taken from the underlying Linux thread.

Yep, but its prio/policy is set *after* the chunk above. Maybe it is
already a sufficient hot-fix to move __real_pthread_setschedparam before
__pse51_thread_create if this has no unwanted side effects.

Jan


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

Re: [Xenomai-core] SCHED_RR

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Hi Gilles,
>>>>>
>>>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>>>> the POSIX skin: simple pthread_create with an attribute block that has
>>>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>>>
>>>>> The way the scheduling policy and parameters get to that user space
>>>>> thread leads via __real_pthread_setschedparam after
>>>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>>>> priority nor the required XNRRB flag make it to the target therefore.
>>>>>
>>>>> Even worse, we are running aperiodic timer mode, so this would not have
>>>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>>>> priorities and/or lacking error codes on creation are annoying + the
>>>>> fact that it likely wouldn't work even with periodic timer mode.
>>>> The fact that it does not work with aperiodic timer is documented.
>>>> However, this used to work and was tested with periodic timer (you can
>>>> find the tests under sim/skins/posix/testsuite), that is why there is no
>>>> reason to return any error code.
>>> ...which is bogus IMHO. Expecting to read the doc when something does
>>> not work as expected is ok. But more or less silently failing even if it
>>> is documented that it will somehow fail is not what Xenomai is usually
>>> known for.
>>>
>>> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
>>> even under periodic mode. Guess I have to enable and test it.
>>>
>>> But I guess the best we can do is to finally remove this limitation by
>>> allowing the use to create a periodic tick timer over aperiodic mode if
>>> there is a need for RR scheduling.
>> In ksrc/skins/posix/syscall.c, function __pthread_create:
>>
>>         pthread_attr_init(&attr);
>>         attr.policy = p->policy;
>>         param.sched_priority = p->rt_priority;
>>         attr.detachstate = PTHREAD_CREATE_DETACHED;
>>
>> So, the priority and policy are taken from the underlying Linux thread.
> 
> Yep, but its prio/policy is set *after* the chunk above. Maybe it is
> already a sufficient hot-fix to move __real_pthread_setschedparam before
> __pse51_thread_create if this has no unwanted side effects.

Useless. Since do_setsched_event ignores the priority change, everything
should work as expected.

-- 
                                                 Gilles.

Re: [Xenomai-core] SCHED_RR

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Hi Gilles,
>>>>>>
>>>>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>>>>> the POSIX skin: simple pthread_create with an attribute block that has
>>>>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>>>>
>>>>>> The way the scheduling policy and parameters get to that user space
>>>>>> thread leads via __real_pthread_setschedparam after
>>>>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>>>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>>>>> priority nor the required XNRRB flag make it to the target therefore.
>>>>>>
>>>>>> Even worse, we are running aperiodic timer mode, so this would not have
>>>>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>>>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>>>>> priorities and/or lacking error codes on creation are annoying + the
>>>>>> fact that it likely wouldn't work even with periodic timer mode.
>>>>> The fact that it does not work with aperiodic timer is documented.
>>>>> However, this used to work and was tested with periodic timer (you can
>>>>> find the tests under sim/skins/posix/testsuite), that is why there is no
>>>>> reason to return any error code.
>>>> ...which is bogus IMHO. Expecting to read the doc when something does
>>>> not work as expected is ok. But more or less silently failing even if it
>>>> is documented that it will somehow fail is not what Xenomai is usually
>>>> known for.
>>>>
>>>> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
>>>> even under periodic mode. Guess I have to enable and test it.
>>>>
>>>> But I guess the best we can do is to finally remove this limitation by
>>>> allowing the use to create a periodic tick timer over aperiodic mode if
>>>> there is a need for RR scheduling.
>>> In ksrc/skins/posix/syscall.c, function __pthread_create:
>>>
>>>         pthread_attr_init(&attr);
>>>         attr.policy = p->policy;
>>>         param.sched_priority = p->rt_priority;
>>>         attr.detachstate = PTHREAD_CREATE_DETACHED;
>>>
>>> So, the priority and policy are taken from the underlying Linux thread.
>> Yep, but its prio/policy is set *after* the chunk above. Maybe it is
>> already a sufficient hot-fix to move __real_pthread_setschedparam before
>> __pse51_thread_create if this has no unwanted side effects.
> 
> Useless. Since do_setsched_event ignores the priority change, everything
> should work as expected.
> 

Don't get what you mean.

Anyway, that reordering doesn't work for shadow threads as they do not
go through xnpod_start_thread from pthread_create, so the XNRRB is not
applied this way.

Thus the only fix to get SCHED_RR working again is to call
__wrap_pthread_setschedparam (or some extract of it) from
__pthread_trampoline, as you suggested. That works, but I cannot asses
right now if it may have side effects.

Jan


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

Re: [Xenomai-core] SCHED_RR

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Hi Gilles,
>>>>>>>
>>>>>>> here is a trivial test case on my desk that points to SCHED_RR issues of
>>>>>>> the POSIX skin: simple pthread_create with an attribute block that has
>>>>>>> SCHED_RR set, but neither SCHED_RR nor the priority reach the new thread.
>>>>>>>
>>>>>>> The way the scheduling policy and parameters get to that user space
>>>>>>> thread leads via __real_pthread_setschedparam after
>>>>>>> __pse51_thread_create. This triggers do_setsched_event in shadow.c. But
>>>>>>> here we filter out all policies except for SCHED_FIFO. So neither the
>>>>>>> priority nor the required XNRRB flag make it to the target therefore.
>>>>>>>
>>>>>>> Even worse, we are running aperiodic timer mode, so this would not have
>>>>>>> worked anyway due to the scheduler limitations. Fortunately, it turned
>>>>>>> out that the customer is fine with SCHED_FIFO as well, but the broken
>>>>>>> priorities and/or lacking error codes on creation are annoying + the
>>>>>>> fact that it likely wouldn't work even with periodic timer mode.
>>>>>> The fact that it does not work with aperiodic timer is documented.
>>>>>> However, this used to work and was tested with periodic timer (you can
>>>>>> find the tests under sim/skins/posix/testsuite), that is why there is no
>>>>>> reason to return any error code.
>>>>> ...which is bogus IMHO. Expecting to read the doc when something does
>>>>> not work as expected is ok. But more or less silently failing even if it
>>>>> is documented that it will somehow fail is not what Xenomai is usually
>>>>> known for.
>>>>>
>>>>> That said, I still don't see (looking at TRUNK) how SCHED_RR should work
>>>>> even under periodic mode. Guess I have to enable and test it.
>>>>>
>>>>> But I guess the best we can do is to finally remove this limitation by
>>>>> allowing the use to create a periodic tick timer over aperiodic mode if
>>>>> there is a need for RR scheduling.
>>>> In ksrc/skins/posix/syscall.c, function __pthread_create:
>>>>
>>>>         pthread_attr_init(&attr);
>>>>         attr.policy = p->policy;
>>>>         param.sched_priority = p->rt_priority;
>>>>         attr.detachstate = PTHREAD_CREATE_DETACHED;
>>>>
>>>> So, the priority and policy are taken from the underlying Linux thread.
>>> Yep, but its prio/policy is set *after* the chunk above. Maybe it is
>>> already a sufficient hot-fix to move __real_pthread_setschedparam before
>>> __pse51_thread_create if this has no unwanted side effects.
>> Useless. Since do_setsched_event ignores the priority change, everything
>> should work as expected.
>>
> 
> Don't get what you mean.
> 
> Anyway, that reordering doesn't work for shadow threads as they do not
> go through xnpod_start_thread from pthread_create, so the XNRRB is not
> applied this way.

That is the point I missed.

> 
> Thus the only fix to get SCHED_RR working again is to call
> __wrap_pthread_setschedparam (or some extract of it) from
> __pthread_trampoline, as you suggested. That works, but I cannot asses
> right now if it may have side effects.

Ok.

-- 
					    Gilles.