This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 3/3] Single thread optimization refactor
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 10 Oct 2017 17:39:19 -0300
- Subject: Re: [PATCH 3/3] Single thread optimization refactor
- Authentication-results: sourceware.org; auth=none
- References: <1502140314-16598-1-git-send-email-adhemerval.zanella@linaro.org> <1502140314-16598-3-git-send-email-adhemerval.zanella@linaro.org> <c4ac2e34-984d-8e66-fadf-c79c2ec5fa3d@linaro.org>
Ping (x2).
On 31/08/2017 16:17, Adhemerval Zanella wrote:
> Ping.
>
> On 07/08/2017 18:11, Adhemerval Zanella wrote:
>> Current GLIBC uses the define TLS_MULTIPLE_THREADS_IN_TCB to whether
>> declare or not the global variables used for the single thread
>> optimization for cancellable syscalls (STOCS). ALthough it does work
>> correctly, this is not the correct define since there are architectures
>> that do not define TLS_MULTIPLE_THREADS_IN_TCB but does not use the
>> global variables (by prefering accessing the TCB/pthread_t fields
>> instead).
>>
>> With single thread optimization for cancellable syscalls refactored
>> by using SINGLE_THREAD_BY_GLOBAL define, we can use it instead to
>> define where the __{libc,pthread}_multiple_threads is defined.
>>
>> This is based on my "Remove sysdep-cancel assembly macro" patchset [1].
>>
>> Checked on x86_64-linux-gnu and on a build with major touched
>> ABis (aarch64-linux-gnu, alpha-linux-gnu, arm-linux-gnueabihf,
>> hppa-linux-gnu, i686-linux-gnu, m68k-linux-gnu, microblaze-linux-gnu,
>> mips-linux-gnu, mips64-linux-gnu, powerpc-linux-gnu,
>> powerpc64le-linux-gnu, s390-linux-gnu, s390x-linux-gnu, sh4-linux-gnu,
>> sparcv9-linux-gnu, sparc64-linux-gnu, tilegx-linux-gnu).
>>
>> * nptl/allocatestack.c (allocate_state): Use
>> SET_SINGLE_THREAD_PTHREAD macro.
>> * nptl/pthread_cancel.c (__pthread_cancel): Likewise.
>> * nptl/libc_multiple_threads.c (__libc_multiple_threads): Define for
>> libc and SINGLE_THREAD_BY_GLOBAL.
>> * nptl/libc_pthread_init.c (__libc_pthread_init): Use same
>> signature whether SINGLE_THREAD_BY_GLOBAL is set or not.
>> * nptl/pthreadP.h (__libc_pthread_init): Likewise.
>> * nptl/nptl-init.c (__libc_multiple_threads_ptr): Use
>> MULTIPLE_THREADS_PTR_DEF.
>> (__pthread_initialize_minimal_internal): Use MULTIPLE_THREADS_PTR_SET
>> macro.
>> * nptl/vars.c (__pthread_multiple_threads): Define for libpthread
>> and SINGLE_THREAD_BY_GLOBAL.
>> * sysdeps/unix/sysv/linux/sysdep-cancel.h (MULTIPLE_THREADS_PTR_DEF):
>> Define.
>> (MULTIPLE_THREADS_PTR_SET): Likewise.
>> (SET_SINGLE_THREAD_PTHREAD): Likewise.
>>
>> [1] https://sourceware.org/ml/libc-alpha/2017-08/msg00095.html
>> ---
>> ChangeLog | 19 +++++++++++++++++++
>> nptl/allocatestack.c | 9 +++------
>> nptl/libc_multiple_threads.c | 4 +---
>> nptl/libc_pthread_init.c | 12 ++++--------
>> nptl/nptl-init.c | 14 +++++---------
>> nptl/pthreadP.h | 14 --------------
>> nptl/pthread_cancel.c | 7 +++----
>> nptl/vars.c | 12 ++++++------
>> sysdeps/unix/sysv/linux/sysdep-cancel.h | 27 +++++++++++++++++++++++++--
>> 9 files changed, 66 insertions(+), 52 deletions(-)
>>
>> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
>> index ce2e24a..89cb0de 100644
>> --- a/nptl/allocatestack.c
>> +++ b/nptl/allocatestack.c
>> @@ -32,6 +32,7 @@
>> #include <futex-internal.h>
>> #include <kernel-features.h>
>> #include <stack-aliasing.h>
>> +#include <sysdep-cancel.h>
>>
>>
>> #ifndef NEED_SEPARATE_REGISTER_STACK
>> @@ -456,9 +457,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
>>
>> /* This is at least the second thread. */
>> pd->header.multiple_threads = 1;
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> - __pthread_multiple_threads = *__libc_multiple_threads_ptr = 1;
>> -#endif
>> + SET_SINGLE_THREAD_PTHREAD (1);
>>
>> #ifndef __ASSUME_PRIVATE_FUTEX
>> /* The thread must know when private futexes are supported. */
>> @@ -576,9 +575,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
>>
>> /* This is at least the second thread. */
>> pd->header.multiple_threads = 1;
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> - __pthread_multiple_threads = *__libc_multiple_threads_ptr = 1;
>> -#endif
>> + SET_SINGLE_THREAD_PTHREAD (1);
>>
>> #ifndef __ASSUME_PRIVATE_FUTEX
>> /* The thread must know when private futexes are supported. */
>> diff --git a/nptl/libc_multiple_threads.c b/nptl/libc_multiple_threads.c
>> index ef6a4be..1ae23e2 100644
>> --- a/nptl/libc_multiple_threads.c
>> +++ b/nptl/libc_multiple_threads.c
>> @@ -18,11 +18,9 @@
>>
>> #include <pthreadP.h>
>>
>> -#if IS_IN (libc)
>> -# ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> +#if IS_IN (libc) && defined (SINGLE_THREAD_BY_GLOBAL)
>> /* Variable set to a nonzero value either if more than one thread runs or ran,
>> or if a single-threaded process is trying to cancel itself. See
>> nptl/descr.h for more context on the single-threaded process case. */
>> int __libc_multiple_threads attribute_hidden;
>> -# endif
>> #endif
>> diff --git a/nptl/libc_pthread_init.c b/nptl/libc_pthread_init.c
>> index 0db7a10..aedd92f 100644
>> --- a/nptl/libc_pthread_init.c
>> +++ b/nptl/libc_pthread_init.c
>> @@ -26,18 +26,12 @@
>> #include <libc-lock.h>
>> #include <sysdep.h>
>> #include <ldsodefs.h>
>> -
>> +#include <sysdep-cancel.h>
>>
>> unsigned long int *__fork_generation_pointer;
>>
>>
>> -#ifdef TLS_MULTIPLE_THREADS_IN_TCB
>> -void
>> -#else
>> -extern int __libc_multiple_threads attribute_hidden;
>> -
>> int *
>> -#endif
>> internal_function
>> __libc_pthread_init (unsigned long int *ptr, void (*reclaim) (void),
>> const struct pthread_functions *functions)
>> @@ -74,8 +68,10 @@ __libc_pthread_init (unsigned long int *ptr, void (*reclaim) (void),
>> __libc_pthread_functions_init = 1;
>> #endif
>>
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> +#ifdef SINGLE_THREAD_BY_GLOBAL
>> return &__libc_multiple_threads;
>> +# else
>> + return NULL;
>> #endif
>> }
>>
>> diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
>> index 2921607..a1320f3 100644
>> --- a/nptl/nptl-init.c
>> +++ b/nptl/nptl-init.c
>> @@ -38,11 +38,9 @@
>> #include <kernel-features.h>
>> #include <libc-pointer-arith.h>
>> #include <pthread-pids.h>
>> +#include <sysdep-cancel.h>
>>
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> -/* Pointer to the corresponding variable in libc. */
>> -int *__libc_multiple_threads_ptr attribute_hidden;
>> -#endif
>> +MULTIPLE_THREADS_PTR_DEF;
>>
>> /* Size and alignment of static TLS block. */
>> size_t __static_tls_size;
>> @@ -457,11 +455,9 @@ __pthread_initialize_minimal_internal (void)
>> GL(dl_wait_lookup_done) = &__wait_lookup_done;
>>
>> /* Register the fork generation counter with the libc. */
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> - __libc_multiple_threads_ptr =
>> -#endif
>> - __libc_pthread_init (&__fork_generation, __reclaim_stacks,
>> - ptr_pthread_functions);
>> + MULTIPLE_THREADS_PTR_SET (__libc_pthread_init (&__fork_generation,
>> + __reclaim_stacks,
>> + ptr_pthread_functions));
>>
>> /* Determine whether the machine is SMP or not. */
>> __is_smp = is_smp_system ();
>> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
>> index 6e7d6ff..a79f0b2 100644
>> --- a/nptl/pthreadP.h
>> +++ b/nptl/pthreadP.h
>> @@ -384,25 +384,11 @@ hidden_proto (__nptl_create_event)
>> hidden_proto (__nptl_death_event)
>>
>> /* Register the generation counter in the libpthread with the libc. */
>> -#ifdef TLS_MULTIPLE_THREADS_IN_TCB
>> -extern void __libc_pthread_init (unsigned long int *ptr,
>> - void (*reclaim) (void),
>> - const struct pthread_functions *functions)
>> - internal_function;
>> -#else
>> extern int *__libc_pthread_init (unsigned long int *ptr,
>> void (*reclaim) (void),
>> const struct pthread_functions *functions)
>> internal_function;
>>
>> -/* Variable set to a nonzero value either if more than one thread runs or ran,
>> - or if a single-threaded process is trying to cancel itself. See
>> - nptl/descr.h for more context on the single-threaded process case. */
>> -extern int __pthread_multiple_threads attribute_hidden;
>> -/* Pointer to the corresponding variable in libc. */
>> -extern int *__libc_multiple_threads_ptr attribute_hidden;
>> -#endif
>> -
>> /* Find a thread given its TID. */
>> extern struct pthread *__find_thread_by_id (pid_t tid) attribute_hidden
>> #ifdef SHARED
>> diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
>> index 742dfe6..f52ea08 100644
>> --- a/nptl/pthread_cancel.c
>> +++ b/nptl/pthread_cancel.c
>> @@ -19,10 +19,11 @@
>> #include <errno.h>
>> #include <signal.h>
>> #include <stdlib.h>
>> -#include "pthreadP.h"
>> #include <atomic.h>
>> #include <sysdep.h>
>> #include <unistd.h>
>> +#include <pthreadP.h>
>> +#include <sysdep-cancel.h>
>>
>> int
>> __pthread_cancel (pthread_t th)
>> @@ -88,9 +89,7 @@ __pthread_cancel (pthread_t th)
>> cannot. So we set multiple_threads to true so that cancellation
>> points get executed. */
>> THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1);
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> - __pthread_multiple_threads = *__libc_multiple_threads_ptr = 1;
>> -#endif
>> + SET_SINGLE_THREAD_PTHREAD (1);
>> }
>> /* Mark the thread as canceled. This has to be done
>> atomically since other bits could be modified as well. */
>> diff --git a/nptl/vars.c b/nptl/vars.c
>> index 198f463..20ebcd5 100644
>> --- a/nptl/vars.c
>> +++ b/nptl/vars.c
>> @@ -30,14 +30,14 @@ int __default_pthread_attr_lock = LLL_LOCK_INITIALIZER;
>> /* Flag whether the machine is SMP or not. */
>> int __is_smp attribute_hidden;
>>
>> -#ifndef TLS_MULTIPLE_THREADS_IN_TCB
>> +/* Table of the key information. */
>> +struct pthread_key_struct __pthread_keys[PTHREAD_KEYS_MAX]
>> + __attribute__ ((nocommon));
>> +hidden_data_def (__pthread_keys)
>> +
>> +#if IS_IN (libpthread) && defined (SINGLE_THREAD_BY_GLOBAL)
>> /* Variable set to a nonzero value either if more than one thread runs or ran,
>> or if a single-threaded process is trying to cancel itself. See
>> nptl/descr.h for more context on the single-threaded process case. */
>> int __pthread_multiple_threads attribute_hidden;
>> #endif
>> -
>> -/* Table of the key information. */
>> -struct pthread_key_struct __pthread_keys[PTHREAD_KEYS_MAX]
>> - __attribute__ ((nocommon));
>> -hidden_data_def (__pthread_keys)
>> diff --git a/sysdeps/unix/sysv/linux/sysdep-cancel.h b/sysdeps/unix/sysv/linux/sysdep-cancel.h
>> index 821114b..86044de 100644
>> --- a/sysdeps/unix/sysv/linux/sysdep-cancel.h
>> +++ b/sysdeps/unix/sysv/linux/sysdep-cancel.h
>> @@ -30,12 +30,24 @@
>> thread check to use global variables instead of the pthread_t
>> field. */
>> #ifdef SINGLE_THREAD_BY_GLOBAL
>> +
>> +extern int __pthread_multiple_threads attribute_hidden;
>> +
>> +/* Variable set to a nonzero value either if more than one thread runs or ran,
>> + or if a single-threaded process is trying to cancel itself. See
>> + nptl/descr.h for more context on the single-threaded process case. */
>> +# define MULTIPLE_THREADS_PTR_DEF \
>> + int *__libc_multiple_threads_ptr attribute_hidden
>> +
>> +# define MULTIPLE_THREADS_PTR_SET(__ptr) \
>> + __libc_multiple_threads_ptr = (__ptr)
>> +
>> # if IS_IN (libc)
>> -extern int __libc_multiple_threads;
>> +extern int __libc_multiple_threads attribute_hidden;
>> # define SINGLE_THREAD_P \
>> __glibc_likely (__libc_multiple_threads == 0)
>> # elif IS_IN (libpthread)
>> -extern int __pthread_multiple_threads;
>> +extern int *__libc_multiple_threads_ptr attribute_hidden;
>> # define SINGLE_THREAD_P \
>> __glibc_likely (__pthread_multiple_threads == 0)
>> # elif IS_IN (librt)
>> @@ -46,7 +58,16 @@ extern int __pthread_multiple_threads;
>> /* For rtld, et cetera. */
>> # define SINGLE_THREAD_P (1)
>> # endif
>> +
>> +# define SET_SINGLE_THREAD_PTHREAD(__value) \
>> + *__libc_multiple_threads_ptr = __pthread_multiple_threads = (__value)
>> +
>> #else
>> +
>> +# define MULTIPLE_THREADS_PTR_DEF
>> +
>> +# define MULTIPLE_THREADS_PTR_SET(__ptr) __ptr
>> +
>> # if IS_IN (libc) || IS_IN (libpthread) || IS_IN (librt)
>> # define SINGLE_THREAD_P \
>> __glibc_likely (THREAD_GETMEM (THREAD_SELF, \
>> @@ -55,6 +76,8 @@ extern int __pthread_multiple_threads;
>> /* For rtld, et cetera. */
>> # define SINGLE_THREAD_P (1)
>> # endif
>> +
>> +# define SET_SINGLE_THREAD_PTHREAD(__value)
>> #endif
>>
>> #define RTLD_SINGLE_THREAD_P \
>>