nptl: Introduce __nptl_main_thread_exited
Commit Message
__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.
Comments
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.
@@ -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>
@@ -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 ();
# 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. */
@@ -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
@@ -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;
@@ -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 ();
+}
/* Deallocate a thread's stack after optionally making sure the thread
descriptor is still valid. */
@@ -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)
@@ -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 *);
};