RFR: 8324781: runtime/Thread/TestAlwaysPreTouchStacks.java failed with Expected a higher ratio between stack committed and reserved [v9]

[Thomas Stuefe]

On Fri, 14 Jun 2024 04:16:08 GMT, Liming Liu <duke at openjdk.org> wrote:

>> Meanwhile, I am warming to the current approach. I understand that this it avoids referring to individual downstream vendors, which I agree may be brittle.
>> 
>> My main concern is to prevent future flag mismatches. Therefore, my proposal is to do what this patch does, but in a more generic way. Essentially, encoding that for certain flags, we cannot rely on older kernel correctly ignoring them. But we assume that downstream kernel vendors will at least fix conflicts when they merge in flags from mainline. We sacrifice the ability to benefit from vendor-specific backports, but that is the compromise.
>> 
>> The flags I'd like to guard for now are:
>> 1) UEK7: MADV_DONTNEED_LOCKED   -> MADV_DOEXEC
>> 2) UEK7: MADV_COLLAPSE          -> MADV_DONTEXEC
>> 3) UEK6: MADV_POPULATE_READ     -> MADV_DOEXEC
>> 4) UEK6: MADV_POPULATE_WRITE    -> MADV_DONTEXEC
>> 
>> If the vendor keeps up its routine of just shifting the proprietary flags to the end of the numerical MADV range for each new mainline flag, we will continue to have problems and this list may grow.
>> 
>> The mechanism could be very close to what @limingliu-ampere does now, only a tad more generic. E.g.:
>> 
>> 
>> bool os::Linux::can_use_madvise_flag(int someflag) { 
>>   // have a hardcoded array of { flag, kernel version } tupels. 
>>   // Search it for someflag, and if found, return false if host kernel version is older than the encoded version. 
>>   // Otherwise return true.
>> }
>> 
>> 
>> and then maybe wrap the madvise call with something like this:
>> 
>> 
>> bool os::Linux::checked_madvise(..., someflag) {
>>   assert(can_use_madvise_flag(someflag))
>>   call real madvise
>> }
>> 
>> 
>> in addition to something like this in initialization:
>> 
>> 
>> if (UseMadvPopulateWrite && ! can_use_madvise_flag(MADV_POPULATE_WRITE)) {
>>    FLAG_SET_ERGO(UseMadvPopulateWrite, false);
>> }
>> 
>> 
>> Do you like this, does this make sense?
>
> Hi, @tstuefe. Could you please take a look? The patch had been limited to testcases, as there were already fixes in UEK and you created a ticket to cover pretouch.

> I don't think @limingliu-ampere has enough rights to backport this fix. @tstuefe is this something that should be backported to JDK 23?

Backporting this is fine and low risk

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

PR Comment: https://git.openjdk.org/jdk/pull/18592#issuecomment-2186375816