From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-4.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 5404B1F8C6 for ; Mon, 12 Jul 2021 16:01:13 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5AF2F388F02E for ; Mon, 12 Jul 2021 16:01:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5AF2F388F02E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1626105672; bh=coLMEyOp+iXpi09piFBupg3PMJ8GwZUmGqOQ2zEhVo8=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=rDCD7Ah0KZ7+E9pyVy6lSNUb6t+B+nbIA5xlEZRP5hCeXLHQ9smkMAGD6PzIOCBA8 4wQvulYhkUyvrFaKBMesYNZfH/RJpalNiN1BBLThMWS0et8NZUkngG0vqavZi9Tsj2 oE9kTa0bQdIEpfuCsqcmMBwFicYdlMN77fluUW5s= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id E2350389000F for ; Mon, 12 Jul 2021 15:59:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E2350389000F Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-157-6r-eCopUNzyES_Nc0yjQnw-1; Mon, 12 Jul 2021 11:59:57 -0400 X-MC-Unique: 6r-eCopUNzyES_Nc0yjQnw-1 Received: by mail-qt1-f197.google.com with SMTP id j15-20020ac84c8f0000b0290257b7db4a28so289345qtv.9 for ; Mon, 12 Jul 2021 08:59:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=coLMEyOp+iXpi09piFBupg3PMJ8GwZUmGqOQ2zEhVo8=; b=L16gtEXxyZKJYTOM845GWMykAyGzTxNOGzhAR/IExP63aNaT+WUfm8RPGxC+6Kjm+F P/YPJbThlJovsnNYvG5V+gwKyQAMF6yPapZo30FtQ7qYeXxdowRrnCIjvMFraGAARutF 49ipmYib3JsyHOFr6Zo23qEs40k5AECmjDjiMdeTtQMfRy6WD/drEIH7yNM5LvNfGiA4 xXWftqQM4x0/c2vwPjYX1IAs5WST1i78lIlRHpLzVOEdcKc91elu0m8C0k7AXTEnD/FR FlR/or55DHrw5LCoF8AO6AwVU45oOrSbjmIdXMgUdCghHL+XHP1HwEZUfN6W+GyXDzqk ubMg== X-Gm-Message-State: AOAM530rrUhr9XfOtOt+zv+Ua3ANCEOil/F9r9XCRK+WJqacZGciAsgm 1cg0wOJ9XH8p3tjkUh0c2PUJc6ThIVgjfr2z6odf4OZyl4aUg5cuE12nKhW+pq4J5B34cI4gsih T5u6Xa6Z5bp+s4vFm7NQo X-Received: by 2002:ad4:5bec:: with SMTP id k12mr12591809qvc.5.1626105596814; Mon, 12 Jul 2021 08:59:56 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyTaLGIALoSbpOr2gBvFT3ALPYcplB41ylH5vGGnLhpg7T1hpkJ5JNOIEnHZe2j3+MMDkxjXw== X-Received: by 2002:ad4:5bec:: with SMTP id k12mr12591788qvc.5.1626105596588; Mon, 12 Jul 2021 08:59:56 -0700 (PDT) Received: from [192.168.1.16] (198-84-214-74.cpe.teksavvy.com. [198.84.214.74]) by smtp.gmail.com with ESMTPSA id x21sm6868582qkn.0.2021.07.12.08.59.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 12 Jul 2021 08:59:55 -0700 (PDT) Subject: Re: [PATCH] Reduce pollution due to dynamic PTHREAD_STACK_MIN To: Florian Weimer , libc-alpha@sourceware.org References: <87a6mrbsnk.fsf@oldenburg.str.redhat.com> Organization: Red Hat Message-ID: <24e4aaa4-f732-c21b-3f02-a9643ee3b9be@redhat.com> Date: Mon, 12 Jul 2021 11:59:54 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <87a6mrbsnk.fsf@oldenburg.str.redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Carlos O'Donell via Libc-alpha Reply-To: Carlos O'Donell Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 7/12/21 8:53 AM, Florian Weimer wrote: > used to be a header file with no declarations. > GCC's libgomp includes in a #pragma GCC visibility hidden block. > Including from (indirectly) declares everything > in with hidden visibility, resulting in linker failures. > > This commit avoids C declarations in assembler mode and only declares > __sysconf in (and not the entire contents of ). > The __sysconf symbol is already part of the ABI. PTHREAD_STACK_MIN > is no longer defined for __USE_DYNAMIC_STACK_SIZE && __ASSEMBLER__ > because there is no possible definition. > > Additionally, PTHREAD_STACK_MIN is now defined by for > __USE_MISC because this is what developers expect based on the macro > name. It also helps to avoid libgomp linker failures in GCC because > libgomp includes before its visibility hacks. We absolutely want to be able to build glibc with an unmodified released gcc, and requiring a fix would impact usability. Therefore regardless of the exact technical merits of this fix, I think it goes in the right direction. At a later date we can cleanup this technical debt. Reviewed-by: Carlos O'Donell > --- > include/bits/local_lim.h | 7 ----- > nptl/tst-minstack-exit.c | 3 +++ > sysdeps/nptl/pthread.h | 4 ++- > sysdeps/unix/sysv/linux/Makefile | 2 +- > sysdeps/unix/sysv/linux/bits/local_lim.h | 10 ++----- > .../sysv/linux/bits/pthread_stack_min-dynamic.h | 31 ++++++++++++++++++++++ > .../linux/include/bits/pthread_stack_min-dynamic.h | 7 +++++ > 7 files changed, 47 insertions(+), 17 deletions(-) > > diff --git a/include/bits/local_lim.h b/include/bits/local_lim.h > deleted file mode 100644 > index 46d82dc729..0000000000 > --- a/include/bits/local_lim.h > +++ /dev/null > @@ -1,7 +0,0 @@ > -/* Don't define PTHREAD_STACK_MIN to sysconf (_SC_THREAD_STACK_MIN) for > - glibc build. */ > -#if !defined _ISOMAC > -# undef __USE_DYNAMIC_STACK_SIZE > -#endif > - > -#include_next OK. This moves from bits/local_lim.h -> sysdeps/unix/sysv/linux/include/bits/pthread_stack_min-dynamic.h > diff --git a/nptl/tst-minstack-exit.c b/nptl/tst-minstack-exit.c > index 4e7dc3e786..e577924178 100644 > --- a/nptl/tst-minstack-exit.c > +++ b/nptl/tst-minstack-exit.c > @@ -24,6 +24,9 @@ > #include > #include > > +_Static_assert (__SC_THREAD_STACK_MIN_VALUE == _SC_THREAD_STACK_MIN, > + "__SC_THREAD_STACK_MIN_VALUE is correct"); > + OK. Keeps the constant value checked so it doesn't change. > static void * > threadfunc (void *closure) > { > diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h > index 52e2eadbfc..f1b7f2bdc6 100644 > --- a/sysdeps/nptl/pthread.h > +++ b/sysdeps/nptl/pthread.h > @@ -29,7 +29,9 @@ > #include > #include > #include > - > +#ifdef __USE_MISC > +# include > +#endif OK. Include it early (everyone using pthread.h would expect it). > > /* Detach state. */ > enum > diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile > index 7d43dc95f2..8257e5b548 100644 > --- a/sysdeps/unix/sysv/linux/Makefile > +++ b/sysdeps/unix/sysv/linux/Makefile > @@ -109,7 +109,7 @@ sysdep_headers += sys/mount.h sys/acct.h \ > bits/types/struct_semid64_ds_helper.h \ > bits/types/struct_shmid64_ds.h \ > bits/types/struct_shmid64_ds_helper.h \ > - bits/pthread_stack_min.h > + bits/pthread_stack_min.h bits/pthread_stack_min-dynamic.h OK. > > tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \ > tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \ > diff --git a/sysdeps/unix/sysv/linux/bits/local_lim.h b/sysdeps/unix/sysv/linux/bits/local_lim.h > index 0b1c7f40be..b8f89236bc 100644 > --- a/sysdeps/unix/sysv/linux/bits/local_lim.h > +++ b/sysdeps/unix/sysv/linux/bits/local_lim.h > @@ -77,14 +77,8 @@ > priority level. */ > #define AIO_PRIO_DELTA_MAX 20 > > -/* Minimum size for a thread. We are free to choose a reasonable value. */ > -#undef PTHREAD_STACK_MIN > -#if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE > -# include > -# define PTHREAD_STACK_MIN sysconf (_SC_THREAD_STACK_MIN) > -#else > -# include > -#endif > +/* Arrange for the definition of PTHREAD_STACK_MIN. */ > +#include OK. > > /* Maximum number of timer expiration overruns. */ > #define DELAYTIMER_MAX 2147483647 > diff --git a/sysdeps/unix/sysv/linux/bits/pthread_stack_min-dynamic.h b/sysdeps/unix/sysv/linux/bits/pthread_stack_min-dynamic.h > new file mode 100644 > index 0000000000..5f5fc5f024 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/bits/pthread_stack_min-dynamic.h > @@ -0,0 +1,31 @@ > +/* Definition of PTHREAD_STACK_MIN, possibly dynamic. > + Copyright (C) 2021 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#ifndef PTHREAD_STACK_MIN > +# if defined __USE_DYNAMIC_STACK_SIZE && __USE_DYNAMIC_STACK_SIZE > +# ifndef __ASSEMBLER__ > +# define __SC_THREAD_STACK_MIN_VALUE 75 > +__BEGIN_DECLS > +extern long int __sysconf (int __name) __THROW; > +__END_DECLS > +# define PTHREAD_STACK_MIN __sysconf (__SC_THREAD_STACK_MIN_VALUE) OK. Decouple from the actual definition of the sysconf value. > +# endif > +# else > +# include > +# endif > +#endif > diff --git a/sysdeps/unix/sysv/linux/include/bits/pthread_stack_min-dynamic.h b/sysdeps/unix/sysv/linux/include/bits/pthread_stack_min-dynamic.h > new file mode 100644 > index 0000000000..d66f584b21 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/include/bits/pthread_stack_min-dynamic.h > @@ -0,0 +1,7 @@ > +/* Don't define PTHREAD_STACK_MIN to __sysconf (_SC_THREAD_STACK_MIN) > + for glibc build. */ > +#ifdef _ISOMAC > +# include > +#else > +# include > +#endif > OK. -- Cheers, Carlos.