RFR 8201799: Build failures after JDK-8195099 (Concurrent safe-memory-reclamation mechanism)
Erik Österlund
erik.osterlund at oracle.com
Thu Apr 19 14:36:49 UTC 2018
Hi Doerr,
I would welcome doing the same to Atomic::inc/dec/add/sub that has
already been done for Atomic::cmpxchg. I think that should make everyone
involved happy.
Thanks,
/Erik
On 2018-04-18 18:37, Doerr, Martin wrote:
> Hi Erik,
>
> thank you for bringing this issue up again.
>
> In my opinion, good multi-threaded code specifies ordering semantics precisely for each lock-free access. I think this was done well enough here by adding a comment.
>
> It'd be great if we could specify semantics for Atomic:add like we do for Atomic::cmpchgx.
> However, the double-fence is a very bad default from performance perspective. I wonder if PPC64 is the only platform which gets hit.
> Would it be acceptable to set the default to memory_order_acquire_release and specify memory_order_conservative for the new usage? I think this would fit perfectly for PPC64, but some people may not like it. One could say PPC64 is the problem, but one could also say the VM code is the problem
>
> I don't really like the straight forward fix to insert a fence with #ifdef AIX.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Erik Österlund [mailto:erik.osterlund at oracle.com]
> Sent: Mittwoch, 18. April 2018 14:47
> To: David Holmes <david.holmes at oracle.com>; Aleksey Shipilev <shade at redhat.com>; hotspot-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>
> Subject: Re: RFR 8201799: Build failures after JDK-8195099 (Concurrent safe-memory-reclamation mechanism)
>
> Hi,
>
> Please also note that the Atomic::add used in write_synchronize assumes
> bidirectional fencing (which is the contract provided by Atomic::add).
> This is however not true for the PPC backend of Atomic::add - it has
> acq_rel semantics instead. So this mechanism is currently not safe to
> use on PPC.
>
> From globalCounter.cpp:
> 61 // Atomic::add must provide fence since we have storeload
> dependency.
> 62 volatile uintx gbl_cnt = Atomic::add((uintx)COUNTER_INCREMENT,
> &_global_counter._counter);
>
> Might want to have a look at the safe use of this mechanism on PPC as a
> follow-up.
>
> Thanks,
> /Erik
>
> On 2018-04-18 14:27, David Holmes wrote:
>> Hi Aleksey,
>>
>> On 18/04/2018 9:50 PM, Aleksey Shipilev wrote:
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8201799
>>>
>>> Fix:
>>>
>>> diff -r bfba4712d4ff
>>> src/hotspot/share/utilities/globalCounter.inline.hpp
>>> --- a/src/hotspot/share/utilities/globalCounter.inline.hpp Wed Apr 18
>>> 11:36:48 2018 +0200
>>> +++ b/src/hotspot/share/utilities/globalCounter.inline.hpp Wed Apr 18
>>> 13:50:34 2018 +0200
>>> @@ -27,6 +27,7 @@
>>>
>>> #include "runtime/orderAccess.inline.hpp"
>>> #include "runtime/thread.hpp"
>> The above line can be deleted now.
>>
>>> +#include "runtime/thread.inline.hpp"
>> Fix is good.
>>
>> Thanks,
>> David
>>
>>> #include "utilities/globalCounter.hpp"
>>>
>>> inline void GlobalCounter::critical_section_begin(Thread *thread) {
>>>
>>> Testing: failing aarch64 build
>>>
>>> Thanks,
>>> -Aleksey
>>>
More information about the hotspot-dev
mailing list