RFR: 8235982: AArch64: gc/stress/jfr/TestStressAllocationGCEventsWithParallel.java assertion failure
thomas.schatzl at oracle.com
Tue Dec 17 14:47:38 UTC 2019
On 17.12.19 14:53, Stefan Johansson wrote:
> Hi Nick,
> Thanks for fixing this issue.
> On 2019-12-17 07:22, Nick Gasson wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8235982
>> Webrev: http://cr.openjdk.java.net/~ngasson/8235982/webrev.01/
>> The above test has been intermittently failing with an assertion error
>> on AArch64 since the changes in "8220465: Use shadow regions for
>> faster ParallelGC full GCs".
>> PSParallelCompact::decrement_destination_counts assumes that
>> shadow_region() is valid if mark_copied() succeeded (i.e.
>> _shadow_state was FilledShadow before).
>> MoveAndUpdateShadowClosure::complete_region sets these with the
>> following code:
>> // Record the shadow region index
>> // Mark the shadow region as filled to indicate the data is ready
>> to be
>> // copied back
>> set_shadow_region() does an ordinary store to a volatile variable and
>> mark_filled() uses a CAS to set _shadow_state to FilledShadow with
>> relaxed memory ordering. On a non-TSO architecture like AArch64 these
>> two stores may be reordered when observed from another thread.
>> Fix by making the CAS to set _shadow_state use the default
>> conservative memory order which inserts a full barrier (see the
>> discussion on the JBS entry).
> I agree with the conclusions in the JBS entry and this fix looks good,
Looks good. If you need a sponsor to push the change I could do that.
More information about the hotspot-gc-dev