RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS

Fri Feb 21 18:26:12 UTC 2020

Hi Richard,

The latest test update looks fine to me. Thank you for digging deeper into this.

Best regards,

Jiangli

On Fri, Feb 21, 2020 at 3:25 AM Reingruber, Richard
<richard.reingruber at sap.com> wrote:
>
> Hi Jiangli,
>
> > > Actually I would prefer not to skip the test then. I'd like to verify if large data structures can
> > > be placed in TLS without stack size adjustment or if there is some kind of regression. That's the
> > > point of regression tests after all.
> > >
>
> > That's a very valid point. I thought about that as well yesterday. My
> > thinking was that the VM and test would need to be updated to remove
> > the workaround for the TLS issue when no longer needed. Do you know if
> > the proposed check using gnu_get_libc_version() covers all cases? I
>
> Other C libraries (e.g. MUSL) are not really covered, but I'm not sure if you mean this by covered...
>
> > came across the following archive that mentioned missing symbol in
> > eglibc-2.13 (eglibc however is already deprecated).
>
> > https://www.mail-archive.com/lfs-dev@linuxfromscratch.org/msg18115.html
>
> I only knew, that 2.11 does not provide __pthread_get_minstack, but then I used
>
>   https://github.com/bminor/glibc
>
> to find out that __pthread_get_minstack was added with 2.14.90 [1]. So 2.15 should be the first
> stable release to include __pthread_get_minstack. I'll correct this in my fix.
>
> Would you be ok with this version of the fix?

>
> Thanks and best regards,
> Richard.
>
> [1] version.h at [2]
>     https://github.com/bminor/glibc/blob/2c1094bd700e63a8d7f547b3f5495bedb55c0a08/version.h
>
> [2] Repo at [3]
>     https://github.com/bminor/glibc/tree/2c1094bd700e63a8d7f547b3f5495bedb55c0a08
>
> [3] Commit that introduced __pthread_get_minstack
>     https://github.com/bminor/glibc/commit/2c1094bd700e63a8d7f547b3f5495bedb55c0a08
>
>
> -----Original Message-----
> From: Jiangli Zhou <jianglizhou at google.com>
> Sent: Donnerstag, 20. Februar 2020 17:32
> To: Reingruber, Richard <richard.reingruber at sap.com>
> Cc: hotspot-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS
>
> Hi Richard,
>
> On Thu, Feb 20, 2020 at 1:03 AM Reingruber, Richard
> <richard.reingruber at sap.com> wrote:
> >
> > Hi Jiangli,
> >
> > thanks for providing feedback.
> >
> >     > David's suggestion of using dlsym seems to be more reliable. Regarding the stability of
> >     > __pthread_get_minstack in future glibc versions, Florian has added comment to
> >     > __pthread_get_minstack about external users to ensure it's not removed until TCB is
> >     > separately allocated.  Please see: http://51.15.138.76/patch/18989/ and
> >     > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=91628#c2.
> >
> > So if I understand you and David correctly, then the test should be skipped if the lookup of
> > __pthread_get_minstack fails.
> >
> > Then the test would be skipped on future glibc versions where __pthread_get_minstack was removed
> > because TLS and TCB are not placed on a threads stack anymore.
> >
> > Actually I would prefer not to skip the test then. I'd like to verify if large data structures can
> > be placed in TLS without stack size adjustment or if there is some kind of regression. That's the
> > point of regression tests after all.
> >
>
> That's a very valid point. I thought about that as well yesterday. My
> thinking was that the VM and test would need to be updated to remove
> the workaround for the TLS issue when no longer needed. Do you know if
> the proposed check using gnu_get_libc_version() covers all cases? I
> came across the following archive that mentioned missing symbol in
> eglibc-2.13 (eglibc however is already deprecated).
>
> https://www.mail-archive.com/lfs-dev@linuxfromscratch.org/msg18115.html
>
> > In other words: I wouldn't change test coverage of future glibc versions. I just want to exclude
> > glibc versions were the tested feature cannot work.
>
> Best regards,
> Jiangli
> >
> > Thanks, Richard.
> >
> > -----Original Message-----
> > From: Jiangli Zhou <jianglizhou at google.com>
> > Sent: Mittwoch, 19. Februar 2020 19:24
> > To: Reingruber, Richard <richard.reingruber at sap.com>
> > Cc: hotspot-dev at openjdk.java.net; hotspot-runtime-dev at openjdk.java.net
> > Subject: Re: RFR(S) 8239449: [TESTBUG] test/hotspot/jtreg/runtime/TLS/TestTLS.java: skip test if glibc too old for AdjustStackSizeForTLS
> >
> > Hi Richard,
> >
> > David's suggestion of using dlsym seems to be more reliable. Regarding
> > the stability of __pthread_get_minstack in future glibc versions,
> > Florian has added comment to __pthread_get_minstack about external
> > users to ensure it's not removed until TCB is separately allocated.
> > Please see: http://51.15.138.76/patch/18989/ and
> > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=91628#c2.
> >
> > Best regards,
> > Jiangli
> >
> >
> >
> >
> > On Wed, Feb 19, 2020 at 2:44 AM Reingruber, Richard
> > <richard.reingruber at sap.com> wrote:
> > >
> > > Hi,
> > >
> > > please review this small test fix
> > >
> > > Webrev: http://cr.openjdk.java.net/~rrich/webrevs/8239449/webrev.0/
> > > Bug:    https://bugs.openjdk.java.net/browse/JDK-8239449
> > >
> > > On linux systems with older glibc versions (e.g. 2.11) the option -XX:+AdjustStackSizeForTLS has no effect, because the private function __pthread_get_minstack is not provided, and the test fails. With the fix the test is skipped instead.
> > >
> > > Thanks, Richard.