[11u] RFR 8265677: CMS: CardTableBarrierSet::write_ref_array_work() lacks storestore barrier

[Reingruber, Richard]

> Let's leave it alone since it's not in tip. Maybe someone will look into it
> there. Also, I see you have 11u approval already. :)

Ok, sure.

> I looked at 8u too, but it doesn't have scanned_concurrently() so it could
> only be done in dirty_MemRegion.

Right. I guess it is pretty unlikely but it can crash the vm. More likely if no
arraycopy stubs are generated. We've found it (depressingly ;)) long ago doing ports for
which we used the default copy routines in the runtime.

Thanks, Richard.

-----Original Message-----
From: Hohensee, Paul <hohensee at amazon.com> 
Sent: Donnerstag, 22. April 2021 17:19
To: Reingruber, Richard <richard.reingruber at sap.com>; jdk-updates-dev at openjdk.java.net
Subject: Re: [11u] RFR 8265677: CMS: CardTableBarrierSet::write_ref_array_work() lacks storestore barrier

Let's leave it alone since it's not in tip. Maybe someone will look into it there. Also, I see you have 11u approval already. :)

I looked at 8u too, but it doesn't have scanned_concurrently() so it could only be done in dirty_MemRegion.

Thanks,
Paul

-----Original Message-----
From: "Reingruber, Richard" <richard.reingruber at sap.com>
Date: Thursday, April 22, 2021 at 7:29 AM
To: "Hohensee, Paul" <hohensee at amazon.com>, "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
Subject: RE: [11u] RFR 8265677: CMS: CardTableBarrierSet::write_ref_array_work() lacks storestore barrier

Thanks for reviewing!

> Putting an unconditional storestore barrier in dirty_MemRegion might be a
> better fix since it will catch everything, not just arrays. But, that's not in
> tip, so lgtm.

I thought about this too. My conclusion was that the only additional execution
path that might require a barrier was the following:

CardTable::dirty_MemRegion(MemRegion) : void
CardTable::invalidate(MemRegion) : void
CardGeneration::invalidate_remembered_set() : void
CardTableBarrierSet::invalidate(MemRegion) : void
CardTableBarrierSet::on_slowpath_allocation_exit(JavaThread *, oop) : void
CardTableBarrierSet::write_region(MemRegion) : void
CardTableBarrierSet::flush_deferred_card_mark_barrier(JavaThread *) : void

And then I assumed that the transition _thread_in_Java to _thread_in_vm would do
the necessary ordering. But looking again I think it does not.

So maybe a should just do as you suggested? dirty_MemRegion doesn't seem to be
on any hot execution path.

Thanks, Richard.

-----Original Message-----
From: Hohensee, Paul <hohensee at amazon.com>
Sent: Mittwoch, 21. April 2021 19:25
To: Reingruber, Richard <richard.reingruber at sap.com>; jdk-updates-dev at openjdk.java.net
Subject: RE: [11u] RFR 8265677: CMS: CardTableBarrierSet::write_ref_array_work() lacks storestore barrier

Putting an unconditional storestore barrier in dirty_MemRegion might be a better fix since it will catch everything, not just arrays. But, that's not in tip, so lgtm.

Thanks,
Paul

-----Original Message-----
From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> on behalf of "Reingruber, Richard" <richard.reingruber at sap.com>
Date: Wednesday, April 21, 2021 at 7:32 AM
To: "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
Subject: RE: [11u] RFR 8265677: CMS: CardTableBarrierSet::write_ref_array_work() lacks storestore barrier

// Resend after removing incorrect link

Hi,

please review this XXS fix for 11u which adds a storestore barrier to
`CardTableBarrierSet::write_ref_array_work(MemRegion mr)`. The barrier is only
executed if CMS is selected. Please refer to the bug report for more details.

Bug:
  https://bugs.openjdk.java.net/browse/JDK-8265677

11u webrev:
  http://cr.openjdk.java.net/~rrich/webrevs/8265677_11u_CMS__CardTableBarrierSet__write_ref_array_work___lacks_storestore_barrier/webrev.0/

Testing:

make run-test-tier1 JTREG="VM_OPTIONS=-XX:+UseConcMarkSweepGC"

CI testing @SAP: JCK and JTREG, also in Xcomp mode, SPECjvm2008, SPECjbb2015, Renaissance Suite,
SAP specific tests with fastdebug and release builds on all platforms

Thanks, Richard.