RFR: 8263551: Provide shared lock-free FIFO queue implementation

[Kim Barrett]

On Sat, 13 Mar 2021 10:59:16 GMT, Man Cao <manc at openjdk.org> wrote:

>> Hi all,
>> 
>> Could anyone review this change that is mainly code motion? It creates a generalized lock-free queue implementation based on G1DirtyCardQueueSet::Queue, which will be used by JDK-8236485 in the future.
>> 
>> The shared LockFreeQueue is similar to the existing LockFreeStack. The notable difference is that the LockFreeQueue has an additional template parameter for whether to use GlobalCounter::CriticalSection to avoid ABA problem.
>> 
>> -Man
>
> src/hotspot/share/utilities/lockFreeQueue.hpp line 99:
> 
>> 97:     } else {
>> 98:       assert(get_next(*old_tail) == NULL, "invariant");
>> 99:       Atomic::store(next_ptr(*old_tail), &first);
> 
> I changed this store from a normal store to an Atomic store. Otherwise there is a data race between this store and the load of _head->_next in pop().

As David said, Atomic::store doesn't indicate any ordering; it's a relaxed atomic store.  The old code was `old_tail->set_next(&first)`, which was hard-wired to the element type being BufferNode (which was okay in its place).  But the BufferNode code predates consistent use of Atomic when accessing atomic/volatile data, so don't currently use Atomic::load/store.  In this generic hoist we no longer want to assume next and set_next functions, just the next_ptr function that returns a pointer to atomic/volatile.  I see that you added get_next, which uses Atomic::load; I think there should be an associated set_next, as there are other places that need it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2986