RFR(XS): 8201218: PPC64: Avoid use of yield instruction on spinlock

[Doerr, Martin]

Hi Gustavo,

thanks for the additional explanations. I've pushed it.

Best regards,
Martin


-----Original Message-----
From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Gustavo Romero
Sent: Dienstag, 10. April 2018 06:03
To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
Subject: Re: RFR(XS): 8201218: PPC64: Avoid use of yield instruction on spinlock

Hi Goetz,

On 04/09/2018 01:03 PM, Lindenmaier, Goetz wrote:
> the change looks good, thanks.

Thanks a lot for reviewing it.


> Interesting to find that this was never implemented ...

Yes... they were removed from the text of ISA v3 btw.


> Will setting the priorities interfere with the thread's
> priorities that can be set in Java? In that case, re-setting
> the level might be not correct.

Underneath Java threads is pthreads and for pthreads PPR has not effect at all
in the sense that it does not modify any kernel scheduling priority neither for
the pthread / process that changed the PPR nor for the other pthreads /
processes. Lowering the PPR priority just makes the waiter use less resources
and release resources for the other threads (or CPUs) on the same core,
including for the thread where the holder might be.

On Linux, the PPR value will be saved/restored across the context switches. Just
on a syscall the value will be reset back to its default (i.e. on Linux:  medium
low, 0b011). On AIX I'm not so sure, but I think that on the worst case AIX
will reset back the PPR to its default value even on context switches.


Regards,
Gustavo

> Best regards,
>    Goetz.
> 
>> -----Original Message-----
>> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-
>> bounces at openjdk.java.net] On Behalf Of Gustavo Romero
>> Sent: Freitag, 6. April 2018 02:23
>> To: hotspot-compiler-dev at openjdk.java.net; ppc-aix-port-
>> dev at openjdk.java.net
>> Subject: RFR(XS): 8201218: PPC64: Avoid use of yield instruction on spinlock
>>
>> Hi,
>>
>> Could the following change be reviewed please?
>>
>> bug   : https://bugs.openjdk.java.net/browse/JDK-8201218
>> webrev: http://cr.openjdk.java.net/~gromero/8201218/v1/
>>
>> It avoids the use of 'yield' ('or 27, 27, 27') supposed to yield
>> up thread resources to another thread once, for instance, it's
>> necessary to perform a spinlock as in the current code found in
>> the following functions:
>>
>> MacroAssembler::rtm_retry_lock_on_abort()
>> MacroAssembler::rtm_retry_lock_on_busy()
>>
>> because 'yield' has no effect on POWER processors, being treat as
>> a 'nop' instruction and taking no action over the PPR (Program
>> Priority Register).
>>
>> Instead of 'yield' 'or 1, 1, 1' instruction is used to set thread
>> priority to low before entering the spin loop in
>> rtm_retry_lock_on_busy and a proper 'or' instruction is used after
>> leaving the spin loop in order to set thread priority back to its
>> default value in userspace (to restore it).
>>
>> The default value in PPR's PRI field in userspace, unfortunately,
>> is different on Linux and AIX: on Linux default is 0b011
>> (medium low) [1] whilst on AIX default is 0b100 (medium / aka
>> normal) [2], so for each case a different 'or' is present
>> immediately after leaving the loop.
>>
>> Before entering the loop, after discussing with the PPC Linux
>> team, the recommendation is the following rule of thumb:
>>
>> - For known long delays (about u/mseconds) use 'very low';
>> - For shorter delays use just 'low' priority.
>>
>> So I selected 'low' priority for the spinlock in question.
>>
>> ISA in fact states that "if a program is waiting on a lock, it
>> could set low priority", but it's not clear from the text if
>> it means low in general sense or indeed 'low' (0b010), so I'm
>> using 'low' before entering the loop based upon the rule above.
>>
>> Finally, 'yield' was removed from rtm_retry_lock_on_abort in
>> order to avoid that the program execution leaves that function
>> with PPR set as 'low' since it's possible to "escape" from that
>> function without ever resetting the PPR back to it's default
>> value in userspace.
>>
>> Thank you and best regards,
>> Gustavo
>>
>> [1]
>> https://github.com/torvalds/linux/blob/master/arch/powerpc/include/asm/
>> processor.h#L32
>> [2]
>> https://www.ibm.com/support/knowledgecenter/en/ssw_aix_71/com.ibm.
>> aix.ktechrf1/thread_set_smt_priority.htm
>