Ping: [PATCH v2] Set default stack size from program environment

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Ping!

On Tue, Feb 05, 2013 at 06:38:32PM +0530, Siddhesh Poyarekar wrote:
> Hi,
> 
> This is take 2 to allow default stack size to be controlled
> independently of process stack by using an environment variable so
> that one need not change code for it.  Following my original
> submission[1], Roland suggested that I break down the patch and post
> them, with justifications for each of them.  The original intent of my
> submission was to get the environment variable so that default thread
> stack size can be controlled without having to change program code,
> similar to features that AIX and Solaris provide.  The new API
> functions were eye-candy to make the package complete, so I don't have
> a separate justification for them other than that they 'complete the
> implementation' so to speak.
> 
> As a result, this patch only implements the environment variable
> support.  I will post the API functions patch separately on top of
> this patch if this is accepted; I don't have any use for the API
> functions alone.  Ditto for the manual patch.
> 
> The patch also includes a test case to verify the new feature.  No
> regressions were reported in the testsuite due to this patch.
> 
> Siddhesh
> 
> [1] http://sourceware.org/ml/libc-alpha/2013-01/msg00572.html
> 
> ChangeLog:
> 
> 	* csu/libc-start.c (__pthread_initialize_minimal): Change
> 	function arguments.
> 	* csu/libc-tls.c (__pthread_initialize_minimal): Likewise.
> 
> nptl/ChangeLog:
> 
> 	* Makefile (tests): Add tst-pthread-stack-env.
> 	(tst-pthread-stack-env-ENV): Set environment for test.
> 	* nptl-init.c (set_default_stacksize): New function.
> 	(__pthread_initialize_minimal_internal): Accept ARGC, ARGV and
> 	ENVP.  Initialize __ENVIRON and set __DEFAULT_STACKSIZE.
>         * tst-pthread-stack-env.c: New test case.
> 
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index d4a135f..edd267c 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -31,7 +31,7 @@ extern int __libc_multiple_libcs;
>  #include <tls.h>
>  #ifndef SHARED
>  # include <dl-osinfo.h>
> -extern void __pthread_initialize_minimal (void);
> +extern void __pthread_initialize_minimal (int, char **, char **);
>  # ifndef THREAD_SET_STACK_GUARD
>  /* Only exported for architectures that don't store the stack guard canary
>     in thread local area.  */
> @@ -174,7 +174,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    /* Initialize the thread library at least a bit since the libgcc
>       functions are using thread functions if these are available and
>       we need to setup errno.  */
> -  __pthread_initialize_minimal ();
> +  __pthread_initialize_minimal (argc, argv, __environ);
>  
>    /* Set up the stack checker's canary.  */
>    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> diff --git a/csu/libc-tls.c b/csu/libc-tls.c
> index 90daaa6..f51a711 100644
> --- a/csu/libc-tls.c
> +++ b/csu/libc-tls.c
> @@ -243,7 +243,7 @@ _dl_tls_setup (void)
>     not used.  */
>  void
>  __attribute__ ((weak))
> -__pthread_initialize_minimal (void)
> +__pthread_initialize_minimal (int argc, char **argv, char **envp)
>  {
>    __libc_setup_tls (TLS_INIT_TCB_SIZE, TLS_INIT_TCB_ALIGN);
>  }
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 6af4b37..1a1ebec 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -251,7 +251,8 @@ tests = tst-typesizes \
>  	tst-exec1 tst-exec2 tst-exec3 tst-exec4 \
>  	tst-exit1 tst-exit2 tst-exit3 \
>  	tst-stdio1 tst-stdio2 \
> -	tst-stack1 tst-stack2 tst-stack3 tst-pthread-getattr \
> +	tst-stack1 tst-stack2 tst-stack3 \
> +	tst-pthread-getattr tst-pthread-stack-env \
>  	tst-unload \
>  	tst-dlsym1 \
>  	tst-sysconf \
> @@ -441,6 +442,8 @@ tst-cancel7-ARGS = --command "exec $(host-test-program-cmd)"
>  tst-cancelx7-ARGS = $(tst-cancel7-ARGS)
>  tst-umask1-ARGS = $(objpfx)tst-umask1.temp
>  
> +tst-pthread-stack-env-ENV = LIBC_PTHREAD_DEFAULT_STACKSIZE_NP=1048576
> +
>  $(objpfx)tst-atfork2: $(libdl) $(shared-thread-library)
>  LDFLAGS-tst-atfork2 = -rdynamic
>  tst-atfork2-ENV = MALLOC_TRACE=$(objpfx)tst-atfork2.mtrace
> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index 19e6616..eb5fe77 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -276,8 +276,26 @@ extern void **__libc_dl_error_tsd (void) __attribute__ ((const));
>  /* This can be set by the debugger before initialization is complete.  */
>  static bool __nptl_initial_report_events __attribute_used__;
>  
> +static void
> +set_default_stacksize (size_t stacksize)
> +{
> +  if (stacksize < PTHREAD_STACK_MIN)
> +    stacksize = PTHREAD_STACK_MIN;
> +
> +  /* Make sure it meets the minimum size that allocate_stack
> +     (allocatestack.c) will demand, which depends on the page size.  */
> +  const uintptr_t pagesz = GLRO(dl_pagesize);
> +  const size_t minstack = pagesz + __static_tls_size + MINIMAL_REST_STACK;
> +
> +  if (stacksize < minstack)
> +    stacksize = minstack;
> +
> +  /* Round the resource limit up to page size.  */
> +  stacksize = (stacksize + pagesz - 1) & -pagesz;
> +  __default_stacksize = stacksize;
> +}
>  void
> -__pthread_initialize_minimal_internal (void)
> +__pthread_initialize_minimal_internal (int argc, char **argv, char **envp)
>  {
>  #ifndef SHARED
>    /* Unlike in the dynamically linked case the dynamic linker has not
> @@ -401,29 +419,41 @@ __pthread_initialize_minimal_internal (void)
>  
>    __static_tls_size = roundup (__static_tls_size, static_tls_align);
>  
> -  /* Determine the default allowed stack size.  This is the size used
> -     in case the user does not specify one.  */
> -  struct rlimit limit;
> -  if (getrlimit (RLIMIT_STACK, &limit) != 0
> -      || limit.rlim_cur == RLIM_INFINITY)
> -    /* The system limit is not usable.  Use an architecture-specific
> -       default.  */
> -    limit.rlim_cur = ARCH_STACK_DEFAULT_SIZE;
> -  else if (limit.rlim_cur < PTHREAD_STACK_MIN)
> -    /* The system limit is unusably small.
> -       Use the minimal size acceptable.  */
> -    limit.rlim_cur = PTHREAD_STACK_MIN;
> +  /* Initialize the environment.  libc.so gets initialized after us due to a
> +     circular dependency and hence __environ is not available otherwise.  */
> +    __environ = envp;
>  
> -  /* Make sure it meets the minimum size that allocate_stack
> -     (allocatestack.c) will demand, which depends on the page size.  */
> -  const uintptr_t pagesz = GLRO(dl_pagesize);
> -  const size_t minstack = pagesz + __static_tls_size + MINIMAL_REST_STACK;
> -  if (limit.rlim_cur < minstack)
> -    limit.rlim_cur = minstack;
> +#ifndef SHARED
> +    __libc_init_secure ();
> +#endif
>  
> -  /* Round the resource limit up to page size.  */
> -  limit.rlim_cur = (limit.rlim_cur + pagesz - 1) & -pagesz;
> -  __default_stacksize = limit.rlim_cur;
> +  size_t stacksize = 0;
> +  char *envval = __libc_secure_getenv ("LIBC_PTHREAD_DEFAULT_STACKSIZE_NP");
> +
> +  if (__glibc_unlikely (envval != NULL && envval[0] != '\0'))
> +    {
> +      char *env_conv = envval;
> +      size_t ret = strtoul (envval, &env_conv, 0);
> +
> +      if (*env_conv == '\0')
> +	stacksize = ret;
> +    }
> +
> +  if (stacksize == 0)
> +    {
> +      /* Determine the default allowed stack size.  This is the size used
> +	 in case the user does not specify one.  */
> +      struct rlimit limit;
> +      if (getrlimit (RLIMIT_STACK, &limit) != 0
> +	  || limit.rlim_cur == RLIM_INFINITY)
> +	/* The system limit is not usable.  Use an architecture-specific
> +	   default.  */
> +	stacksize = ARCH_STACK_DEFAULT_SIZE;
> +      else
> +	stacksize = limit.rlim_cur;
> +    }
> +
> +  set_default_stacksize (stacksize);
>  
>  #ifdef SHARED
>    /* Transfer the old value from the dynamic linker's internal location.  */
> diff --git a/nptl/tst-pthread-stack-env.c b/nptl/tst-pthread-stack-env.c
> new file mode 100644
> index 0000000..0c7273e
> --- /dev/null
> +++ b/nptl/tst-pthread-stack-env.c
> @@ -0,0 +1,72 @@
> +/* Verify that pthreads uses the default thread stack size set with the
> +   LIBC_PTHREAD_DEFAULT_STACKSIZE_NP environment variable.
> +   Copyright (C) 2013 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
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#define STACKSIZE 1024 * 1024L
> +
> +void *
> +thr (void *u)
> +{
> +  size_t stacksize, guardsize;
> +  pthread_attr_t attr;
> +  pthread_getattr_np (pthread_self (), &attr);
> +
> +  pthread_attr_getstacksize (&attr, &stacksize);
> +  pthread_attr_getguardsize (&attr, &guardsize);
> +
> +  /* FIXME once guardsize is excluded from stacksize.  */
> +  if (stacksize - guardsize != STACKSIZE)
> +    {
> +      printf ("Stack size is %zu, should be %zu\n", stacksize - guardsize,
> +	      STACKSIZE);
> +      return (void *) 1;
> +    }
> +
> +  return NULL;
> +}
> +
> +int
> +do_test (int argc, char **argv)
> +{
> +  pthread_t t;
> +  void *thr_ret;
> +  int ret;
> +
> +  if ((ret = pthread_create (&t, NULL, thr, NULL)) != 0)
> +    {
> +      printf ("thread create failed: %s\n", strerror (ret));
> +      return 1;
> +    }
> +
> +  if ((ret = pthread_join (t, &thr_ret)) != 0)
> +    {
> +      printf ("join failed: %s\n", strerror (ret));
> +      return 1;
> +    }
> +
> +  if (thr_ret != NULL && thr_ret != PTHREAD_CANCELED)
> +    return 1;
> +
> +  return 0;
> +}
> +
> +#include "../test-skeleton.c"