RFR : 8215962: Support ThreadPriorityPolicy mode 1 for non-root users on linux/bsd

[David Holmes]

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. 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. 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.");

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.

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