nptl: Introduce __nptl_main_thread_exited

  __libc_start_main previously accessed both __nptl_nthreads and
__nptl_deallocate_tsd.  Move this code into
__nptl_main_thread_exited in nptl/pthread_create.c, to concentrate
nptl processing logic in the nptl subdirectory.

2019-02-03  Florian Weimer  <fweimer@redhat.com>

	* nptl/pthreadP.h (__nptl_main_thread_exited): Declare.
	(__nptl_deallocate_tsd): Remove declaration.
	* sysdeps/nptl/pthread-functions.h (struct pthread_functions):
	Remove ptr_nthreads, ptr__nptl_deallocate_tsd.  Add
	ptr__nptl_main_thread_exited.
	(HAVE_PTR_NTHREADS): Remove macro.  No longer used.
	* nptl/pthread_create.c (__nptl_deallocate_tsd): Make static.
	(__nptl_main_thread_exited): New function.  Extracted from
	__libc_start_main.
	* nptl/nptl-init.c (pthread_functions): Do not initialize
	ptr_nthreads, ptr__nptl_deallocate_tsd.  Initialize
	ptr__nptl_main_thread_exited.
	* csu/libc-start.c (LIBC_START_MAIN): Call
	__nptl_main_thread_exited on pthread_exit/pthread_cancel.
	* nptl/pthread_exit.c: Update comment.
  

On 04/02/2019 08:04, Florian Weimer wrote:
> __libc_start_main previously accessed both __nptl_nthreads and
> __nptl_deallocate_tsd.  Move this code into
> __nptl_main_thread_exited in nptl/pthread_create.c, to concentrate
> nptl processing logic in the nptl subdirectory.
> 
> 2019-02-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* nptl/pthreadP.h (__nptl_main_thread_exited): Declare.
> 	(__nptl_deallocate_tsd): Remove declaration.
> 	* sysdeps/nptl/pthread-functions.h (struct pthread_functions):
> 	Remove ptr_nthreads, ptr__nptl_deallocate_tsd.  Add
> 	ptr__nptl_main_thread_exited.
> 	(HAVE_PTR_NTHREADS): Remove macro.  No longer used.
> 	* nptl/pthread_create.c (__nptl_deallocate_tsd): Make static.
> 	(__nptl_main_thread_exited): New function.  Extracted from
> 	__libc_start_main.
> 	* nptl/nptl-init.c (pthread_functions): Do not initialize
> 	ptr_nthreads, ptr__nptl_deallocate_tsd.  Initialize
> 	ptr__nptl_main_thread_exited.
> 	* csu/libc-start.c (LIBC_START_MAIN): Call
> 	__nptl_main_thread_exited on pthread_exit/pthread_cancel.
> 	* nptl/pthread_exit.c: Update comment.

LGTM with two nits below.

> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 5d9c3675fa..23ded05333 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -43,12 +43,6 @@ uintptr_t __pointer_chk_guard_local
>  # endif
>  #endif
>  
> -#ifdef HAVE_PTR_NTHREADS
> -/* We need atomic operations.  */
> -# include <atomic.h>
> -#endif
> -
> -
>  #ifndef SHARED
>  # include <link.h>
>  # include <dl-irel.h>

Ok.

> @@ -309,30 +303,23 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>      }
>    else
>      {
> -      /* Remove the thread-local data.  */
> +      /* The main is terminating via unwinding, either via
> +	 pthread_exit or pthread_cancel.  Call into the thread
> +	 library, to call deallocate thread-local data.  This may
> +	 terminate the thread if it is not the last thread.  */
>  # ifdef SHARED
> -      PTHFCT_CALL (ptr__nptl_deallocate_tsd, ());
> +      PTHFCT_CALL (ptr__nptl_main_thread_exited, ());
>  # else
> -      extern void __nptl_deallocate_tsd (void) __attribute ((weak));
> -      __nptl_deallocate_tsd ();
> +      /* The function is only invoked on pthread_exit or
> +	 pthread_cancel, which means we necessarily have a definition
> +	 for it, and no null pointer check is needed here.  */
> +      extern void __nptl_main_thread_exited (void)
> +	__attribute__ ((weak)) attribute_hidden;
> +      __nptl_main_thread_exited ();

User weak_function macro.

>  # endif
>  
> -      /* One less thread.  Decrement the counter.  If it is zero we
> -	 terminate the entire process.  */
> +      /* Exit status for cancellation.  */
>        result = 0;
> -# ifdef SHARED
> -      unsigned int *ptr = __libc_pthread_functions.ptr_nthreads;
> -#  ifdef PTR_DEMANGLE
> -      PTR_DEMANGLE (ptr);
> -#  endif
> -# else
> -      extern unsigned int __nptl_nthreads __attribute ((weak));
> -      unsigned int *const ptr = &__nptl_nthreads;
> -# endif
> -
> -      if (! atomic_decrement_and_test (ptr))
> -	/* Not much left to do but to exit the thread, not the process.  */
> -	__exit_thread ();
>      }
>  #else
>    /* Nothing fancy, just call the function.  */

