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

[Aleksey Shipilev]

On Fri, 21 Apr 2023 04:57:28 GMT, David Holmes <dholmes 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

I like this, despite the runtime check. AFAIU, this does not affect directly affect RSS, because we don't commit guard pages?

src/hotspot/os/linux/os_linux.cpp line 823:

> 821: 
> 822: // In glibc versions prior to 2.27 the guard size mechanism
> 823: // was not implemented properly. The posix standard requires adding

"POSIX"

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);

src/hotspot/os/linux/os_linux.cpp line 842:

> 840:     // If the minimum stack size changed when we added the guard pages
> 841:     // then we need to perform the adjustment.
> 842:     os::Linux::AdjustStackSizeForGuardPages = (min_stack2 - min_stack > 0);

Subtracting the unsigned `size_t` is somewhat unsafe in face of underflows here. Just `min_stack2 > min_stack` solves it.

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`.

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

PR Review: https://git.openjdk.org/jdk/pull/13571#pullrequestreview-1395806426
PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173797138
PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173797646
PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173799393
PR Review Comment: https://git.openjdk.org/jdk/pull/13571#discussion_r1173791889