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

[Thomas Stüfe]

Hi Igor,

thanks for doing this! Some comments:

---

I am a bit confused about the various notions of page size  we now have.

We have os::vm_page_size(), os::large_page_size(), os::page_sizes[], now
Linux::default_page_size and the notion of a set of valid huge page sizes
not explicitly encoded (but maybe we should?)

How does this map to each other? Should os::page_sizes[] not contain the
non-default valid huge page sizes too?

---

2952 // If MAP_HUGETLB is set, and the system supports multiple huge page
sizes,
2953 // flag bits [26:31] can be used to encode the log2 of the desired
huge page size.
2954 // Otherwise the system's default huge page size will be used.
2955 // See mmap(2) man page for more info (since Linux 3.8).
2956 // https://lwn.net/Articles/533499/
2957 #ifndef MAP_HUGE_SHIFT
2958   #define MAP_HUGE_SHIFT 26
2959 #endif

I may be slow but I needed a while to understand this comment; as a
standalone comment it is confusing. What flags? Could you clarify the
comment a bit, especially making clear that "flags" means mmap flags to be
used in conjunction with MAP_HUGE_TLB?

---

bool os::Linux::is_valid_large_page_size(size_t large_page_size)

Do we have to read /sys/kernel/mm/hugepages each time? Note that
os::Linux::is_valid_large_page_size() sounds rather inconspicuous; as a
caller, without looking at the implementation, I would not have assumed it
does file IO. Could we not just cache the valid set somewhere? Again,
should this set not be part of os::_page_sizes[] ?

Sorry for my confusion,

Thanks, Thomas



On Wed, May 13, 2020 at 3:56 PM Ivan Walulya <ivan.walulya at oracle.com>
wrote:

> Thanks Thomas
>
> Please find new webrev:
>
> https://cr.openjdk.java.net/~iwalulya/8244752/01/
>
>
>
> > On 12 May 2020, at 10:50, Thomas Schatzl <thomas.schatzl at oracle.com>
> wrote:
> >
> > Hi,
> >
> > On 11.05.20 18:46, Ivan Walulya 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
> >
> > CI does not have large page enabled machines at the moment. What local
> testing did you perform?
>
> Local tests with both 1Gb and 2Mb (Linux 2.6.32 and 4.15.0), and also run
> specjbb2015 with largepages. Note: all done on x86.
>
> >
> >> //Ivan
> >
> > - in the comment with the MAP_HUGE* flags, please add the information
> that you got this from the mmap(2) man page, and that this is since Linux
> 3.8. (I understand it's "obvious", but just to make it clear)
>
> Noted
>
> >
> > - what happens when run on < Linux 3.8 and the flags are used? (not sure
> Hotspot minimal requirements _are_ Linux 3.8+, at least for hugetlbfs
> support).
> Flags would have no effect and with the new modifications, they wouldn’t
> be set if not supported
>
> >
> > - os_linux.cpp:3846: "if(entry->d_type == DT_DIR" missing space after
> the "if". At the end of the condition there is an unusual extra space.
>
> Noted
>
> >
> > - in os::Linux::reserve_memory_special_huge_tlbfs_only, are you sure
> that on non-x64 (e.g. arm64) the only supported page sizes for hugetlbfs
> are 2M and 1GB?
> > From what I understand, on e.g. arm64 this is only the case with 4k
> "small" page size. I do not know if that is different with e.g. 64k pages
> (as in: if you have 16k/64k pages, then the hugetlbfs page sizes change
> too).
> >
> > Looking at https://wiki.debian.org/Hugepages, in the arm64 "Multiple
> huge page size support" section, it mentions:
> >
> > "If one elects to build their own Debian arm64 kernel with
> CONFIG_ARM64_64K_PAGES=y, then only 512MB HugeTLB (and THP) pages are
> available. These are available at run time. "
> >
> > So I recommend calculating the flag value based on the
> LargePageSizeInBytes value, not strictly using the constants (which then
> can be removed). Still there is some concern that on Linux < 3.8 this won't
> work (or fail).
>
> True, this has been changed.
>
> >
> > Thanks,
> >  Thomas
>
> //Ivan
>
>