RFR: 8280056: gtest/LargePageGtests.java#use-large-pages failed "os.release_one_mapping_multi_commits_vm" [v4]

[Thomas Stuefe]

On Wed, 25 Oct 2023 13:38:04 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> The `attempt_allocate_memory_at(addr, size)` may fail for some reasons that `::mmap`  will report. To find out why an attempt failed, an `int *` is passed down deep to the `::mmap` to return the `ERRNO` back to the callers.
>> This error code is used in gTest test case to show a proper message.
>> 
>> The changed test case never failed after 10K+ repetitions, but in case it happens in future the reason is also printed out.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove extra parameter for windows and bsd versions.

Looks good. Thank you for going with my suggestion. This is a tricky test to deal with, and I would understand if you guys throw it away if it keeps making trouble. Or, cut the second part - the re-reservation - down. That would still leave us with the os::release call, and we would at least partly test it.

Lets hope that is it for now. Thanks for your patience.

... Thomas

test/hotspot/gtest/runtime/test_os.cpp line 509:

> 507: 
> 508:   // ...re-reserve the middle stripes. This should work unless release silently failed.
> 509:   address p2 = (address)os::attempt_reserve_memory_at((char*)p_middle_stripes, middle_stripe_len, false);

small nit, unnecessary, since it is the default, but up to you.

test/hotspot/gtest/runtime/test_os.cpp line 557:

> 555:   //  committed and uncommitted regions:
> 556:   const size_t stripe_len = os::vm_allocation_granularity();
> 557:   const int num_stripes = 6;

Good, smaller stripe makes the hole less attractive

test/hotspot/gtest/runtime/test_os.cpp line 567:

> 565:   PRINT_MAPPINGS("B");
> 566: 
> 567:   // re-reserve it. This should work unless release failed.

Yes, in hindsight this comment is obviously wrong :(

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

Marked as reviewed by stuefe (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16240#pullrequestreview-1699861602
PR Review Comment: https://git.openjdk.org/jdk/pull/16240#discussion_r1373306909
PR Review Comment: https://git.openjdk.org/jdk/pull/16240#discussion_r1373309574
PR Review Comment: https://git.openjdk.org/jdk/pull/16240#discussion_r1373312431