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

[David Holmes]

On Wed, 18 Oct 2023 14:58:15 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:
> 
>   some pd_xxx calls were missed.

I don't see how this fixes the problem raised in the associated JBS issue - the gtest will still fail but now we will see why. If this was an enabler to see why the test failed, it should probably be done under a different bug id (e.g. a subtask of 8280056).

That said, I really do not like polluting the VM code with this `fail_reason` just to allow a gtest to be more informative. Perhaps there should be logging at the lower-level instead?

Thanks

src/hotspot/share/runtime/os.hpp line 200:

> 198:   static char*  pd_reserve_memory(size_t bytes, bool executable);
> 199: 
> 200:   static char*  pd_attempt_reserve_memory_at(char* addr, size_t bytes, bool executable, int *fail_reason);

If `fail_reason` defaults to `nullptr` you would save making a few changes above.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16240#pullrequestreview-1686492987
PR Review Comment: https://git.openjdk.org/jdk/pull/16240#discussion_r1364767111