RFR : 8215962: Support ThreadPriorityPolicy mode 1 for non-root users on linux/bsd
Baesken, Matthias
matthias.baesken at sap.com
Fri Jan 4 15:38:08 UTC 2019
Hi David/Dan , here is my new webrev, the warning (for non-root) is back and uses the improved wording :
http://cr.openjdk.java.net/~mbaesken/webrevs/8215962.2/
Best Regards , Matthias
> -----Original Message-----
> From: David Holmes <david.holmes at oracle.com>
> Sent: Donnerstag, 3. Januar 2019 23:52
> To: daniel.daugherty at oracle.com; Baesken, Matthias
> <matthias.baesken at sap.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
> dev at openjdk.java.net>
> Subject: Re: RFR : 8215962: Support ThreadPriorityPolicy mode 1 for non-root
> users on linux/bsd
>
> Hi Dan,
>
> Thanks for your reluctant agreement. I like your updated wording for the
> warning.
>
> David
>
> On 4/01/2019 8:21 am, Daniel D. Daugherty wrote:
> > On 1/3/19 4:49 PM, David Holmes wrote:
> >> Hi Matthias,
> >>
> >> On 4/01/2019 12:56 am, Baesken, Matthias wrote:
> >>>
> >>>> I still want to know how the OS_ERR gets handled by all the higher
> >>>> level
> >>>> code. How will this failure at runtime get reported back to
> >>>> application code?
> >>>
> >>> Hi David , the "best practice" currently is to just ignore the
> >>> return code of os::set_native_priority , it is called and we hope
> >>> for the best ,
> >>
> >> Silent failure is not good.
> >
> > Agreed.
> >
> >
> >> In that case I think it is appropriate to issue a warning whenever
> >> ThreadPriorityPolicy=1 is set, though compatibility dictates no
> >> warning in the case that you are root.
> >
> > Agreed that you should not get the warning if you are root.
> > Reluctantly agree to always issue the warning if
> > -XX:ThreadPriorityPolicy=1 is specified and user != root.
> >
> >
> >> Which brings us back to square one and the original patch with a
> >> different warning message:
> >>
> >> warning("-XX:ThreadPriorityPolicy=1 requires system level permissions
> >> to be applied. If these permissions do not exist, changes to priority
> >> will be silently ignored.");
> >
> > Perhaps:
> >
> > warning("-XX:ThreadPriorityPolicy=1 may require system level permission,
> > e.g., being the 'root' user. If the necessary permission is not
> > possessed, changes to priority will be silently ignored.");
> >
> > I changed:
> >
> > - "requires" -> "may require" because you don't always need
> > a special permission to do the operation.
> > - "system level permissions to be applied" -> "system level permission"
> > so switch to singular permission, dropped "to be applied"
> > - added ", e.g., being the 'root' user"
> > - "If these permissions do not exist" -> "If the necessary permission is
> > not possessed"
> > so switch to singular permission, switch from "do not exist" to
> > "is not possessed"
> >
> >
> >> which then takes us back to Dan's comments that the warning should be
> >> at time of use. But to that I maintain that because use may or may not
> >> fail depending on both the available permissions and the requested
> >> priority value, that it is better to have a single generic warning in
> >> the existing place.
> >
> > This is where David and I disagree. I do not think we should issue a
> > warning unless the operation failed and David prefers the generic
> > warning in one place.
> >
> > I will reluctantly agree to always issue the warning if
> > -XX:ThreadPriorityPolicy=1 is specified and user != root.
> >
> > Dan
> >
> >>
> >>> for example :
> >>>
> >>> jdk/src/hotspot/share/runtime/vmThread.cpp
> >>>
> >>> 299 int prio = (VMThreadPriority == -1)
> >>> 300 ? os::java_to_os_priority[NearMaxPriority]
> >>> 301 : VMThreadPriority;
> >>> 302 // Note that I cannot call os::set_priority because it expects Java
> >>> 303 // priorities and I am *explicitly* using OS priorities so that
> >>> it's
> >>> 304 // possible to set the VM thread priority higher than any Java
> >>> thread.
> >>> 305 os::set_native_priority( this, prio );
> >>>
> >>>
> >>> jdk/src/hotspot/share/compiler/compileBroker.cpp
> >>>
> >>> 783 int native_prio = CompilerThreadPriority;
> >>> 784 if (native_prio == -1) {
> >>> 785 if (UseCriticalCompilerThreadPriority) {
> >>> 786 native_prio = os::java_to_os_priority[CriticalPriority];
> >>> 787 } else {
> >>> 788 native_prio = os::java_to_os_priority[NearMaxPriority];
> >>> 789 }
> >>> 790 }
> >>> 791 os::set_native_priority(thread, native_prio);
> >>>
> >>>
> >>> A difference is
> >>>
> >>> jdk/src/hotspot/share/runtime/os.cpp
> >>>
> >>> 217OSReturn os::set_priority(Thread* thread, ThreadPriority p) {
> >>> 218 debug_only(Thread::check_for_dangling_thread_pointer(thread);)
> >>> 219
> >>> 220 if (p >= MinPriority && p <= MaxPriority) {
> >>> 221 int priority = java_to_os_priority[p];
> >>> 222 return set_native_priority(thread, priority);
> >>>
> >>> Where the return code of set_native_priority() is returned
> >>> (however then it is later usually not handled by the callers of
> >>> os::set_priority.
> >>>
> >>>>
> >>>> As I stated this is not a complete statement as on Linux at least you
> >>>> also have to account for RLIMIT_RTPRIO.
> >>>>
> >>>
> >>> If you want me to do, I can for course add a short statement about
> >>> this .
> >>
> >> Quite the opposite, I'd rather see a generic statement about
> >> permissions than try to cover all the possible situations.
> >>
> >> Thanks,
> >> David
> >>
> >>>
> >>> Best regards, Matthias
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes <david.holmes at oracle.com>
> >>>> Sent: Donnerstag, 3. Januar 2019 14:02
> >>>> To: Baesken, Matthias <matthias.baesken at sap.com>;
> >>>> daniel.daugherty at oracle.com; 'hotspot-dev at openjdk.java.net'
> <hotspot-
> >>>> dev at openjdk.java.net>
> >>>> Subject: Re: RFR : 8215962: Support ThreadPriorityPolicy mode 1 for
> >>>> non-root
> >>>> users on linux/bsd
> >>>>
> >>>> Hi Matthias,
> >>>>
> >>>> On 3/01/2019 9:13 pm, Baesken, Matthias wrote:
> >>>>> Hello David and Dan , here is a second webrev :
> >>>>>
> >>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8215962.1/
> >>>>>
> >>>>> - adjusted copyright years + fixed some typos
> >>>>> - added the missing return for FreeBSD (pointed out by Dan)
> >>>>> - removed the warning message completely
> >>>>
> >>>> I still want to know how the OS_ERR gets handled by all the higher
> >>>> level
> >>>> code. How will this failure at runtime get reported back to application
> >>>> code?
> >>>>
> >>>> ! // It is only used when ThreadPriorityPolicy=1 and requires root
> >>>> privilege or
> >>>> ! // CAP_SYS_NICE capability.
> >>>>
> >>>> As I stated this is not a complete statement as on Linux at least you
> >>>> also have to account for RLIMIT_RTPRIO.
> >>>>
> >>>> Thanks,
> >>>> David
> >>>> -----
> >>>>
> >>>
> >
More information about the hotspot-dev
mailing list