RFR: 8273872: ZGC: Explicitly use 2M large pages [v2]

Per Liden pliden at openjdk.java.net
Thu Sep 16 09:41:31 UTC 2021


On Thu, 16 Sep 2021 08:15:08 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Per Liden has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Review comments
>
> src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp line 210:
> 
>> 208: 
>> 209:   // Create file
>> 210:   const int extra_flags = ZLargePages::is_explicit() ? (MFD_HUGETLB | MFD_HUGE_2MB) : 0;
> 
> Potentially the use of the constant `MFD_HUGE_2MB` could be generalized a little and calculated from a required page size like we do in `Linux::commit_memory_special` via a helper function like `os::Linux::hugetlbfs_page_size_flag`; at least that extra flag just looks like it is actually generated the same way. Looking through `memfd.h` it is *exactly* the same as for the corresponding `HUGETLB_*` flags.
> But since this is really ZGC specific code, up to you.

I don't think we should be passing `MAP_*` flags to `memfd_create()` as there's no guarantee that `MAP_HUGE_2MB` and `MFD_HUGE_2MB` are the same values. It's more of a lucky/convenient implementation detail that they happen to be the same.

> src/hotspot/os/linux/gc/z/zPhysicalMemoryBacking_linux.cpp line 216:
> 
>> 214:     log_debug_p(gc, init)("Failed to create memfd file (%s)",
>> 215:                           (ZLargePages::is_explicit() && (err == EINVAL || err == ENODEV)) ?
>> 216:                           "Hugepages (2M) not supported" : err.to_string());
> 
> Maybe this should be something like:
> 
> Suggestion:
> 
>                           "Hugepages (2M) not available" : err.to_string());
> 
> 
> As in `ZArguments` the code already checks that 2M page size are requested (if any).

Fixed

> src/hotspot/share/gc/z/zArguments.cpp line 75:
> 
>> 73:   }
>> 74: 
>> 75:   // Only 2M large pages is supported
> 
> Suggestion:
> 
>   // Only 2M large pages are supported.

Fixed

> src/hotspot/share/gc/z/zArguments.cpp line 77:
> 
>> 75:   // Only 2M large pages is supported
>> 76:   if (!FLAG_IS_DEFAULT(LargePageSizeInBytes) && LargePageSizeInBytes != 2 * M) {
>> 77:     vm_exit_during_initialization("Invalid -XX:LargePageSizeInBytes (only 2M large pages is supported)");
> 
> Suggestion:
> 
>     vm_exit_during_initialization("Invalid -XX:LargePageSizeInBytes (only 2M large pages are supported)");

Fixed

-------------

PR: https://git.openjdk.java.net/jdk/pull/5541


More information about the hotspot-dev mailing list