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

[Yangfei (Felix)]

Hi,

    Thanks for reviewing this and the great comments.
    I have put fix-request labels and accompanying comments on this issue for these three repos.

Felix

> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Friday, July 17, 2020 12:52 PM
> To: Yangfei (Felix) <felix.yang at huawei.com>; Andrew Haley
> <aph at redhat.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
> 
> Hi Felix,
> 
> On 17/07/2020 1:05 pm, Yangfei (Felix) wrote:
> > 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?
> 
> It certainly looks better to me. (This code still puzzles me somewhat but I'm
> not going to expend more time trying to understand it more broadly.)
> 
> Thanks,
> David
> -----
> 
> > 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
> >