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

[Thomas Stüfe]

Hi Jiangli,

http://cr.openjdk.java.net/~jiangli/8225035/webrev.03/make/test/JtregNativeHotspot.gmk.udiff.html

Do we really need -lpthread?

http://cr.openjdk.java.net/~jiangli/8225035/webrev.03/src/hotspot/os/linux/os_linux.cpp.udiff.html

+    tls_size = minstack_size - os::vm_page_size() - PTHREAD_STACK_MIN;
+    assert(tls_size > 0, "unexpected size");

tls_size is size_t is unsigned, so I do not think this works as intended.
You may need to do something like:

if (tls_size > os::vm_page_size() + PTHREAD_STACK_MIN) {
   tls_size -= os::vm_page_size() - PTHREAD_STACK_MIN;
}

--

+  // Configure glibc guard page.
+  pthread_attr_setguardsize(&attr, guard_size);

Can you add a "must happen before calling get_static_tls_area_size()" to
the comment ?

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

All the rest looks good. Thank you for the comments, they are well written.

Cheers, Thomas

On Fri, Jun 28, 2019 at 5:58 AM Jiangli Zhou <jianglizhou at google.com> wrote:

> Updated webrev:
> http://cr.openjdk.java.net/~jiangli/8225035/webrev.03/
>
> Thanks for everyone's contribution on carving out the current workaround!
>
> Best regards,
> Jiangli
>
> On Thu, Jun 27, 2019 at 11:42 AM Jiangli Zhou <jianglizhou at google.com>
> wrote:
> >
> > Thank you Thomas and David! Glad to see that we are converging on an
> > acceptable approach here. I'll try to factor in all the latest inputs
> > from everyone and send out a new update.
> >
> > Thanks and best regards,
> > Jiangli
> >
> > On Thu, Jun 27, 2019 at 11:13 AM David Holmes <david.holmes at oracle.com>
> wrote:
> > >
> > > Trimming ....
> > >
> > > On 27/06/2019 12:35 pm, Jiangli Zhou wrote:
> > > > On Thu, Jun 27, 2019 at 9:23 AM Florian Weimer <fweimer at redhat.com>
> wrote:
> > > >> I think you can handle the guard size in this way:
> > > >>
> > > >>    pthread_attr_setguardsize(&attr, guard_size);
> > > >>
> > > >>    size_t stack_adjust_size = 0;
> > > >>    if (AdjustStackSizeForTLS) {
> > > >>      size_t minstack_size = get_minstack(&attr);
> > > >>      size_t tls_size = minstack_size - vm_page_size() -
> PTHREAD_STACK_MIN;
> > > >>      // In glibc before 2.27, tls_size still includes guard_size.
> > > >>      // In glibc 2.27 and later, guard_size is automatically
> > > >>      // added to the stack size by pthread_create.
> > > >>      // In both cases, the guard size is taken into account.
> > > >>      stack_adjust_size += tls_size;
> > > >>    } else {
> > > >>      stack_adjust_size += guard_size;
> > > >>    }
> > > >
> > > > Is the vm_page_size() counted for the dl_pagesize? As long as others
> > > > are okay with the above suggested adjustment, it looks good to me.
> > > > Thomas, David and others, any objection?
> > >
> > > I find the above acceptable. I've been waiting for the dust to settle.
> > >
> > > Thanks,
> > > David
> > >
> > > > Thanks and best regards,
> > > > Jiangli
> > > >>
> > > >> Thanks,
> > > >> Florian
>