Ok.

> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
> index b5895fabf3..757397fcb0 100644
> --- a/nptl/nptl-init.c
> +++ b/nptl/nptl-init.c
> @@ -132,9 +132,8 @@ static const struct pthread_functions pthread_functions =
>      .ptr___pthread_setspecific = __pthread_setspecific,
>      .ptr__pthread_cleanup_push_defer = __pthread_cleanup_push_defer,
>      .ptr__pthread_cleanup_pop_restore = __pthread_cleanup_pop_restore,
> -    .ptr_nthreads = &__nptl_nthreads,
>      .ptr___pthread_unwind = &__pthread_unwind,
> -    .ptr__nptl_deallocate_tsd = __nptl_deallocate_tsd,
> +    .ptr__nptl_main_thread_exited = &__nptl_main_thread_exited,
>  # ifdef SIGSETXID
>      .ptr__nptl_setxid = __nptl_setxid,
>  # endif

Ok.

> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 626bd4b096..7277567c58 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -590,7 +590,11 @@ extern void _pthread_cleanup_push_defer (struct _pthread_cleanup_buffer *buffer,
>  extern void _pthread_cleanup_pop_restore (struct _pthread_cleanup_buffer *buffer,
>  					  int execute);
>  
> -extern void __nptl_deallocate_tsd (void) attribute_hidden;
> +/* Called from __libc_start_main in csu/libc-start.c (via the nptl
> +   function pointer interface) if the main thread is terminating via
> +   pthread_exit or pthread_cancel (and not via a regular return from
> +   main).  */
> +void __nptl_main_thread_exited (void) attribute_hidden;
>  
>  extern void __nptl_setxid_error (struct xid_command *cmdp, int error)
>    attribute_hidden;

Ok.

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 2bd2b10727..04d1c6453e 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -245,8 +245,7 @@ __find_in_stack_list (struct pthread *pd)
>  
>  
>  /* Deallocate POSIX thread-local-storage.  */
> -void
> -attribute_hidden
> +static void
>  __nptl_deallocate_tsd (void)
>  {
>    struct pthread *self = THREAD_SELF;
> @@ -337,6 +336,15 @@ __nptl_deallocate_tsd (void)
>      }
>  }
>  
> +void
> +__nptl_main_thread_exited (void)
> +{
> +  __nptl_deallocate_tsd ();
> +
> +  if (! atomic_decrement_and_test (&__nptl_nthreads))
> +    /* Exit the thread, but not the process.  */
> +    __exit_thread ();
> +}

Why not use "atomic_exchange_and_add ((mem), -1) == 1" instead? My understanding
is we are moving away for these architecture micro-optimization and let compiler
handling these kind of stuff.

>  
>  /* Deallocate a thread's stack after optionally making sure the thread
>     descriptor is still valid.  */
> diff --git a/nptl/pthread_exit.c b/nptl/pthread_exit.c
> index abc9019334..a6d56751b0 100644
> --- a/nptl/pthread_exit.c
> +++ b/nptl/pthread_exit.c
> @@ -29,6 +29,6 @@ __pthread_exit (void *value)
>  }
>  weak_alias (__pthread_exit, pthread_exit)
>  
> -/* After a thread terminates, __libc_start_main decrements
> -   __nptl_nthreads defined in pthread_create.c.  */
> +/* Exiting from main via pthread_exit needs a definition of
> +   __nptl_main_thread_exited.  */
>  PTHREAD_STATIC_FN_REQUIRE (__pthread_create)

Ok.

> diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
> index cd5e94d1a6..d95a636220 100644
> --- a/sysdeps/nptl/pthread-functions.h
> +++ b/sysdeps/nptl/pthread-functions.h
> @@ -88,11 +88,9 @@ struct pthread_functions
>  					   void (*) (void *), void *);
>    void (*ptr__pthread_cleanup_pop_restore) (struct _pthread_cleanup_buffer *,
>  					    int);
> -#define HAVE_PTR_NTHREADS
> -  unsigned int *ptr_nthreads;
>    void (*ptr___pthread_unwind) (__pthread_unwind_buf_t *)
>         __attribute ((noreturn)) __cleanup_fct_attribute;
> -  void (*ptr__nptl_deallocate_tsd) (void);
> +  void (*ptr__nptl_main_thread_exited) (void);
>    int (*ptr__nptl_setxid) (struct xid_command *);
>    void (*ptr_set_robust) (struct pthread *);
>  };
> 

Ok.