RFR: 8261401: Add sanity check for UseSHM large pages similar to the one used with hugetlb large pages [v5]

Stefan Johansson sjohanss at openjdk.java.net
Wed Feb 17 10:56:43 UTC 2021


On Wed, 17 Feb 2021 10:14:18 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Stefan Johansson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Clean up test after merge
>
> src/hotspot/os/linux/os_linux.cpp line 3570:
> 
>> 3568:   // Try to create a large shared memory segment.
>> 3569:   int shmid = shmget(IPC_PRIVATE, page_size, SHM_HUGETLB|IPC_CREAT|SHM_R|SHM_W);
>> 3570:   if (shmid == -1) {
> 
> I think the flags should contain the `SHM_HUGE_*` flags (considering large pages) similar to hugetlbfs for proper checking to avoid the failure like in [JDK-8261636](https://bugs.openjdk.java.net/browse/JDK-8261636) reported just recently for the same reason.
> 
> According to the [man pages](https://man7.org/linux/man-pages/man2/shmget.2.html)] such flags exist.

I would agree if the later usage of `shmget()` would make use of those flags but it don't. We might want o add a CR for enabling use of other page sizes than the default large page size for SHM as well. But there is also discussions about trying to remove this SHM support.

> src/hotspot/os/linux/os_linux.cpp line 3574:
> 
>> 3572:     // 1. shmmax is too small for the request.
>> 3573:     //    > check shmmax value: cat /proc/sys/kernel/shmmax
>> 3574:     //    > increase shmmax value: echo "0xffffffff" > /proc/sys/kernel/shmmax
> 
> I'd avoid using a 32 bit constant on potentially 64 bit systems. According to that same man page, the limit on 64 bits is 127 TB which is more than the suggested 4 GB :)
> 
> The man page talks about `ULONG_MAX - 2^24` as default (for 64 bit systems?).

The comment is a copy of the old one where we actually use `shmget()`, but I agree that it could use an update. Not even sure we have to suggest a specific value, something like:
Suggestion:

    //    > increase shmmax value: echo "new value" > /proc/sys/kernel/shmmax
Might be good enough. 

I will also update the original comment.

> src/hotspot/os/linux/os_linux.cpp line 3578:
> 
>> 3576:     //    > check available large pages: cat /proc/meminfo
>> 3577:     //    > increase amount of large pages:
>> 3578:     //          echo new_value > /proc/sys/vm/nr_hugepages
> 
> This is just my opinion, but I would prefer to refer to some generic documentation about these options (and use the "new" locations in `/sys/kernel/mm/hugepages/hugepages-*/` instead of the "legacy" /proc/sys, e.g. `https://lwn.net/Articles/376606/` - yes this is not official documentation, but the best I could find on the spot)

I agree, the documentation I usually refer to is:
https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt

Since our SHM-support currently only handles the default large page size I think we should suggest to use:
`sysctl -w vm.nr_hugepages=new_value`

Suggestion:

    //          sysctl -w vm.nr_hugepages=new_value
    //    > For more information regarding large pages please refer to:
    //      https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt

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

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


More information about the hotspot-dev mailing list