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