RFR [XS] 8215707: [macosx] fix pthread_getschedparam and pthread_setschedparam calls - was : RE: RFR [XS] : 8215707: [macosx] check return code of pthread_getschedparam

[David Holmes]

On 21/12/2018 5:46 pm, Baesken, Matthias wrote:
> Thanks !
> 
> Is  there a  (jtreg)  test  available to  check  that  the prio/niceness  level  changes  with   os::set_native_priority   used ?

Nope. As I've said this code is basically unused. The only time I've 
heard of it being used was on Solaris and that was years back.

David
-----

> 
> As far as I know ,  at least on Linux  it would be possible to observe  the changed  thread  niceness values  from outside  with e.g.
> 
>   top -H     (shows  the NI   (niceness values)   of all thread ),     or  ps  -T -l    (long format,   -l adds the niceness values too at least on my distro).
>   Ps -l  works also on Mac and disaplays the NI value .
> 
> However  currently  the root -check  would block any  testing   under "normal" test users .
> 
> Best regards, Matthias
> 
> 
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Freitag, 21. Dezember 2018 08:23
>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR [XS] 8215707: [macosx] fix pthread_getschedparam and
>> pthread_setschedparam calls - was : RE: RFR [XS] : 8215707: [macosx] check
>> return code of pthread_getschedparam
>>
>> Hi Matthias,
>>
>> On 21/12/2018 5:01 pm, Baesken, Matthias wrote:
>>> Hello David, I adjusted  the  bug synopsis and description,  the second
>> webrev   uses  the pthread id  of the passed  Thread*  instead of
>> pthread_self()  :
>>>
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8215707
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8215707.1/
>>
>> Looks good.
>>
>> Thanks,
>> David
>>
>>>
>>>
>>> Best regards, Matthias
>>>
>>>
>>>> -----Original Message-----
>>>> From: David Holmes <david.holmes at oracle.com>
>>>> Sent: Donnerstag, 20. Dezember 2018 13:19
>>>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>>>> Subject: Re: RFR [XS] : 8215707: [macosx] check return code of
>>>> pthread_getschedparam
>>>>
>>>> On 20/12/2018 9:56 pm, Baesken, Matthias wrote:
>>>>>> Note this code is likely never exercised due to the default
>>>>>> ThreadPriorityPolicy of 0.
>>>>>
>>>>> Hi  David,  ThreadPriorityPolicy=1 is a valid setting / product flag.
>>>>
>>>> Yes it is. That doesn't mean it is actually used to any great extent.
>>>>
>>>>> You find quite a few people setting  ThreadPriorityPolicy when you
>> search
>>>> for it,  so  the coding should better be fixed I guess .
>>>>
>>>> Quite a few? I could only find a couple (Cassandra, Gatling) and they
>>>> were on Linux. I was surprised to find how many people knew about the
>>>> bug that allowed the root check to be by-passed.
>>>>
>>>> The fact this code is incorrect on OS X indicates to me it is not
>>>> actually being used - or by chance priority is only ever changed in the
>>>> current thread.
>>>>
>>>>>>
>>>>>>     That seems more likely given that the code is incorrect:
>>>>>>
>>>>>> pthread_getschedparam(pthread_self(), &policy, &sp);
>>>>>>
>>>>>> this always gets the priority of the current thread, but the method
>> need
>>>>>> not be called on the current thread!
>>>>>
>>>>> Yes , true !
>>>>> Same for  os::set_native_priority  in  os_bsd.cpp , there pthread_self()
>> is
>>>> also  used instead of the  real Thread .
>>>>>
>>>>> 2338OSReturn os::set_native_priority(Thread* thread, int newpri) {
>>>>> 2339  if (!UseThreadPriorities || ThreadPriorityPolicy == 0) return
>> OS_OK;
>>>>> 2340
>>>>>      ....
>>>>> 2346#elif defined(__APPLE__) || defined(__NetBSD__)
>>>>> 2347  struct sched_param sp;
>>>>> 2348  int policy;
>>>>> 2349  pthread_t self = pthread_self();
>>>>> 2350
>>>>> 2351  if (pthread_getschedparam(self, &policy, &sp) != 0) {
>>>>> 2352    return OS_ERR;
>>>>> 2353  }
>>>>
>>>> If you intend fixing this probably best to modify the bug synopsis and
>>>> description.
>>>>
>>>> Cheers,
>>>> David
>>>>
>>>