RFR (S): 8165859: gcTaskThread is missing volatile qualifier and barriers for _time_stamps

[Carsten Varming]

Dear Erik,

See inline.

On Tue, Sep 20, 2016 at 4:49 AM, Erik Österlund <erik.osterlund at oracle.com>
wrote:
>
> I believe Kim is referring precisely to the release in the fence before
> the compare-and-exchange in that comment you refer to, rather than the
> membar Store* after the fence.
>

In this case there is no data, written prior to a successful exchange on
_time_stamps, being passed to the reader of _time_stamps, so the fence
before the exchange is not needed. In fact, it seems to me that a relaxed
atomic cmpxchg (using memory_order_relaxed) is sufficient for the update
and a plain read is sufficient to read _time_stamps.

I would prefer to have explicit memory ordering between the linked producer
> and consumer pair, as Kim suggests. This is how the JMM implements
> sequential consistency - cooperatively with release_store_fence and
> load_acquire pairs on the same data.
>

Hotspot is a C++ program. Mimicking the JMM in Hospot code is misguided
(IMHO). In April, migrating the barriers towards the java model was
suggested (
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-April/019079.html),
but the conclusion was a small move towards C++ 11 barriers (
https://bugs.openjdk.java.net/browse/JDK-8155949). It would be great if
there could be general consensus about the direction of changes to the
memory barriers and their use in Hotspot (cc hotspot-runtime-dev as this
was discussed there recently).


> It might very well be that specific architectures might work fine without
> the acquire for the consumer as the producer implementation itself is
> strong enough to guarantee sequential consistency, but I don't see why it
> would be wise to bet on that if we don't have to, and it does not cost us
> anything.
>

I don't undersand you reference to sequential consistency here as the only
data being passed between threads is the value stored in the field updated
by cmpxchg.

Carsten


> Thanks,
> /Erik
>
>
> On 2016-09-19 21:36, Carsten Varming wrote:
>
> Dear Kim,
>
> Reading a volatile field updated by cmpxchg does not need an acquire
> operation.
>
> BTW. The specification in atomic.hpp says cmpxchg provides: "<fence>
> compare-and-exchange <membar StoreLoad|StoreStore>". The release is part of
> the fence. Are you suggesting that there is a release after the exchange?
>
> Carsten
>
> On Mon, Sep 19, 2016 at 3:18 PM, Kim Barrett <kim.barrett at oracle.com>
> wrote:
>
>> > On Sep 19, 2016, at 2:26 PM, Carsten Varming <varming at gmail.com> wrote:
>> >
>> > Dear Erik,
>> >
>> > According to orderAccess.hpp an acquire is supposed to be paired with a
>> release. It doesn't look like there is any synchronization on the data
>> written to the time stamp array, so what exactly is going on?
>> >
>> > Carsten
>>
>> _time_stamps gets written with a cmpxchg_ptr (line 63 of
>> gcTaskThread.cpp), which includes release semantics for the write; see
>> atomic.hpp.
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160920/9f3ab970/attachment.htm>