Yes, it's fine. Thanks. -- Nadav Har'El [email protected]
On Mon, Aug 26, 2019 at 12:07 AM Waldek Kozaczuk <[email protected]> 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 <[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 > <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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjtWWUxZz5P2E0r5PgdQD%3DhAzfnd%3DD34LSiESDbaS3guAg%40mail.gmail.com.
