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

[Liming Liu]

On Thu, 6 Jun 2024 12:38:33 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Liming Liu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix the wrong condition
>
> 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.

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

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