From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10715 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] add support for pthread_{get,set}attr_default_np GNU extension Date: Mon, 7 Nov 2016 22:52:16 -0500 Message-ID: <20161108035216.GG1555@brightrain.aerifal.cx> References: <20161031172658.970-1-timo.teras@iki.fi> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1478577177 9989 195.159.176.226 (8 Nov 2016 03:52:57 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 8 Nov 2016 03:52:57 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-10728-gllmg-musl=m.gmane.org@lists.openwall.com Tue Nov 08 04:52:51 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1c3xSV-0007gD-Rq for gllmg-musl@m.gmane.org; Tue, 08 Nov 2016 04:52:27 +0100 Original-Received: (qmail 3072 invoked by uid 550); 8 Nov 2016 03:52:29 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 2028 invoked from network); 8 Nov 2016 03:52:28 -0000 Content-Disposition: inline In-Reply-To: <20161031172658.970-1-timo.teras@iki.fi> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:10715 Archived-At: On Mon, Oct 31, 2016 at 07:26:58PM +0200, Timo Teräs wrote: > While generally this is a bad API, it is the only existing API to > affect c++ (std::thread) and c11 (thrd_create) thread stack size. > This patch allows applications only to increate stack and guard > page sizes. > --- > include/pthread.h | 2 ++ > src/thread/pthread_create.c | 7 +++++++ > src/thread/pthread_setattr_default_np.c | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+) > create mode 100644 src/thread/pthread_setattr_default_np.c > > diff --git a/include/pthread.h b/include/pthread.h > index 94ef919..bba9587 100644 > --- a/include/pthread.h > +++ b/include/pthread.h > @@ -215,6 +215,8 @@ int pthread_getaffinity_np(pthread_t, size_t, struct cpu_set_t *); > int pthread_setaffinity_np(pthread_t, size_t, const struct cpu_set_t *); > int pthread_getattr_np(pthread_t, pthread_attr_t *); > int pthread_setname_np(pthread_t, const char *); > +int pthread_getattr_default_np(pthread_attr_t *); > +int pthread_setattr_default_np(const pthread_attr_t *); > int pthread_tryjoin_np(pthread_t, void **); > int pthread_timedjoin_np(pthread_t, void **, const struct timespec *); > #endif > diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c > index 9f6b98e..28cf6d4 100644 > --- a/src/thread/pthread_create.c > +++ b/src/thread/pthread_create.c > @@ -163,6 +163,8 @@ static void *dummy_tsd[1] = { 0 }; > weak_alias(dummy_tsd, __pthread_tsd_main); > > volatile int __block_new_threads = 0; > +size_t __default_stacksize = 0; > +size_t __default_guardsize = 0; > > static FILE *volatile dummy_file = 0; > weak_alias(dummy_file, __stdin_used); > @@ -204,6 +206,11 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att > if (attrp && !c11) attr = *attrp; > > __acquire_ptc(); > + if (!attrp || c11) { > + attr._a_stacksize = __default_stacksize; > + attr._a_guardsize = __default_guardsize; > + } > + > if (__block_new_threads) __wait(&__block_new_threads, 0, 1, 1); > > if (attr._a_stackaddr) { > diff --git a/src/thread/pthread_setattr_default_np.c b/src/thread/pthread_setattr_default_np.c > new file mode 100644 > index 0000000..21cd0f3 > --- /dev/null > +++ b/src/thread/pthread_setattr_default_np.c > @@ -0,0 +1,31 @@ > +#include "pthread_impl.h" > + > +extern size_t __default_stacksize; > +extern size_t __default_guardsize; > + > +int pthread_setattr_default_np(const pthread_attr_t *attrp) > +{ > + if (attrp->_a_stackaddr || attrp->_a_detach || > + attrp->_a_sched || attrp->_a_policy || attrp->_a_prio) > + return EINVAL; > + > + __inhibit_ptc(); > + if (DEFAULT_STACK_SIZE+attrp->_a_stacksize >= DEFAULT_STACK_SIZE+__default_stacksize) > + __default_stacksize = attrp->_a_stacksize; > + if (DEFAULT_GUARD_SIZE+attrp->_a_guardsize >= DEFAULT_GUARD_SIZE+__default_guardsize) > + __default_guardsize = attrp->_a_guardsize; > + __release_ptc(); > + > + return 0; > +} > + > +int pthread_getattr_default_np(pthread_attr_t *attrp) > +{ > + __acquire_ptc(); > + *attrp = (pthread_attr_t) { > + ._a_stacksize = __default_stacksize, > + ._a_guardsize = __default_guardsize, > + }; > + __release_ptc(); > + return 0; > +} As discussed on #musl, I've changed the existing code not to use the ugly default-relative sizes with addition/subtraction of the defaults everywhere; see: https://git.musl-libc.org/cgit/musl/commit/?id=33ce920857405d4f4b342c85b74588a15e2702e5 Your patch needs some minor changes to match that. Otherwise it doesn't look too bad. What do you think about limiting how high the defaults can be set? It's an idea I mentioned on irc before but I don't remember if we followed up with it. Rich