RFR: 8229147: Linux os::create_thread() overcounts guardpage size with newer glibc (>=2.27)

[Aleksey Shipilev]

On Fri, 21 Apr 2023 13:53:28 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> We can now detect whether glibc includes the guard pages as part of the requested stack size or not, and so only need to make adjustments when glibc requires it.
>> 
>> The intent was to use a local variable as the "flag" but unfortunately it is also needed in os_posix.cpp so I had to make it part of the os::Linux API.
>> 
>> See bug report (and related) for details.
>> 
>> Testing:
>>   - Manually checked log output for stack sizes and boundaries on systems with and without the glibc fix. (Again see JBS issue)
>>   -  Tiers 1-3 sanity
>> Thanks
>
> src/hotspot/os/linux/os_linux.cpp line 837:
> 
>> 835:     pthread_attr_init(&attr);
>> 836:     size_t min_stack = _get_minstack_func(&attr);
>> 837:     pthread_attr_setguardsize(&attr, 16*K );
> 
> Suggestion:
> 
>     pthread_attr_setguardsize(&attr, 16*K);

Does this work on arches with 64K pages? Perhaps we need to check the return value of this call to make sure we were successful in setting the guard size to begin with. Otherwise we can "accidentally" discover the min_stack is not changing?

> src/hotspot/os/linux/os_linux.hpp line 152:
> 
>> 150:   static size_t default_guard_size(os::ThreadType thr_type);
>> 151: 
>> 152:   static bool AdjustStackSizeForGuardPages; // See comments in os_linux.cpp
> 
> With the capitalization like that, this looks like a global flag. Other statics in this file have different style. Suggestion: `_adjust_stacksize_for_guard_pages`.

Can we also assert the lifecycle for this flag is correct? E.g.:


private:
 static bool _init_adjust_...; // Maybe simpler name?
 static bool _adjust_...;

 static void init_...() {
   ...
   _init_adjust_... = true;
 }

public:
 static bool adjust_stacksize_for_guard_pages() {
   assert(_init_adjust..., "Should be initialized");
   return _adjust_...;
 }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173808251
PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173795679