Re: [Xenomai-core] [PATCH v2 1/4] POSIX: Fix SCHED_RR thread creation

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

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

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>> Jan Kiszka wrote:
>>>>>>>>> Gilles Chanteperdrix wrote:
>>>>>>>>>> Jan Kiszka wrote:
>>>>>>>>>>> @@ -192,6 +192,9 @@ static void *__pthread_trampoline(void *arg)
>>>>>>>>>>>  
>>>>>>>>>>>  	param.sched_priority = iargs->prio;
>>>>>>>>>>>  	policy = iargs->policy;
>>>>>>>>>>> +	if (policy == SCHED_RR)
>>>>>>>>>>> +		/* Restrict round-robin scheduling to the Xenomai domain. */
>>>>>>>>>>> +		policy = SCHED_FIFO;
>>>>>>>>>> Should not there be the same thing in __wrap_pthread_setschedparam ?
>>>>>>>>> Yes, and setschedparam_ex, here we go:
>>>>>>>> Actually, I am wondering if we can not get rid of these calls to
>>>>>>>> __real_pthread_setschedparam, now that propagating kernel-space priority
>>>>>>>> to user-space is done by a signal ?
>>>>>>> Not with the existing code, as that only forwards prio changes, but no
>>>>>>> policy changes.
>>>>>> If we map SCHED_RR to SCHED_FIFO, are there any policy changes?
>>>>> Yes, the initial one. Keep in mind that not all pthread implementations
>>>>> may respect the pthread_attr or have problems assigning SCHED_FIFO to
>>>>> threads of non-root users.
>>>> Ok. What about calling xnshadow_renice in xnshadow_map ?
>>> Sorry, I can't follow. xnshadow_renice does not help user space to find
>>> the right policy, or what do you mean?
>> xnshadow_renice sends a signal to the user-space thread which in turn
>> calls __real_pthread_setschedparam.
> 
> Ah, and user space derives the policy from the prio. That's what I missed.
> 
>> The advantage of this solution is that it avoids calling
>> __real_pthread_setschedparam in many places in the user-space skins.
> 
> We would save the call in the trampolines at least (my other changes in
> the second revision were nonsense BTW - those paths belong to the
> non-Xenomai case). We would just have to pass the priority down on
> pthread_create as well.
> 
> Will look into this and come up with a new version for this patch. But
> that need not delay merging the others.

I played with it, but raising a signal from xnshadow_map is no good
idea: If we inject the signal before the migration, xnshadow_map fails
due to the pending signal, we exit to user space, run the signal,
restart the call, raise the signal, and so on. If we raise after
migration, we immediately force the new thread into secondary mode
again, definitely causing problems when the mode switch warning is
active, but also risking other regressions for xnshadow_map users that
expect to find themselves in primary mode on return.

In short: I think it is better to stick with my original version of this
patch.

Jan


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