RFR: 8244752: Enable Linux support for multiple huge page sizes -XX:LargePageSizeInBytes

[Ivan Walulya]

Thanks Kim,

Suggested modifications have been made in a new webrev:

https://cr.openjdk.java.net/~iwalulya/8244752/01/

//Ivan

> On 12 May 2020, at 13:39, Kim Barrett <kim.barrett at oracle.com> wrote:
> 
>> On May 11, 2020, at 12:46 PM, Ivan Walulya <ivan.walulya at oracle.com> wrote:
>> 
>> Hi all,
>> 
>> Please review this enhancement to enable selecting a desired huge page size on Linux systems that support multiple huge page sizes. 
>> 
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244752
>> Webrev: http://cr.openjdk.java.net/~iwalulya/8244752/00/
>> Testing: Tier 1 - 3
>> 
>> 
>> //Ivan
> 
> ------------------------------------------------------------------------------
> src/hotspot/os/linux/os_linux.cpp  
> 3819         sscanf(entry->d_name, "hugepages-%zukB", &page_size) ) {
> 
> Implicit boolean.
> 
> ------------------------------------------------------------------------------
> src/hotspot/os/linux/os_linux.cpp 
> 3845   if (!FLAG_IS_DEFAULT(LargePageSizeInBytes) && LargePageSizeInBytes != _large_page_size ) {
> 3846      if (is_valid_large_page_size(LargePageSizeInBytes)) {
> 
> Inner if appears to be indented 3 spaces rather than the usual 2.
> 
> ------------------------------------------------------------------------------
> src/hotspot/os/linux/os_linux.cpp 
> 3799 void os::Linux::find_large_page_sizes() {
> 
> I think that's not a great name for what this function is doing, which
> is collecting large page sizes in _page_sizes[].  However, I think
> there are other problems here.
> 
> The collection of sizes into _page_sizes[] doesn't conform to the
> description of _page_sizes[], which says the array is sorted in
> decreasing order.  Here it's whatever order the directory iteration
> happens upon them.
> 
> I think that the pre-existing code at the end of setup_large_page_size
> will fix it up on any reasonable configuration, but I'm not certain of
> that and I'm not certain all systems are configured reasonably.
> 
> find_large_page_sizes is only called from is_valid_large_page_size,
> which is only called by setup_large_page_size.  It seems like it would
> be more straightforward to hoist the iteration over /sys/kernel/mm/hugepages
> into is_valid_large_page_size and not touch _page_sizes at all there.
> 
> ------------------------------------------------------------------------------
> src/hotspot/os/linux/os_linux.cpp  
> 4097      flags |= (os::large_page_size() > (1 << (MAP_HUGE_2MB >> MAP_HUGE_SHIFT)))
> 4098                 ? MAP_HUGE_1GB
> 4099                 : MAP_HUGE_2MB;
> 
> Shouldn't this just be:
> 
>  flags |= (os::large_page_size() > (2 * M)) ? MAP_HUGE_1G : MAP_HUGE_2MB;
> 
> (Though there's also Thomas's comments to account for.)
> 
> There's no particular reason here to think the encoding used for
> MAP_HUGE_2MB has any direct relation to a size in bytes.
> 
> Similarly here:
> 4173      flags |= (os::large_page_size() > (1 << (MAP_HUGE_2MB >> MAP_HUGE_SHIFT)))
> 4174                 ? MAP_HUGE_1GB
> 4175                 : MAP_HUGE_2MB;
> 
> ------------------------------------------------------------------------------
> src/hotspot/os/linux/os_linux.cpp 
> 4096   if (!FLAG_IS_DEFAULT(LargePageSizeInBytes)) {
> 
> Why is this checking for non-default value for the option? It seems
> like this might do the wrong thing if an explicit command line option
> was “rejected" because it had an unsupported value.
> 
> Similarly here:
> 4172   if (!FLAG_IS_DEFAULT(LargePageSizeInBytes)) {
> 
> ------------------------------------------------------------------------------
>