Message ID | 20171015151100.GA29201@gmail.com |
---|---|
State | New |
Headers | show |
Series | Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298] | expand |
On 15/10/2017 13:11, H.J. Lu wrote: > It is incorrect to define __PTHREAD_MUTEX_HAVE_PREV to 1 only when > __WORDSIZE == 64. For x32, __PTHREAD_MUTEX_HAVE_PREV should be 1, but > it has __WORDSIZE == 32. This patch defines __PTHREAD_MUTEX_HAVE_PREV > based on __WORDSIZE only if it is undefined. __PTHREAD_MUTEX_HAVE_PREV > check is changed from "#ifdef" to "#if" to support values of 0 or 1. > > OK for master and 2.26 branch? Thanks for checking and I think changing '#ifdef' to '#if' is a good way forward, however instead of check if __PTHREAD_MUTEX_HAVE_PREV is define to redefine is based on __WORDSIZE, I think it would be better if we define it by arch-bases as other __PTHREAD_* defines. Based on your patch, I rechecked all the architectures and the expected __PTHREAD_MUTEX_HAVE_PREV and move its definition to pthreadtypes-arch.h. What do you think about the below: --- diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index ad9add8..1cc7893 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -753,7 +753,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, - offsetof (pthread_mutex_t, __data.__list.__next)); pd->robust_head.list_op_pending = NULL; -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV pd->robust_prev = &pd->robust_head; #endif pd->robust_head.list = &pd->robust_head; diff --git a/nptl/descr.h b/nptl/descr.h index c5ad0c8..c83b17b 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -169,7 +169,7 @@ struct pthread pid_t pid_ununsed; /* List of robust mutexes the thread is holding. */ -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV void *robust_prev; struct robust_list_head robust_head; diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index 2921607..869e926 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -297,7 +297,7 @@ __pthread_initialize_minimal_internal (void) /* Initialize the robust mutex data. */ { -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV pd->robust_prev = &pd->robust_head; #endif pd->robust_head.list = &pd->robust_head; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 992331e..51ae60d 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -518,7 +518,7 @@ START_THREAD_DEFN #ifndef __ASSUME_SET_ROBUST_LIST /* If this thread has any robust mutexes locked, handle them now. */ -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV void *robust = pd->robust_head.list; # else __pthread_slist_t *robust = pd->robust_list.__next; @@ -536,7 +536,7 @@ START_THREAD_DEFN __list.__next)); robust = *((void **) robust); -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV this->__list.__prev = NULL; # endif this->__list.__next = NULL; diff --git a/sysdeps/aarch64/nptl/bits/pthreadtypes-arch.h b/sysdeps/aarch64/nptl/bits/pthreadtypes-arch.h index d13a75d..3c43707 100644 --- a/sysdeps/aarch64/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/aarch64/nptl/bits/pthreadtypes-arch.h @@ -45,6 +45,11 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#ifdef __ILP32__ +#define __PTHREAD_MUTEX_HAVE_PREV 0 +#else +#define __PTHREAD_MUTEX_HAVE_PREV 1 +#endif #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/alpha/nptl/bits/pthreadtypes-arch.h b/sysdeps/alpha/nptl/bits/pthreadtypes-arch.h index b6f6cb1..687028a 100644 --- a/sysdeps/alpha/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/alpha/nptl/bits/pthreadtypes-arch.h @@ -33,6 +33,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 1 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/arm/nptl/bits/pthreadtypes-arch.h b/sysdeps/arm/nptl/bits/pthreadtypes-arch.h index 3f9eca4..fac38ac 100644 --- a/sysdeps/arm/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/arm/nptl/bits/pthreadtypes-arch.h @@ -34,6 +34,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/hppa/nptl/bits/pthreadtypes-arch.h b/sysdeps/hppa/nptl/bits/pthreadtypes-arch.h index c158562..7ad0ce7 100644 --- a/sysdeps/hppa/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/hppa/nptl/bits/pthreadtypes-arch.h @@ -48,6 +48,7 @@ pthread_mutex_t is larger than Linuxthreads. */ #define __PTHREAD_COMPAT_PADDING_END int __reserved[2]; #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT __attribute__ ((__aligned__(16))) #define __ONCE_ALIGNMENT diff --git a/sysdeps/ia64/nptl/bits/pthreadtypes-arch.h b/sysdeps/ia64/nptl/bits/pthreadtypes-arch.h index 631cb33..d456aba 100644 --- a/sysdeps/ia64/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/ia64/nptl/bits/pthreadtypes-arch.h @@ -33,6 +33,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 1 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/m68k/nptl/bits/pthreadtypes-arch.h b/sysdeps/m68k/nptl/bits/pthreadtypes-arch.h index 845b9e6..25e93be 100644 --- a/sysdeps/m68k/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/m68k/nptl/bits/pthreadtypes-arch.h @@ -35,6 +35,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT __attribute__ ((__aligned__ (4))) #define __ONCE_ALIGNMENT __attribute__ ((__aligned__ (4))) diff --git a/sysdeps/microblaze/nptl/bits/pthreadtypes-arch.h b/sysdeps/microblaze/nptl/bits/pthreadtypes-arch.h index d687e2c..b9130b6 100644 --- a/sysdeps/microblaze/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/microblaze/nptl/bits/pthreadtypes-arch.h @@ -35,6 +35,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/mips/nptl/bits/pthreadtypes-arch.h b/sysdeps/mips/nptl/bits/pthreadtypes-arch.h index 6aa1bda..c5cc1f8 100644 --- a/sysdeps/mips/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/mips/nptl/bits/pthreadtypes-arch.h @@ -42,6 +42,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV (_MIPS_SIM == _ABI64) #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/nios2/nptl/bits/pthreadtypes-arch.h b/sysdeps/nios2/nptl/bits/pthreadtypes-arch.h index e2732f9..431da41 100644 --- a/sysdeps/nios2/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/nios2/nptl/bits/pthreadtypes-arch.h @@ -35,6 +35,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h index 68b82b6..5dfe01e 100644 --- a/sysdeps/nptl/bits/thread-shared-types.h +++ b/sysdeps/nptl/bits/thread-shared-types.h @@ -58,8 +58,7 @@ #include <bits/pthreadtypes-arch.h> /* Common definition of pthread_mutex_t. */ - -#if __WORDSIZE == 64 +#if __PTHREAD_MUTEX_HAVE_PREV typedef struct __pthread_internal_list { struct __pthread_internal_list *__prev; @@ -74,7 +73,7 @@ typedef struct __pthread_internal_slist /* Lock elision support. */ #if __PTHREAD_MUTEX_LOCK_ELISION -# if __WORDSIZE == 64 +# if __PTHREAD_MUTEX_HAVE_PREV # define __PTHREAD_SPINS_DATA \ short __spins; \ short __elision @@ -101,17 +100,16 @@ struct __pthread_mutex_s int __lock __LOCK_ALIGNMENT; unsigned int __count; int __owner; -#if __WORDSIZE == 64 +#if __PTHREAD_MUTEX_HAVE_PREV unsigned int __nusers; #endif /* KIND must stay at this position in the structure to maintain binary compatibility with static initializers. */ int __kind; __PTHREAD_COMPAT_PADDING_MID -#if __WORDSIZE == 64 +#if __PTHREAD_MUTEX_HAVE_PREV __PTHREAD_SPINS_DATA; __pthread_list_t __list; -# define __PTHREAD_MUTEX_HAVE_PREV 1 #else unsigned int __nusers; __extension__ union diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c index 4bb87e2..48676c2 100644 --- a/sysdeps/nptl/fork.c +++ b/sysdeps/nptl/fork.c @@ -166,7 +166,7 @@ __libc_fork (void) inherit the correct value from the parent. We do not need to clear the pending operation because it must have been zero when fork was called. */ -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV self->robust_prev = &self->robust_head; # endif self->robust_head.list = &self->robust_head; diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index 632ea7b..2b2b386 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -83,7 +83,7 @@ enum #endif -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV # define PTHREAD_MUTEX_INITIALIZER \ { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } # ifdef __USE_GNU diff --git a/sysdeps/powerpc/nptl/bits/pthreadtypes-arch.h b/sysdeps/powerpc/nptl/bits/pthreadtypes-arch.h index f29119b..d56897b 100644 --- a/sysdeps/powerpc/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/powerpc/nptl/bits/pthreadtypes-arch.h @@ -42,6 +42,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 1 +#define __PTHREAD_MUTEX_HAVE_PREV (__WORDSIZE == 64) #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h index 3a9ac57..e4c164a 100644 --- a/sysdeps/s390/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/s390/nptl/bits/pthreadtypes-arch.h @@ -45,6 +45,7 @@ #else #define __PTHREAD_MUTEX_LOCK_ELISION 0 #endif +#define __PTHREAD_MUTEX_HAVE_PREV (__WORDSIZE == 64) #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/sh/nptl/bits/pthreadtypes-arch.h b/sysdeps/sh/nptl/bits/pthreadtypes-arch.h index b2615fe..711e7bb 100644 --- a/sysdeps/sh/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/sh/nptl/bits/pthreadtypes-arch.h @@ -34,6 +34,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV 0 #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/sparc/nptl/bits/pthreadtypes-arch.h b/sysdeps/sparc/nptl/bits/pthreadtypes-arch.h index 1e188cf..6e7f47a 100644 --- a/sysdeps/sparc/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/sparc/nptl/bits/pthreadtypes-arch.h @@ -43,6 +43,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV (__WORDSIZE == 64) #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/tile/nptl/bits/pthreadtypes-arch.h b/sysdeps/tile/nptl/bits/pthreadtypes-arch.h index 145ee42..c753a9b 100644 --- a/sysdeps/tile/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/tile/nptl/bits/pthreadtypes-arch.h @@ -43,6 +43,7 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 0 +#define __PTHREAD_MUTEX_HAVE_PREV (__WORDSIZE == 64) #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT diff --git a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h index fd86806..3592667 100644 --- a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h @@ -35,6 +35,7 @@ # define __SIZEOF_PTHREAD_BARRIER_T 20 # endif #else +# define __PTHREAD_MUTEX_HAVE_PREV 0 # define __SIZEOF_PTHREAD_MUTEX_T 24 # define __SIZEOF_PTHREAD_ATTR_T 36 # define __SIZEOF_PTHREAD_MUTEX_T 24 @@ -51,6 +52,11 @@ #define __PTHREAD_COMPAT_PADDING_MID #define __PTHREAD_COMPAT_PADDING_END #define __PTHREAD_MUTEX_LOCK_ELISION 1 +#ifdef __x86_64__ +# define __PTHREAD_MUTEX_HAVE_PREV 1 +#else +# define __PTHREAD_MUTEX_HAVE_PREV 0 +#endif #define __LOCK_ALIGNMENT #define __ONCE_ALIGNMENT
On Okt 16 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h > index 68b82b6..5dfe01e 100644 > --- a/sysdeps/nptl/bits/thread-shared-types.h > +++ b/sysdeps/nptl/bits/thread-shared-types.h > @@ -58,8 +58,7 @@ > #include <bits/pthreadtypes-arch.h> > > /* Common definition of pthread_mutex_t. */ > - > -#if __WORDSIZE == 64 > +#if __PTHREAD_MUTEX_HAVE_PREV > typedef struct __pthread_internal_list > { > struct __pthread_internal_list *__prev; > @@ -74,7 +73,7 @@ typedef struct __pthread_internal_slist > > /* Lock elision support. */ > #if __PTHREAD_MUTEX_LOCK_ELISION > -# if __WORDSIZE == 64 > +# if __PTHREAD_MUTEX_HAVE_PREV > # define __PTHREAD_SPINS_DATA \ > short __spins; \ > short __elision > @@ -101,17 +100,16 @@ struct __pthread_mutex_s > int __lock __LOCK_ALIGNMENT; > unsigned int __count; > int __owner; > -#if __WORDSIZE == 64 > +#if __PTHREAD_MUTEX_HAVE_PREV > unsigned int __nusers; > #endif The name doesn't really fit here. There is nothing matching prev in __nusers or __PTHREAD_SPINS_DATA. Andreas.
On Mon, Oct 16, 2017 at 7:31 AM, Andreas Schwab <schwab@suse.de> wrote: > On Okt 16 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > >> diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h >> index 68b82b6..5dfe01e 100644 >> --- a/sysdeps/nptl/bits/thread-shared-types.h >> +++ b/sysdeps/nptl/bits/thread-shared-types.h >> @@ -58,8 +58,7 @@ >> #include <bits/pthreadtypes-arch.h> >> >> /* Common definition of pthread_mutex_t. */ >> - >> -#if __WORDSIZE == 64 >> +#if __PTHREAD_MUTEX_HAVE_PREV >> typedef struct __pthread_internal_list >> { >> struct __pthread_internal_list *__prev; >> @@ -74,7 +73,7 @@ typedef struct __pthread_internal_slist >> >> /* Lock elision support. */ >> #if __PTHREAD_MUTEX_LOCK_ELISION >> -# if __WORDSIZE == 64 >> +# if __PTHREAD_MUTEX_HAVE_PREV >> # define __PTHREAD_SPINS_DATA \ >> short __spins; \ >> short __elision >> @@ -101,17 +100,16 @@ struct __pthread_mutex_s >> int __lock __LOCK_ALIGNMENT; >> unsigned int __count; >> int __owner; >> -#if __WORDSIZE == 64 >> +#if __PTHREAD_MUTEX_HAVE_PREV >> unsigned int __nusers; >> #endif > > The name doesn't really fit here. There is nothing matching prev in > __nusers or __PTHREAD_SPINS_DATA. > True. But they are tied together.
On Mon, Oct 16, 2017 at 6:50 AM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > On 15/10/2017 13:11, H.J. Lu wrote: >> It is incorrect to define __PTHREAD_MUTEX_HAVE_PREV to 1 only when >> __WORDSIZE == 64. For x32, __PTHREAD_MUTEX_HAVE_PREV should be 1, but >> it has __WORDSIZE == 32. This patch defines __PTHREAD_MUTEX_HAVE_PREV >> based on __WORDSIZE only if it is undefined. __PTHREAD_MUTEX_HAVE_PREV >> check is changed from "#ifdef" to "#if" to support values of 0 or 1. >> >> OK for master and 2.26 branch? > > Thanks for checking and I think changing '#ifdef' to '#if' is a good > way forward, however instead of check if __PTHREAD_MUTEX_HAVE_PREV is > define to redefine is based on __WORDSIZE, I think it would be better > if we define it by arch-bases as other __PTHREAD_* defines. > > Based on your patch, I rechecked all the architectures and the expected > __PTHREAD_MUTEX_HAVE_PREV and move its definition to pthreadtypes-arch.h. > What do you think about the below: See my comments below. > diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h > index 68b82b6..5dfe01e 100644 > --- a/sysdeps/nptl/bits/thread-shared-types.h > +++ b/sysdeps/nptl/bits/thread-shared-types.h > @@ -58,8 +58,7 @@ > #include <bits/pthreadtypes-arch.h> > > /* Common definition of pthread_mutex_t. */ > - > -#if __WORDSIZE == 64 > +#if __PTHREAD_MUTEX_HAVE_PREV Do we need to issue an error if __PTHREAD_MUTEX_HAVE_PREV isn't defined? > diff --git a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h > index fd86806..3592667 100644 > --- a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h > +++ b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h > @@ -35,6 +35,7 @@ > # define __SIZEOF_PTHREAD_BARRIER_T 20 > # endif > #else > +# define __PTHREAD_MUTEX_HAVE_PREV 0 ^^^^^^^^^^^^^^^^^^^^^^^ No need for it. > # define __SIZEOF_PTHREAD_MUTEX_T 24 > # define __SIZEOF_PTHREAD_ATTR_T 36 > # define __SIZEOF_PTHREAD_MUTEX_T 24 > @@ -51,6 +52,11 @@ > #define __PTHREAD_COMPAT_PADDING_MID > #define __PTHREAD_COMPAT_PADDING_END > #define __PTHREAD_MUTEX_LOCK_ELISION 1 > +#ifdef __x86_64__ > +# define __PTHREAD_MUTEX_HAVE_PREV 1 > +#else > +# define __PTHREAD_MUTEX_HAVE_PREV 0 > +#endif > > #define __LOCK_ALIGNMENT > #define __ONCE_ALIGNMENT
On 16/10/2017 13:00, H.J. Lu wrote: > On Mon, Oct 16, 2017 at 6:50 AM, Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> On 15/10/2017 13:11, H.J. Lu wrote: >>> It is incorrect to define __PTHREAD_MUTEX_HAVE_PREV to 1 only when >>> __WORDSIZE == 64. For x32, __PTHREAD_MUTEX_HAVE_PREV should be 1, but >>> it has __WORDSIZE == 32. This patch defines __PTHREAD_MUTEX_HAVE_PREV >>> based on __WORDSIZE only if it is undefined. __PTHREAD_MUTEX_HAVE_PREV >>> check is changed from "#ifdef" to "#if" to support values of 0 or 1. >>> >>> OK for master and 2.26 branch? >> >> Thanks for checking and I think changing '#ifdef' to '#if' is a good >> way forward, however instead of check if __PTHREAD_MUTEX_HAVE_PREV is >> define to redefine is based on __WORDSIZE, I think it would be better >> if we define it by arch-bases as other __PTHREAD_* defines. >> >> Based on your patch, I rechecked all the architectures and the expected >> __PTHREAD_MUTEX_HAVE_PREV and move its definition to pthreadtypes-arch.h. >> What do you think about the below: > > See my comments below. > > >> diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h >> index 68b82b6..5dfe01e 100644 >> --- a/sysdeps/nptl/bits/thread-shared-types.h >> +++ b/sysdeps/nptl/bits/thread-shared-types.h >> @@ -58,8 +58,7 @@ >> #include <bits/pthreadtypes-arch.h> >> >> /* Common definition of pthread_mutex_t. */ >> - >> -#if __WORDSIZE == 64 >> +#if __PTHREAD_MUTEX_HAVE_PREV > > Do we need to issue an error if __PTHREAD_MUTEX_HAVE_PREV > isn't defined? > I think the build error is enough to indicate the architecture should define it. > >> diff --git a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h >> index fd86806..3592667 100644 >> --- a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h >> +++ b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h >> @@ -35,6 +35,7 @@ >> # define __SIZEOF_PTHREAD_BARRIER_T 20 >> # endif >> #else >> +# define __PTHREAD_MUTEX_HAVE_PREV 0 > ^^^^^^^^^^^^^^^^^^^^^^^ No need for it. > >> # define __SIZEOF_PTHREAD_MUTEX_T 24 >> # define __SIZEOF_PTHREAD_ATTR_T 36 >> # define __SIZEOF_PTHREAD_MUTEX_T 24 >> @@ -51,6 +52,11 @@ >> #define __PTHREAD_COMPAT_PADDING_MID >> #define __PTHREAD_COMPAT_PADDING_END >> #define __PTHREAD_MUTEX_LOCK_ELISION 1 >> +#ifdef __x86_64__ >> +# define __PTHREAD_MUTEX_HAVE_PREV 1 >> +#else >> +# define __PTHREAD_MUTEX_HAVE_PREV 0 >> +#endif >> >> #define __LOCK_ALIGNMENT >> #define __ONCE_ALIGNMENT Ack.
On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > On Mon, Oct 16, 2017 at 7:31 AM, Andreas Schwab <schwab@suse.de> wrote: >> On Okt 16 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: >> >>> diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h >>> index 68b82b6..5dfe01e 100644 >>> --- a/sysdeps/nptl/bits/thread-shared-types.h >>> +++ b/sysdeps/nptl/bits/thread-shared-types.h >>> @@ -58,8 +58,7 @@ >>> #include <bits/pthreadtypes-arch.h> >>> >>> /* Common definition of pthread_mutex_t. */ >>> - >>> -#if __WORDSIZE == 64 >>> +#if __PTHREAD_MUTEX_HAVE_PREV >>> typedef struct __pthread_internal_list >>> { >>> struct __pthread_internal_list *__prev; >>> @@ -74,7 +73,7 @@ typedef struct __pthread_internal_slist >>> >>> /* Lock elision support. */ >>> #if __PTHREAD_MUTEX_LOCK_ELISION >>> -# if __WORDSIZE == 64 >>> +# if __PTHREAD_MUTEX_HAVE_PREV >>> # define __PTHREAD_SPINS_DATA \ >>> short __spins; \ >>> short __elision >>> @@ -101,17 +100,16 @@ struct __pthread_mutex_s >>> int __lock __LOCK_ALIGNMENT; >>> unsigned int __count; >>> int __owner; >>> -#if __WORDSIZE == 64 >>> +#if __PTHREAD_MUTEX_HAVE_PREV >>> unsigned int __nusers; >>> #endif >> >> The name doesn't really fit here. There is nothing matching prev in >> __nusers or __PTHREAD_SPINS_DATA. >> > > True. But they are tied together. Are they? They look rather unrelated, or only related by chance. Andreas.
On Mon, Oct 16, 2017 at 8:25 AM, Andreas Schwab <schwab@suse.de> wrote: > On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> On Mon, Oct 16, 2017 at 7:31 AM, Andreas Schwab <schwab@suse.de> wrote: >>> On Okt 16 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: >>> >>>> diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h >>>> index 68b82b6..5dfe01e 100644 >>>> --- a/sysdeps/nptl/bits/thread-shared-types.h >>>> +++ b/sysdeps/nptl/bits/thread-shared-types.h >>>> @@ -58,8 +58,7 @@ >>>> #include <bits/pthreadtypes-arch.h> >>>> >>>> /* Common definition of pthread_mutex_t. */ >>>> - >>>> -#if __WORDSIZE == 64 >>>> +#if __PTHREAD_MUTEX_HAVE_PREV >>>> typedef struct __pthread_internal_list >>>> { >>>> struct __pthread_internal_list *__prev; >>>> @@ -74,7 +73,7 @@ typedef struct __pthread_internal_slist >>>> >>>> /* Lock elision support. */ >>>> #if __PTHREAD_MUTEX_LOCK_ELISION >>>> -# if __WORDSIZE == 64 >>>> +# if __PTHREAD_MUTEX_HAVE_PREV >>>> # define __PTHREAD_SPINS_DATA \ >>>> short __spins; \ >>>> short __elision >>>> @@ -101,17 +100,16 @@ struct __pthread_mutex_s >>>> int __lock __LOCK_ALIGNMENT; >>>> unsigned int __count; >>>> int __owner; >>>> -#if __WORDSIZE == 64 >>>> +#if __PTHREAD_MUTEX_HAVE_PREV >>>> unsigned int __nusers; >>>> #endif >>> >>> The name doesn't really fit here. There is nothing matching prev in >>> __nusers or __PTHREAD_SPINS_DATA. >>> >> >> True. But they are tied together. > > Are they? They look rather unrelated, or only related by chance. > > Andreas. > Glibc 2.25 has #ifdef __x86_64__ typedef struct __pthread_internal_list { struct __pthread_internal_list *__prev; struct __pthread_internal_list *__next; } __pthread_list_t; #else typedef struct __pthread_internal_slist { struct __pthread_internal_slist *__next; } __pthread_slist_t; #endif /* Data structures for mutex handling. The structure of the attribute type is not exposed on purpose. */ typedef union { struct __pthread_mutex_s { int __lock; unsigned int __count; int __owner; #ifdef __x86_64__ unsigned int __nusers; #endif /* KIND must stay at this position in the structure to maintain binary compatibility with static initializers. */ int __kind; #ifdef __x86_64__ short __spins; short __elision; __pthread_list_t __list; # define __PTHREAD_MUTEX_HAVE_PREV 1 /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */ # define __PTHREAD_SPINS 0, 0 #else unsigned int __nusers; __extension__ union { struct { short __espins; short __elision; # define __spins __elision_data.__espins # define __elision __elision_data.__elision # define __PTHREAD_SPINS { 0, 0 } } __elision_data; __pthread_slist_t __list; }; #endif } __data; char __size[__SIZEOF_PTHREAD_MUTEX_T]; long int __align; } pthread_mutex_t; Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS.
On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties > __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS. That's what I would call related by chance. The position of __nusers differs only because __kind must not be moved, and that difference existed already before the introduction of __prev and __next. Andreas.
On Mon, Oct 16, 2017 at 9:05 AM, Andreas Schwab <schwab@suse.de> wrote: > On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties >> __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS. > > That's what I would call related by chance. The position of __nusers > differs only because __kind must not be moved, and that difference > existed already before the introduction of __prev and __next. > By chance or not, they can't be changed on x86.
On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > On Mon, Oct 16, 2017 at 9:05 AM, Andreas Schwab <schwab@suse.de> wrote: >> On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >>> Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties >>> __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS. >> >> That's what I would call related by chance. The position of __nusers >> differs only because __kind must not be moved, and that difference >> existed already before the introduction of __prev and __next. >> > > By chance or not, they can't be changed on x86. This file is used by all architectures. Andreas.
On Mon, Oct 16, 2017 at 10:39 AM, Andreas Schwab <schwab@suse.de> wrote: > On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> On Mon, Oct 16, 2017 at 9:05 AM, Andreas Schwab <schwab@suse.de> wrote: >>> On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>> >>>> Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties >>>> __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS. >>> >>> That's what I would call related by chance. The position of __nusers >>> differs only because __kind must not be moved, and that difference >>> existed already before the introduction of __prev and __next. >>> >> >> By chance or not, they can't be changed on x86. > > This file is used by all architectures. > > Andreas. > The current code has /* Common definition of pthread_mutex_t. */ #if __WORDSIZE == 64 typedef struct __pthread_internal_list { struct __pthread_internal_list *__prev; struct __pthread_internal_list *__next; } __pthread_list_t; #else typedef struct __pthread_internal_slist { struct __pthread_internal_slist *__next; } __pthread_slist_t; #endif /* Lock elision support. */ #if __PTHREAD_MUTEX_LOCK_ELISION # if __WORDSIZE == 64 # define __PTHREAD_SPINS_DATA \ short __spins; \ short __elision # define __PTHREAD_SPINS 0, 0 # else # define __PTHREAD_SPINS_DATA \ struct \ { \ short __espins; \ short __eelision; \ } __elision_data # define __PTHREAD_SPINS { 0, 0 } # define __spins __elision_data.__espins # define __elision __elision_data.__eelision # endif #else # define __PTHREAD_SPINS_DATA int __spins /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */ # define __PTHREAD_SPINS 0 #endif struct __pthread_mutex_s { int __lock __LOCK_ALIGNMENT; unsigned int __count; int __owner; #if __WORDSIZE == 64 unsigned int __nusers; #endif /* KIND must stay at this position in the structure to maintain binary compatibility with static initializers. */ int __kind; __PTHREAD_COMPAT_PADDING_MID #if __WORDSIZE == 64 __PTHREAD_SPINS_DATA; __pthread_list_t __list; # define __PTHREAD_MUTEX_HAVE_PREV 1 #else unsigned int __nusers; __extension__ union { __PTHREAD_SPINS_DATA; __pthread_slist_t __list; }; #endif __PTHREAD_COMPAT_PADDING_END }; __PTHREAD_MUTEX_HAVE_PREV is the same as __WORDSIZE == 64, which ties it together with __nusers and __PTHREAD_SPINS_DATA. We can't change it for existing targets.
On 16/10/2017 16:00, H.J. Lu wrote: > On Mon, Oct 16, 2017 at 10:39 AM, Andreas Schwab <schwab@suse.de> wrote: >> On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >>> On Mon, Oct 16, 2017 at 9:05 AM, Andreas Schwab <schwab@suse.de> wrote: >>>> On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>>> >>>>> Here __x86_64__ == __PTHREAD_MUTEX_HAVE_PREV, which ties >>>>> __PTHREAD_MUTEX_HAVE_PREV with __nusers and __PTHREAD_SPINS. >>>> >>>> That's what I would call related by chance. The position of __nusers >>>> differs only because __kind must not be moved, and that difference >>>> existed already before the introduction of __prev and __next. >>>> >>> >>> By chance or not, they can't be changed on x86. >> >> This file is used by all architectures. >> >> Andreas. >> > > The current code has > > /* Common definition of pthread_mutex_t. */ > > #if __WORDSIZE == 64 > typedef struct __pthread_internal_list > { > struct __pthread_internal_list *__prev; > struct __pthread_internal_list *__next; > } __pthread_list_t; > #else > typedef struct __pthread_internal_slist > { > struct __pthread_internal_slist *__next; > } __pthread_slist_t; > #endif > > /* Lock elision support. */ > #if __PTHREAD_MUTEX_LOCK_ELISION > # if __WORDSIZE == 64 > # define __PTHREAD_SPINS_DATA \ > short __spins; \ > short __elision > # define __PTHREAD_SPINS 0, 0 > # else > # define __PTHREAD_SPINS_DATA \ > struct \ > { \ > short __espins; \ > short __eelision; \ > } __elision_data > # define __PTHREAD_SPINS { 0, 0 } > # define __spins __elision_data.__espins > # define __elision __elision_data.__eelision > # endif > #else > # define __PTHREAD_SPINS_DATA int __spins > /* Mutex __spins initializer used by PTHREAD_MUTEX_INITIALIZER. */ > # define __PTHREAD_SPINS 0 > #endif > > struct __pthread_mutex_s > { > int __lock __LOCK_ALIGNMENT; > unsigned int __count; > int __owner; > #if __WORDSIZE == 64 > unsigned int __nusers; > #endif > /* KIND must stay at this position in the structure to maintain > binary compatibility with static initializers. */ > int __kind; > __PTHREAD_COMPAT_PADDING_MID > #if __WORDSIZE == 64 > __PTHREAD_SPINS_DATA; > __pthread_list_t __list; > # define __PTHREAD_MUTEX_HAVE_PREV 1 > #else > unsigned int __nusers; > __extension__ union > { > __PTHREAD_SPINS_DATA; > __pthread_slist_t __list; > }; > #endif > __PTHREAD_COMPAT_PADDING_END > }; > > __PTHREAD_MUTEX_HAVE_PREV is the same as __WORDSIZE == 64, > which ties it together with __nusers and __PTHREAD_SPINS_DATA. We > can't change it for existing targets. I agree, in the end __PTHREAD_MUTEX_HAVE_PREV will define which internal layout __pthread_mutex_s will use to place __nusers and __list. We can add an extra architecture define to set the __nusers/__list layout along with __PTHREAD_MUTEX_HAVE_PREV, however this will only add complexity. I plan to send an update version of this patch and I think it worth add a comment about __PTHREAD_MUTEX_HAVE_PREV and the internal layout coupling.
On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> __PTHREAD_MUTEX_HAVE_PREV is the same as __WORDSIZE == 64,
Just because it happens to match now does not mean that all new
architectures want to follow the idiosyncrasies of linuxthreads
compatibility.
Andreas.
On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> By chance or not, they can't be changed on x86.
Who said that x86 must change??
Andreas.
On Tue, Oct 17, 2017 at 12:20 AM, Andreas Schwab <schwab@suse.de> wrote: > On Okt 16 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> __PTHREAD_MUTEX_HAVE_PREV is the same as __WORDSIZE == 64, > > Just because it happens to match now does not mean that all new > architectures want to follow the idiosyncrasies of linuxthreads > compatibility. > 1. We first add a test to check offsets of all fields in struct __pthread_mutex_s. 2. Define __PTHREAD_SPINS_DATA_IN_STRUCT to control __PTHREAD_SPINS_DATA 3. __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to put __nusers.
On Okt 17 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > 2. Define __PTHREAD_SPINS_DATA_IN_STRUCT to control > __PTHREAD_SPINS_DATA > 3. __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to > put __nusers. Yes, this looks like the right approach. Though I would define the macros in a way so that defining them to 0 results in the preferred layout for future architectures (thus __PTHREAD_MUTEX_NUSERS_AFTER_KIND instead of __PTHREAD_MUTEX_NUSERS_BEFORE_KIND). __PTHREAD_SPINS_DATA_IN_STRUCT could then imply __PTHREAD_MUTEX_HAVE_PREV as before, since both are only needed due to space constraints for linuxthreads compatibility (perhaps rename it to __PTHREAD_MUTEX_USE_UNION?). Andreas.
On 18/10/2017 06:21, Andreas Schwab wrote: > On Okt 17 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> 2. Define __PTHREAD_SPINS_DATA_IN_STRUCT to control >> __PTHREAD_SPINS_DATA >> 3. __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to >> put __nusers. > > Yes, this looks like the right approach. Though I would define the > macros in a way so that defining them to 0 results in the preferred > layout for future architectures (thus __PTHREAD_MUTEX_NUSERS_AFTER_KIND > instead of __PTHREAD_MUTEX_NUSERS_BEFORE_KIND). > __PTHREAD_SPINS_DATA_IN_STRUCT could then imply > __PTHREAD_MUTEX_HAVE_PREV as before, since both are only needed due to > space constraints for linuxthreads compatibility (perhaps rename it to > __PTHREAD_MUTEX_USE_UNION?). > > Andreas. > Alright, I will adapt my first submission to follow this.
On Wed, Oct 18, 2017 at 4:06 AM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > On 18/10/2017 06:21, Andreas Schwab wrote: >> On Okt 17 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >>> 2. Define __PTHREAD_SPINS_DATA_IN_STRUCT to control >>> __PTHREAD_SPINS_DATA >>> 3. __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to >>> put __nusers. >> >> Yes, this looks like the right approach. Though I would define the >> macros in a way so that defining them to 0 results in the preferred >> layout for future architectures (thus __PTHREAD_MUTEX_NUSERS_AFTER_KIND >> instead of __PTHREAD_MUTEX_NUSERS_BEFORE_KIND). >> __PTHREAD_SPINS_DATA_IN_STRUCT could then imply >> __PTHREAD_MUTEX_HAVE_PREV as before, since both are only needed due to >> space constraints for linuxthreads compatibility (perhaps rename it to >> __PTHREAD_MUTEX_USE_UNION?). >> >> Andreas. >> > > Alright, I will adapt my first submission to follow this. Please use a separate patch to check offsets which should be backported to 2.25 branch.
On 18/10/2017 10:35, H.J. Lu wrote: > On Wed, Oct 18, 2017 at 4:06 AM, Adhemerval Zanella > <adhemerval.zanella@linaro.org> wrote: >> >> >> On 18/10/2017 06:21, Andreas Schwab wrote: >>> On Okt 17 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>> >>>> 2. Define __PTHREAD_SPINS_DATA_IN_STRUCT to control >>>> __PTHREAD_SPINS_DATA >>>> 3. __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to >>>> put __nusers. >>> >>> Yes, this looks like the right approach. Though I would define the >>> macros in a way so that defining them to 0 results in the preferred >>> layout for future architectures (thus __PTHREAD_MUTEX_NUSERS_AFTER_KIND >>> instead of __PTHREAD_MUTEX_NUSERS_BEFORE_KIND). >>> __PTHREAD_SPINS_DATA_IN_STRUCT could then imply >>> __PTHREAD_MUTEX_HAVE_PREV as before, since both are only needed due to >>> space constraints for linuxthreads compatibility (perhaps rename it to >>> __PTHREAD_MUTEX_USE_UNION?). >>> >>> Andreas. >>> >> >> Alright, I will adapt my first submission to follow this. > > Please use a separate patch to check offsets which should be backported > to 2.25 branch. > Yes, My idea is to follow your suggestion: 1. One patch to check offsets of all relevant fields in struct __pthread_mutex_s. 2. Another patch to define __PTHREAD_SPINS_DATA_IN_STRUCT to control __PTHREAD_SPINS_DATA 3. Another to define __PTHREAD_MUTEX_NUSERS_BEFORE_KIND to control where to put __nusers (not sure if it worth to bind 2. and 3. together).
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c index ad9add8d2a..1cc7893195 100644 --- a/nptl/allocatestack.c +++ b/nptl/allocatestack.c @@ -753,7 +753,7 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp, - offsetof (pthread_mutex_t, __data.__list.__next)); pd->robust_head.list_op_pending = NULL; -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV pd->robust_prev = &pd->robust_head; #endif pd->robust_head.list = &pd->robust_head; diff --git a/nptl/descr.h b/nptl/descr.h index c5ad0c8dba..c83b17b674 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -169,7 +169,7 @@ struct pthread pid_t pid_ununsed; /* List of robust mutexes the thread is holding. */ -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV void *robust_prev; struct robust_list_head robust_head; diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c index 29216077a2..869e926f17 100644 --- a/nptl/nptl-init.c +++ b/nptl/nptl-init.c @@ -297,7 +297,7 @@ __pthread_initialize_minimal_internal (void) /* Initialize the robust mutex data. */ { -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV pd->robust_prev = &pd->robust_head; #endif pd->robust_head.list = &pd->robust_head; diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 992331e280..51ae60dfca 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -518,7 +518,7 @@ START_THREAD_DEFN #ifndef __ASSUME_SET_ROBUST_LIST /* If this thread has any robust mutexes locked, handle them now. */ -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV void *robust = pd->robust_head.list; # else __pthread_slist_t *robust = pd->robust_list.__next; @@ -536,7 +536,7 @@ START_THREAD_DEFN __list.__next)); robust = *((void **) robust); -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV this->__list.__prev = NULL; # endif this->__list.__next = NULL; diff --git a/sysdeps/nptl/bits/thread-shared-types.h b/sysdeps/nptl/bits/thread-shared-types.h index 68b82b6bd6..d2c4f671f4 100644 --- a/sysdeps/nptl/bits/thread-shared-types.h +++ b/sysdeps/nptl/bits/thread-shared-types.h @@ -59,7 +59,15 @@ /* Common definition of pthread_mutex_t. */ -#if __WORDSIZE == 64 +#ifndef __PTHREAD_MUTEX_HAVE_PREV +# if __WORDSIZE == 64 +# define __PTHREAD_MUTEX_HAVE_PREV 1 +# else +# define __PTHREAD_MUTEX_HAVE_PREV 0 +# endif +#endif + +#if __PTHREAD_MUTEX_HAVE_PREV typedef struct __pthread_internal_list { struct __pthread_internal_list *__prev; @@ -74,7 +82,7 @@ typedef struct __pthread_internal_slist /* Lock elision support. */ #if __PTHREAD_MUTEX_LOCK_ELISION -# if __WORDSIZE == 64 +# if __PTHREAD_MUTEX_HAVE_PREV # define __PTHREAD_SPINS_DATA \ short __spins; \ short __elision @@ -101,17 +109,16 @@ struct __pthread_mutex_s int __lock __LOCK_ALIGNMENT; unsigned int __count; int __owner; -#if __WORDSIZE == 64 +#if __PTHREAD_MUTEX_HAVE_PREV unsigned int __nusers; #endif /* KIND must stay at this position in the structure to maintain binary compatibility with static initializers. */ int __kind; __PTHREAD_COMPAT_PADDING_MID -#if __WORDSIZE == 64 +#if __PTHREAD_MUTEX_HAVE_PREV __PTHREAD_SPINS_DATA; __pthread_list_t __list; -# define __PTHREAD_MUTEX_HAVE_PREV 1 #else unsigned int __nusers; __extension__ union diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c index 4bb87e2331..48676c2f48 100644 --- a/sysdeps/nptl/fork.c +++ b/sysdeps/nptl/fork.c @@ -166,7 +166,7 @@ __libc_fork (void) inherit the correct value from the parent. We do not need to clear the pending operation because it must have been zero when fork was called. */ -# ifdef __PTHREAD_MUTEX_HAVE_PREV +# if __PTHREAD_MUTEX_HAVE_PREV self->robust_prev = &self->robust_head; # endif self->robust_head.list = &self->robust_head; diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index 632ea7bc36..2b2b386ab3 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -83,7 +83,7 @@ enum #endif -#ifdef __PTHREAD_MUTEX_HAVE_PREV +#if __PTHREAD_MUTEX_HAVE_PREV # define PTHREAD_MUTEX_INITIALIZER \ { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } } # ifdef __USE_GNU diff --git a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h index fd86806800..2446d8d88a 100644 --- a/sysdeps/x86/nptl/bits/pthreadtypes-arch.h +++ b/sysdeps/x86/nptl/bits/pthreadtypes-arch.h @@ -21,6 +21,7 @@ #include <bits/wordsize.h> #ifdef __x86_64__ +# define __PTHREAD_MUTEX_HAVE_PREV 1 # if __WORDSIZE == 64 # define __SIZEOF_PTHREAD_MUTEX_T 40 # define __SIZEOF_PTHREAD_ATTR_T 56 @@ -35,6 +36,7 @@ # define __SIZEOF_PTHREAD_BARRIER_T 20 # endif #else +# define __PTHREAD_MUTEX_HAVE_PREV 0 # define __SIZEOF_PTHREAD_MUTEX_T 24 # define __SIZEOF_PTHREAD_ATTR_T 36 # define __SIZEOF_PTHREAD_MUTEX_T 24