RFR: 8262291: Refactor reserve_memory_special_huge_tlbfs [v4]

Stefan Johansson sjohanss at openjdk.java.net
Fri Mar 26 11:07:30 UTC 2021


On Thu, 25 Mar 2021 09:00:37 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Stefan Johansson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Self review.
>>   
>>   Update helper name to better match commit_memory_special().
>
> src/hotspot/os/linux/os_linux.cpp line 3932:
> 
>> 3930:                                        size_t page_size,
>> 3931:                                        char* req_addr,
>> 3932:                                        bool exec) {
> 
> I'd prefer if this were file scope static and not exported (don't think this needs anything from the os::Linux namespace, or?).
> 
> Also, mid term we could probably merge this with commit_memory. AFAICS the only differences to the latter is that this can do huge pages, and the error handling is a bit different.
> 
> I am also not sure the return type makes a lot of sense. Either we handle commit errors inside this function, then we should return void. Or, we return a boolean and handle it in the caller.
> 
> Long term I would prefer to handle allocation errors not by aborting but by leaving it up to the caller what to do. Because it makes sense to have the option to "reserve if we get large pages, otherwise just use small pages". Eg this I would like to do in a future Metaspace.

That was my first plan as well, but it uses `os::Linux::hugetlbfs_page_size_flag()` which in turn depends on `os::Linux::default_large_page_size()`. So I went with this. 

I agree that we long term should look at using/merging `commit_memoy()` but didn't want to change to much functionality at once here. What we get now is very much a like what we got before but all large pages at the beginning of the mapping. 

I also agree that now when this is really just a commit function, returning the address is a bit strange. I don't see a good way to handle potential failures in here so I opted for returning a `bool`.

In some sense it is left to the caller (`ReservedSpace`) right now. If using large pages fail, we will try using small pages instead. But not sure if you would want this logic to move to another layer.

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

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


More information about the hotspot-dev mailing list