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

Thomas Stuefe stuefe at openjdk.org
Sat Apr 22 14:15:53 UTC 2023


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

>> 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_...;
>  }

I would it prefer if this switch were named like the behaviour we probe for, not the action that results from it. E.g. "_glibc_includes_guards_in_stack_size". Also makes it clearer that this is a glibc-only thing. And I agree with @shipilev that this looks like a flag. I sometimes prefix local globals with `g_`.

But its up to you.

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

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


More information about the hotspot-runtime-dev mailing list