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

[Baesken, Matthias]

Thanks !

Is  there a  (jtreg)  test  available to  check  that  the prio/niceness  level  changes  with   os::set_native_priority   used ?


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
> >>
> >