ReE: [11u] RFR (XS) 8259271: gc/parallel/TestDynShrinkHeap.java still fails "assert(covered_region.contains(new_memregion)) failed: new region is not in covered_region"

[Aleksey Shipilev]

Thanks, tagged.

On 2/16/21 2:12 AM, Hohensee, Paul wrote:
> Lgtm.
> 
> Thanks,
> Paul
> 
> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-retn at openjdk.java.net> on behalf of Aleksey Shipilev <shade at redhat.com>
> Date: Monday, February 15, 2021 at 10:47 AM
> To: "jdk-updates-dev at openjdk.java.net" <jdk-updates-dev at openjdk.java.net>
> Subject: [11u] RFR (XS) 8259271: gc/parallel/TestDynShrinkHeap.java still fails "assert(covered_region.contains(new_memregion)) failed: new region is not in covered_region"
> 
> Original bug:
>     https://bugs.openjdk.java.net/browse/JDK-8259271
>     https://github.com/openjdk/jdk16/commit/685c03dc
> 
> Like with JDK-8260704 backport, we do not have Atomic methods yet, those were moved from
> OrderAccess to Atomic in JDK 14 with JDK-8234562. So, 11u patch uses OrderAccess. This backport
> would land after JDK-8260704, so no additional #include is needed.
> 
> 11u variant:
> 
> diff -r 48895ff9213d src/hotspot/share/gc/parallel/mutableSpace.cpp
> --- a/src/hotspot/share/gc/parallel/mutableSpace.cpp    Mon Feb 15 18:17:01 2021 +0000
> +++ b/src/hotspot/share/gc/parallel/mutableSpace.cpp    Mon Feb 15 19:46:06 2021 +0100
> @@ -193,11 +193,15 @@
>    }
> 
>    // This version is lock-free.
>    HeapWord* MutableSpace::cas_allocate(size_t size) {
>      do {
> -    HeapWord* obj = top();
> +    // Read top before end, else the range check may pass when it shouldn't.
> +    // If end is read first, other threads may advance end and top such that
> +    // current top > old end and current top + size > current end.  Then
> +    // pointer_delta underflows, allowing installation of top > current end.
> +    HeapWord* obj = OrderAccess::load_acquire(top_addr());
>        if (pointer_delta(end(), obj) >= size) {
>          HeapWord* new_top = obj + size;
>          HeapWord* result = Atomic::cmpxchg(new_top, top_addr(), obj);
>          // result can be one of two:
>          //  the old top value: the exchange succeeded
> 
> 
> Testing: tier{1,2} with Parallel
> 
> --
> Thanks,
> -Aleksey
> 
> 


-- 
Thanks,
-Aleksey