Message ID | 87tvhjx41u.fsf@oldenburg2.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | nptl: Introduce __nptl_main_thread_exited | expand |
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.
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> @@ -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. */ 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 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; 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 (); +} /* 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) 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 *); };