RFR: 8262291: Refactor reserve_memory_special_huge_tlbfs [v2]

[Marcus G K Williams]

On Wed, 24 Mar 2021 13:10:03 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Please review this refactoring of the hugetlbfs reservation code.
>> 
>> **Summary**
>> In recent adventures in this area of the code I noticed a strange condition in `reserve_memory_special_huge_tlbfs` where we take the "mixed-mapping" route even if the size doesn't require any small pages to be used:
>>   if (is_aligned(bytes, os::large_page_size()) && alignment <= os::large_page_size()) {
>>     return reserve_memory_special_huge_tlbfs_only(bytes, req_addr, exec);
>>   } else {
>>     return reserve_memory_special_huge_tlbfs_mixed(bytes, alignment, req_addr, exec);
>>   }
>> 
>> The second condition here is needed because if the alignment is larger than the large page size, we needed to enforce this and can't just trust `mmap` to give us a properly aligned address. Doing this by using the mixed-function feels a bit weird and looking a bit more at this I found a way to refactor this function to avoid having the two helpers. 
>> 
>> Instead of only having the mixed path honor the passed down alignment, make sure that is always done. This will also have the side-effect that all large pages in a "mixed"-mapping will be at the start and then we will have a tail of small pages. This actually also ensures that we will use large pages for a mixed mapping, in the past there was a corner case where we could end up with just a head and tail of small pages and no large page in between (if the mapping was smaller than 2 large pages and there was no alignment constraint).
>> 
>> **Testing**
>> Mach5 tier1-3 and a lot of local testing with different large page configurations.
>
> Stefan Johansson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Ivan review
>   
>   Renamed helper to commit_memory_special and updated the comments.

Changes requested by mgkwill at github.com (no known OpenJDK username).

src/hotspot/os/linux/os_linux.cpp line 3976:

> 3974:   // If the size is not a multiple of the large page size, we
> 3975:   // will mix the type of pages used, but in a decending order.
> 3976:   // Start of by reserving a range of the given size that is

Small typo, `  // Start of by reserving` should be `  // Start off by reserving` or  `  // Start by reserving`

src/hotspot/os/linux/os_linux.cpp line 3987:

> 3985:   }
> 3986: 
> 3987:   // Start of by committing large pages.

Small Typo `  // Start of` should be `  // Start off` or `  // Start by`

src/hotspot/os/linux/os_linux.cpp line 4003:

> 4001:     // Failed to commit large pages, so we need to unmap the
> 4002:     // reminder of the orinal reservation.
> 4003:     ::munmap(small_start, small_size);

I'm assuming that if mmap fails for large pages, it un-maps the reservation area requested for large pages and thus here we only need to munmap for remaining reservation (small pages)?

src/hotspot/os/linux/os_linux.cpp line 4007:

> 4005:   }
> 4006: 
> 4007:   // Commit the reminding bytes using small pages.

`  // Commit the reminding` should be `  // Commit the remaining`

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

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