Yes, it's fine. Thanks. -- Nadav Har'El n...@scylladb.com
On Mon, Aug 26, 2019 at 12:07 AM Waldek Kozaczuk <jwkozac...@gmail.com> wrote: > 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 <jwkoz...@gmail.com> >>> 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 <jwkoz...@gmail.com> >>>> --- >>>> 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 osv...@googlegroups.com. >>>> 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 osv-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/osv-dev/d699b16c-ce63-457a-92c2-68e3a1918dbb%40googlegroups.com > <https://groups.google.com/d/msgid/osv-dev/d699b16c-ce63-457a-92c2-68e3a1918dbb%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- 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 osv-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjtWWUxZz5P2E0r5PgdQD%3DhAzfnd%3DD34LSiESDbaS3guAg%40mail.gmail.com.
