I have sent new updated patch.
On Sunday, August 25, 2019 at 6:06:49 AM UTC-4, Nadav Har'El wrote:
>
>
> On Thu, Aug 22, 2019 at 4:43 AM Waldemar Kozaczuk <[email protected]
> <javascript:>> wrote:
>
>> This patch provides minimal implementation of following
>> 4 functions to handle SCHED_OTHER policy:
>> - sched_get_priority_min
>> - sched_get_priority_max
>> - pthread_getschedparam
>> - pthread_setschedparam
>>
>> This implementation of these 4 functions is enough to make simple 'Hello
>> World' -
>> https://github.com/cloudius-systems/osv-apps/tree/master/mono-example -
>> run properly on OSv.
>>
>> Fixes #34
>>
>
> Thanks. Would be great to close #34, and with such a simple patch, but
> unfortunately I have one nitpick below:
>
>
>> Signed-off-by: Waldemar Kozaczuk <[email protected] <javascript:>>
>> ---
>> libc/pthread.cc | 33 ++++++++++++++++++++++++---------
>> 1 file changed, 24 insertions(+), 9 deletions(-)
>>
>> diff --git a/libc/pthread.cc b/libc/pthread.cc
>> index 125d579f..9b4eb768 100644
>> --- a/libc/pthread.cc
>> +++ b/libc/pthread.cc
>> @@ -928,30 +928,45 @@ void pthread_exit(void *retval)
>> t->_thread->exit();
>> }
>>
>> -int sched_get_priority_max(int policy)
>> +static int sched_get_priority_minmax(int policy)
>>
>
> I don't want to replace the two completely different functions, max() and
> min() with just one.
> Even if it means code duplication. Because "eventually" (maybe in 10 years
> ;-)), these are supposed to
> be different functions.
>
> {
>> - WARN_STUBBED();
>> - return EINVAL;
>> + switch (policy) {
>> + case SCHED_OTHER:
>> + return 0;
>>
>
> Did you check that Linux returns 0 here, and not some actual numbers like
> -20, 20, etc?
>
Yes in two places:
1) Per http://man7.org/linux/man-pages/man2/sched_get_priority_min.2.html which
says this:
"
Linux allows the static priority range 1 to 99 for the *SCHED_FIFO *and
*SCHED_RR *policies, and the priority 0 for the remaining policies.
Scheduling priority ranges for the various policies are not
alterable.
"
2) Also in the code -
https://github.com/torvalds/linux/blob/master/kernel/sched/core.c#L5613-L5656
>
> + default:
>> + return EINVAL;
>> + }
>> }
>>
>> +// Following 4 functions provide minimal implementation
>> +// that ONLY covers default Linux SCHED_OTHER policy
>> int sched_get_priority_min(int policy)
>> {
>> - WARN_STUBBED();
>> - return EINVAL;
>> + return sched_get_priority_minmax(policy);
>> +}
>> +
>> +int sched_get_priority_max(int policy)
>> +{
>> + return sched_get_priority_minmax(policy);
>> }
>>
>> int pthread_setschedparam(pthread_t thread, int policy,
>> const struct sched_param *param)
>> {
>> - WARN_STUBBED();
>> - return EINVAL;
>> + switch (policy) {
>> + case SCHED_OTHER:
>> + return 0;
>> + default:
>> + return EINVAL;
>> + }
>> }
>>
>> int pthread_getschedparam(pthread_t thread, int *policy,
>> struct sched_param *param)
>> {
>> - WARN_STUBBED();
>> - return EINVAL;
>> + *policy = SCHED_OTHER;
>> + param->sched_priority = 0;
>> + return 0;
>> }
>>
>> int pthread_kill(pthread_t thread, int sig)
>> --
>> 2.20.1
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "OSv Development" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to [email protected] <javascript:>.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/osv-dev/20190822014340.7771-1-jwkozaczuk%40gmail.com
>> .
>>
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/d7f2dcd5-e1c2-4321-acd7-98462a45f3b1%40googlegroups.com.
