I have just committed this patch by accident. But I hope it addressed your suggestions accurately.
On Sunday, August 25, 2019 at 1:22:14 PM UTC-4, Waldek Kozaczuk wrote: > > 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]> >> 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? >> > 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]. >>> 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/d699b16c-ce63-457a-92c2-68e3a1918dbb%40googlegroups.com.
