RFR: 8315923: pretouch_memory by atomic-add-0 fragments huge pages unexpectedly [v21]

[Thomas Stuefe]

On Fri, 12 Jan 2024 06:30:03 GMT, Liming Liu <duke at openjdk.org> wrote:

>> src/hotspot/share/runtime/os.cpp line 2125:
>> 
>>> 2123:   for (char* cur = static_cast<char*>(first); /* break */; cur += page_size) {
>>> 2124:     Atomic::add(reinterpret_cast<int*>(cur), 0, memory_order_relaxed);
>>> 2125:     if (cur >= last) break;
>> 
>> I suggest a slightly different flow, similar to how we do things in other areas:
>> 
>> os.hpp
>> 
>> 
>> private:
>> bool pd_pretouch_memory(..);
>> public:
>> void pretouch_memory(..);
>> 
>> 
>> os.cpp
>> 
>> void os::pretouch_memory(..) {
>>   // Ask platform first
>>   if (pd_pretouch_memory(..)) {
>>     return;
>>   }
>>   ... do pretouching by touching
>> }
>> 
>> 
>> then provide a pd_pretouch for every platform; let other platforms be just a noop returning false, on Linux - if THPs are enabled and so forth, do the madvise and return true.
>> 
>> One function less in the os namespace, and we don't call back from a pd_... function into a generic function which is unusual.
>
> Linux requires a different page size for touching when THP is enabled. Should I change page_size to a reference or let pd_pretouch_memory return the page size for touching rather than a false?

Oh, you are right. Yes, that makes sense, return the pagesize as "touching pagesize overrides huge page size". Alternatively, return a boolean that says "touch with system page size, not with the assumed large page size".

Up to you. Also up to you if you do this as a separate return value (as in `bool os::pd_pretouch_memory(.., .., bool& use_system_page_size);` or whether you do this via return value, e.g. as in "returns 0 if pretouching was done, >0 if not done, in which case its the page size to use". (While writing this, I feel that a separate parameter is cleaner, but up to you).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15781#discussion_r1450153626