RFR(M): 8202080: Introduce ordering semantics for Atomic::add

[David Holmes]

Hi Martin,

You've added:

   enum cmpxchg_memory_order {
     memory_order_relaxed = 0,
!   memory_order_acquire = 2,
!   memory_order_release = 3,
!   memory_order_acq_rel = 4,

but not defined them. What do they mean? Are they the same as C++11/17? 
Do we actually want that? There was a discussion on all this when the 
enum was introduced. Also note the enum is named cmpxchg_memory_order - 
if we're going to generalise this then it should be renamed and 
behaviour clearly specified.

!   inline static D add(I add_value, D volatile* dest,
!                       cmpxchg_memory_order order = memory_order_acq_rel);

The default should be conservative - to match what is stated in 
atomic.hpp regarding expectations for all atomic r-m-w operations.

  63   // All of the atomic operations that imply a read-modify-write action
   64   // guarantee a two-way memory barrier across that operation. 
Historically
   65   // these semantics reflect the strength of atomic operations 
that are
   66   // provided on SPARC/X86. We assume that strength is necessary 
unless
   67   // we can prove that a weaker form is sufficiently safe.

That's how "conservative" was defined when this was introduced. Some of 
the ports chose to not follow this "contract" and that's up to them, but 
only those ports should be utilising weaker operations unless it can be 
proven that a weaker form is always correct. The onus is on the person 
changing the code. (There was long discussion when a relaxed cmpxchg was 
attempted to be introduced and it was shown to not be correct.)

David
-----

On 21/04/2018 2:04 AM, Doerr, Martin wrote:
> Hi,
> 
> JDK-8195099 “Concurrent safe-memory-reclamation mechanism” introduced a 
> usage of Atomic::add for which we’re not using the correct memory 
> barriers on PPC64 and s390.
> 
> Some more description can be found in the bug: 
> https://bugs.openjdk.java.net/browse/JDK-8202080
> 
> We can fix this by the following change:
> 
> http://cr.openjdk.java.net/~mdoerr/8202080_atomic_add/webrev.00/
> 
> Please note that I only implemented the platform code for linux x86, 
> PPC64 and s390 so far. The remaining ones should be trivial and I’ll 
> take care of them after I got some feedback.
> 
> I can also do the same change for other atomic functions like sub, inc 
> and dec if you like.
> 
> Intention of this change is only to fix this new code and possibly other 
> future enhancements, not to change the behavior of any older usages of 
> Atomic::add.
> 
> Feedback is welcome.
> 
> Best regards,
> 
> Martin
>