On Thu, Aug 22, 2019 at 4:43 AM Waldemar Kozaczuk <[email protected]>
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]>
> ---
>  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?

+        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].
> 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/CANEVyju5NmX68tT1Fj5cbVzwxpjxhNo7qePuzkqXLuxhfNVmKA%40mail.gmail.com.

Reply via email to