nptl: Use C11-style atomics for access to __nptl_nthreads

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

	* nptl/allocatestack.c (__reclaim_stacks): Use C-11-style atomic
	access for __nptl_nthreads.
	* nptl/pthread_create.c (__nptl_deallocate_tsd): Likewise.
	(START_THREAD_DEFN): Likewise.
	(__pthread_create_2_1): Likewise.
  

On 04/02/2019 08:05, Florian Weimer wrote:
> 2019-02-03  Florian Weimer  <fweimer@redhat.com>
> 
> 	* nptl/allocatestack.c (__reclaim_stacks): Use C-11-style atomic
> 	access for __nptl_nthreads.
> 	* nptl/pthread_create.c (__nptl_deallocate_tsd): Likewise.
> 	(START_THREAD_DEFN): Likewise.
> 	(__pthread_create_2_1): Likewise.
> 
> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
> index 670cb8ffe6..0dbb155f70 100644
> --- a/nptl/allocatestack.c
> +++ b/nptl/allocatestack.c
> @@ -952,7 +952,7 @@ __reclaim_stacks (void)
>      list_add (&self->list, &stack_used);
>  
>    /* There is one thread running.  */
> -  __nptl_nthreads = 1;
> +  atomic_store_relaxed (&__nptl_nthreads, 1);
>  
>    in_flight_stack = 0;
>  

Ok.

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 04d1c6453e..15fbeceac7 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -341,7 +341,7 @@ __nptl_main_thread_exited (void)
>  {
>    __nptl_deallocate_tsd ();
>  
> -  if (! atomic_decrement_and_test (&__nptl_nthreads))
> +  if (atomic_fetch_add_relaxed (&__nptl_nthreads, -1) > 1)
>      /* Exit the thread, but not the process.  */
>      __exit_thread ();
>  }

It changes the semantic from acquire to relaxed. I think we should document
why it is safe.


> @@ -510,7 +510,7 @@ START_THREAD_DEFN
>    /* If this is the last thread we terminate the process now.  We
>       do not notify the debugger, it might just irritate it if there
>       is no thread left.  */
> -  if (__glibc_unlikely (atomic_decrement_and_test (&__nptl_nthreads)))
> +  if (atomic_fetch_add_relaxed (&__nptl_nthreads, -1) == 1)
>      /* This was the last thread.  */
>      exit (0);

I am not sure if is the correct semantic. Using a similar analogy, c++
share_ptr uses memory_order_acq_rel for the decrement. Are we sure 
relaxed semantic is correct here?

>  
> @@ -769,7 +769,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>          collect_default_sched (pd);
>      }
>  
> -  if (__glibc_unlikely (__nptl_nthreads == 1))
> +  if (__glibc_unlikely (atomic_load_relaxed (&__nptl_nthreads) == 1))
>      _IO_enable_locks ();
>  
>    /* Pass the descriptor to the caller.  */
> @@ -785,7 +785,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>       we momentarily store a false value; this doesn't matter because there
>       is no kosher thing a signal handler interrupting us right here can do
>       that cares whether the thread count is correct.  */
> -  atomic_increment (&__nptl_nthreads);
> +  atomic_fetch_add_relaxed (&__nptl_nthreads, 1);
>  
>    /* Our local value of stopped_start and thread_ran can be accessed at
>       any time. The PD->stopped_start may only be accessed if we have
> @@ -850,7 +850,7 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
>  	     NOTES above).  */
>  
>  	  /* Oops, we lied for a second.  */
> -	  atomic_decrement (&__nptl_nthreads);
> +	  atomic_fetch_add_relaxed (&__nptl_nthreads, -1);
>  
>  	  /* Perhaps a thread wants to change the IDs and is waiting for this
>  	     stillborn thread.  */
>