RFR: 8245203/8245204/8245208: ZGC: Don't hold the ZPageAllocator lock while committing/uncommitting memory

[Stefan Karlsson]

On 2020-05-19 14:37, Per Liden wrote:
> On 5/19/20 11:10 AM, Stefan Karlsson wrote:
>> On 2020-05-18 23:23, Per Liden wrote:
>>> Please review this series of three patches to rework the page 
>>> allocation path so that we don't hold the ZPageAllocator lock while 
>>> committing/uncommitting memory. Patch 1 & 2 are small and 
>>> preparatory. Patch 3 is the main patch and it's unfortunately fairly 
>>> large as it was hard to break up in a sensible way.
>>>
>>> ---------------------
>>>
>>> 1) 8245203: ZGC: Don't track size in ZPhysicalMemoryBacking
>>>
>>> To make it easier to allow for more concurrent commit/uncommit we 
>>> should not track the backing "size" in ZPhysicalMemoryBacking. 
>>> Instead we can add the full physical memory range (from 0 to 
>>> max_capacity) to the list of uncommitted memory at startup. This 
>>> removes the need to track and update the backing size (which 
>>> otherwise needs to be properly synchronized).
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8245203
>>> Webrev: http://cr.openjdk.java.net/~pliden/8245203/webrev.0
>>
>> Most of this looks good. You removed the EINTR while loop from the 
>> ftruncate call. Is that not needed anymore?
>
> That was an oversight on my part. Added it back.
>
> diff --git 
> a/src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp 
> b/src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp
> --- a/src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp
> +++ b/src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp
> @@ -126,10 +126,12 @@
>    }
>
>    // Truncate backing file
> -  if (ftruncate(_fd, max_capacity) == -1) {
> -    ZErrno err;
> -    log_error(gc)("Failed to truncate backing file (%s)", 
> err.to_string());
> -    return;
> +  while (ftruncate(_fd, max_capacity) == -1) {
> +    if (errno != EINTR) {
> +      ZErrno err;
> +      log_error(gc)("Failed to truncate backing file (%s)", 
> err.to_string());
> +      return;
> +    }
>    }
>
>    // Get filesystem statistics
>
>
> Thanks for reviewing!

Looks good.

StefanK

>
> cheers,
> Per