RFR: 8225035: Thread stack size issue caused by large TLS size

[Thomas Stüfe]

On Thu, Jun 27, 2019 at 5:05 PM Jiangli Zhou <jianglizhou at google.com> wrote:

> On Thu, Jun 27, 2019 at 12:57 AM Florian Weimer <fweimer at redhat.com>
> wrote:
> >
> > * Thomas Stüfe:
> >
> > >> I think
> > >>
> > >>   stack_adjust_size += guard_size;
> > >>
> > >> is redundant with the get_minstack-based adjustment because it is
> either
> > >> included in it directly in the get_minstack value, or indirectly in
> the
> > >> updated stack size computation in glibc (where it adds the guard size
> > >> internally).
> > >
> > > Please do not change this!
> > >
> > > To me and I guess many others it is very important that nothing at all
> > > changes if this workaround is off.
> >
> > That has already happened because recent glibc adds the guard size to
> > the stack size internally now.
> >
> > But what I meant: only perform the += adjustment above for the
> > !AdjustStackSizeForTLS case.
>
> I'm reluctant to do that, because it adds additional dependencies on
> the glibc implementation. Subtracting the non-TLS size from the value
> returned by __pthread_get_minstack may not be forward compatible in
> the future when __pthread_get_minstack changes. I agree with Thomas to
> keep the workaround as simple as possible.
>
> I think it is best to move forward with using __pthread_get_minstack
> as is without binding too tightly to it's currently implementation
> (subtracting the non-TLS values the function includes).
>
>
I agree. Lets keep assumptions at a minimum.

I see it like this: __pthread_get_minstack() is a mystery function which
may or may not exist; if it exists it returns a size which we assume is
large enough to hold the TLS areas, but maybe a bit larger, and we live
with the "a bit larger". On sensible OSes this will only affect virtual
size, not the working set. And since this is all opt-in existing
installations will not be bothered.

For the record, I still would have preferred a simple "MinStackSize=xx"
switch over the __pthread_get_minstack() solution. But I keep out for now,
this mail thread is massive enough :), and wait what the consensus turns
out to be.

Cheers, Thomas



> Best regards,
> Jiangli
>
> >
> > >> I suspect this new flag is is working well because it effectively
> > >> adds 20 KiB to all requested stack sizes (page size plus
> > >> PTHREAD_MIN_STACK).  The static TLS size adjustment may not matter
> > >> at all.  On older glibcs, the number is even higher because your
> > >> code adds the guard size twice (once in __pthread_get_minstack, once
> > >> in the Hotspot code).
> > >
> > > Which, again, would be an argument for a much simpler
> > > "-XX:MinStackSize" option...
> >
> > Indeed.
> >
> > Thanks,
> > Florian
>