[aarch64-port-dev ] RFR(S): 8248851: CMS: Missing memory fences between free chunk check and klass read

[Yangfei (Felix)]

Hi,

  Thanks for the suggestions.  It makes sense to me. 
  BTW: OrderAccess::loadload() and OrderAccess::acquire() both map to the same instruction for aarch64: dmb ishld. 
  Updated webrev: 
    http://cr.openjdk.java.net/~fyang/8248851-8u/webrev.01/ 
    http://cr.openjdk.java.net/~fyang/8248851-11u/webrev.01/ 
    http://cr.openjdk.java.net/~fyang/8248851-13u/webrev.01/ 

  Performed the same test as before, result looks good. 
  Does it look better? 

Felix

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Friday, July 17, 2020 6:30 AM
> To: Andrew Haley <aph at redhat.com>; Yangfei (Felix)
> <felix.yang at huawei.com>; Kim Barrett <kim.barrett at oracle.com>
> Cc: jdk8u-dev at openjdk.java.net; hotspot-gc-dev at openjdk.java.net;
> aarch64-port-dev at openjdk.java.net; jdk-updates-dev at openjdk.java.net
> Subject: Re: [aarch64-port-dev ] RFR(S): 8248851: CMS: Missing memory
> fences between free chunk check and klass read
> 
> On 16/07/2020 11:43 pm, Andrew Haley wrote:
> > On 16/07/2020 10:16, David Holmes wrote:
> >> Seems to me that you want OrderAccess::loadload() barriers to order
> >> the loads, not OrderAccess::acquire(). You should only use acquire
> >> semantics to pair with a corresponding release operation.
> >
> > I agree, but it's unlikely to matter in practice.
> 
> In terms of what underlying hardware barriers get used, no it won't
> (likely) matter in practice.
> 
> But from a code understandability perspective it matters very much IMHO.
> We have been actively trying to ensure that the right OrderAccess APIs are
> used, in the right way and only where actually needed. An acquire without a
> corresponding release shows a lack of understanding and leads to confusion
> for other developers. If you need ordered loads then use a
> loadload() barrier. If the loads need to be ordered you need to ensure there
> are not corresponding writes that also need to be ordered - which may show
> where release() is missing.
> 
> > Having said that, I'm strongly of the opinion that if you see a naked
> > StoreStore it may well be a bug, or at least you've got something very
> > hard to analyse. I know of a few cases (e.g. zeroing an object) where
> > this isn't true.
> >
> > https://www.hboehm.info/c++mm/no_write_fences.html, etc.
> >
> > But that's an argument for another day.
> 
> Indeed :)
> 
> Cheers,
> David