diff mbox

Remove __PTHREAD_MUTEX_HAVE_ELISION undefined warning

Message ID 533338C4.5030107@linux.vnet.ibm.com
State Committed
Delegated to: Carlos O'Donell
Headers show

Commit Message

Adhemerval Zanella Netto March 26, 2014, 8:29 p.m. UTC
On 26-03-2014 14:50, Roland McGrath wrote:
> I don't understand what "support for 64 bits" or "support for 32 bits"
> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
> is really just about the x86-private layout of pthread_mutex_t.  It seems
> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
>
> That can be a separate cleanup if you want, and others may want to kibitz.
> But that might involve dropping the header you're adding here, so maybe you
> want to just resolve it now.
>
> If you want to go ahead with this change, then it's OK with the other nits
> above and this comment rewritten to describe concretely what the macro
> means.  In actual usage so far, it doesn't actually mean anything about
> elision support per se.  It just means something about how the fields of
> pthread_mutex_t are structured and hence what the initializer must look like.
> If that's all it's for, it should be made clear.
>
Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
layout is also arch specific and does not make sense disassociate them.
This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.

--

	* nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION):
	Remove macro usage.
	(__PTHREAD_SPINS): Move definition to ...
	* nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
	(__PTHREAD_SPINS): ... here.
	* nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.
	* sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Likewise.

ports/ChangeLog.hppa
	* ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
	(__PTHREAD_SPIN): Moved defintion from pthread.h.

---

Comments

Roland McGrath March 26, 2014, 11:11 p.m. UTC | #1
This looks great to me, except for ...

> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */

... this typo ("initializer") repeated many times over.

But for final approval I'll defer to anyone else who is an arch maintainer
and/or has touched this area of the code more recently than I have.


Thanks,
Roland
Adhemerval Zanella Netto April 2, 2014, 12:35 p.m. UTC | #2
Ping for arch maintainers.

On 26-03-2014 17:29, Adhemerval Zanella wrote:
> On 26-03-2014 14:50, Roland McGrath wrote:
>> I don't understand what "support for 64 bits" or "support for 32 bits"
>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
>>
>> That can be a separate cleanup if you want, and others may want to kibitz.
>> But that might involve dropping the header you're adding here, so maybe you
>> want to just resolve it now.
>>
>> If you want to go ahead with this change, then it's OK with the other nits
>> above and this comment rewritten to describe concretely what the macro
>> means.  In actual usage so far, it doesn't actually mean anything about
>> elision support per se.  It just means something about how the fields of
>> pthread_mutex_t are structured and hence what the initializer must look like.
>> If that's all it's for, it should be made clear.
>>
> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
> layout is also arch specific and does not make sense disassociate them.
> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.
>
> --
>
> 	* nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION):
> 	Remove macro usage.
> 	(__PTHREAD_SPINS): Move definition to ...
> 	* nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> 	(__PTHREAD_SPINS): ... here.
> 	* nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
>
> ports/ChangeLog.hppa
> 	* ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Moved defintion from pthread.h.
>
> ---
>
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index 1e0c5dc..40a3e21 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -82,15 +82,6 @@ enum
>  #endif
>
>
> -/* Mutex initializers.  */
> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout.  */
> -#define __PTHREAD_SPINS 0, 0
> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout.  */
> -#define __PTHREAD_SPINS { 0, 0 }
> -#else
> -#define __PTHREAD_SPINS 0
> -#endif
> -
>  #ifdef __PTHREAD_MUTEX_HAVE_PREV
>  # define PTHREAD_MUTEX_INITIALIZER \
>    { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> index 71bd3ae..20e5fc0 100644
> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> index 23a1698..716f151 100644
> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> @@ -20,8 +20,6 @@
>
>  #include <bits/wordsize.h>
>
> -# define __PTHREAD_MUTEX_HAVE_ELISION   0
> -
>  #if __WORDSIZE == 64
>  # define __SIZEOF_PTHREAD_ATTR_T 56
>  # define __SIZEOF_PTHREAD_MUTEX_T 40
> @@ -107,6 +105,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> index e42d94e..f92958c 100644
> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> index be615b6..7b9edf2 100644
> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> index 28e5144..0ca10f2 100644
> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> @@ -105,7 +105,8 @@ typedef union
>      short __elision;
>      __pthread_list_t __list;
>  # define __PTHREAD_MUTEX_HAVE_PREV	1
> -# define __PTHREAD_MUTEX_HAVE_ELISION   1
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +# define __PTHREAD_SPINS             0, 0
>  #else
>      unsigned int __nusers;
>      __extension__ union
> @@ -116,7 +117,7 @@ typedef union
>  	short __elision;
>  # define __spins d.__espins
>  # define __elision d.__elision
> -# define __PTHREAD_MUTEX_HAVE_ELISION   2
> +# define __PTHREAD_SPINS         { 0, 0 }
>        } d;
>        __pthread_slist_t __list;
>      };
> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> index deec4da..621895b 100644
> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> @@ -94,6 +94,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> index f11eeab..76b94b4 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> @@ -74,6 +74,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> index 6f85eae..c633c6d 100644
> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> @@ -73,6 +73,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> index 26edce5..0a0af56 100644
> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> index b77b80a..49969bb 100644
> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> @@ -74,6 +74,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> index 283f240..b21f5f8 100644
> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> index ca053e3..d4c5600 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> @@ -76,6 +76,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> index 9d9386b..712076c 100644
> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> index f469352..17bb112 100644
> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> @@ -112,6 +112,9 @@ typedef union
>    int __align;
>  } pthread_mutexattr_t;
>
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +
>
>  /* Data structure for conditional variable handling.  The structure of
>     the attribute type is not exposed on purpose.  */
>
Stefan Liebler April 4, 2014, 1:09 p.m. UTC | #3
Hi,

this is okay on s390. I have tested the patch with s390/s390x.
Thanks.

On 04/02/2014 02:35 PM, Adhemerval Zanella wrote:
> Ping for arch maintainers.
>
> On 26-03-2014 17:29, Adhemerval Zanella wrote:
>> On 26-03-2014 14:50, Roland McGrath wrote:
>>> I don't understand what "support for 64 bits" or "support for 32 bits"
>>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.
>>>
>>> That can be a separate cleanup if you want, and others may want to kibitz.
>>> But that might involve dropping the header you're adding here, so maybe you
>>> want to just resolve it now.
>>>
>>> If you want to go ahead with this change, then it's OK with the other nits
>>> above and this comment rewritten to describe concretely what the macro
>>> means.  In actual usage so far, it doesn't actually mean anything about
>>> elision support per se.  It just means something about how the fields of
>>> pthread_mutex_t are structured and hence what the initializer must look like.
>>> If that's all it's for, it should be made clear.
>>>
>> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
>> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
>> layout is also arch specific and does not make sense disassociate them.
>> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.
>>
>> --
>>
>> 	* nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION):
>> 	Remove macro usage.
>> 	(__PTHREAD_SPINS): Move definition to ...
>> 	* nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>> 	(__PTHREAD_SPINS): ... here.
>> 	* nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>> 	* sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Likewise.
>>
>> ports/ChangeLog.hppa
>> 	* ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>> 	(__PTHREAD_SPIN): Moved defintion from pthread.h.
>>
>> ---
>>
>> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
>> index 1e0c5dc..40a3e21 100644
>> --- a/nptl/sysdeps/pthread/pthread.h
>> +++ b/nptl/sysdeps/pthread/pthread.h
>> @@ -82,15 +82,6 @@ enum
>>   #endif
>>
>>
>> -/* Mutex initializers.  */
>> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout.  */
>> -#define __PTHREAD_SPINS 0, 0
>> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout.  */
>> -#define __PTHREAD_SPINS { 0, 0 }
>> -#else
>> -#define __PTHREAD_SPINS 0
>> -#endif
>> -
>>   #ifdef __PTHREAD_MUTEX_HAVE_PREV
>>   # define PTHREAD_MUTEX_INITIALIZER \
>>     { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
>> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>> index 71bd3ae..20e5fc0 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>> @@ -106,6 +106,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>> index 23a1698..716f151 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>> @@ -20,8 +20,6 @@
>>
>>   #include <bits/wordsize.h>
>>
>> -# define __PTHREAD_MUTEX_HAVE_ELISION   0
>> -
>>   #if __WORDSIZE == 64
>>   # define __SIZEOF_PTHREAD_ATTR_T 56
>>   # define __SIZEOF_PTHREAD_MUTEX_T 40
>> @@ -107,6 +105,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>> index e42d94e..f92958c 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>> @@ -77,6 +77,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>> index be615b6..7b9edf2 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>> @@ -106,6 +106,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>> index 28e5144..0ca10f2 100644
>> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>> @@ -105,7 +105,8 @@ typedef union
>>       short __elision;
>>       __pthread_list_t __list;
>>   # define __PTHREAD_MUTEX_HAVE_PREV	1
>> -# define __PTHREAD_MUTEX_HAVE_ELISION   1
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +# define __PTHREAD_SPINS             0, 0
>>   #else
>>       unsigned int __nusers;
>>       __extension__ union
>> @@ -116,7 +117,7 @@ typedef union
>>   	short __elision;
>>   # define __spins d.__espins
>>   # define __elision d.__elision
>> -# define __PTHREAD_MUTEX_HAVE_ELISION   2
>> +# define __PTHREAD_SPINS         { 0, 0 }
>>         } d;
>>         __pthread_slist_t __list;
>>       };
>> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>> index deec4da..621895b 100644
>> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>> @@ -94,6 +94,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>> index f11eeab..76b94b4 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>> @@ -74,6 +74,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>> index 6f85eae..c633c6d 100644
>> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>> @@ -73,6 +73,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>> index 26edce5..0a0af56 100644
>> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>> @@ -77,6 +77,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>> index b77b80a..49969bb 100644
>> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>> @@ -74,6 +74,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>> index 283f240..b21f5f8 100644
>> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>> @@ -77,6 +77,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>> index ca053e3..d4c5600 100644
>> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>> @@ -76,6 +76,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>> index 9d9386b..712076c 100644
>> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>> @@ -106,6 +106,9 @@ typedef union
>>     long int __align;
>>   } pthread_mutex_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>   typedef union
>>   {
>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>> index f469352..17bb112 100644
>> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>> @@ -112,6 +112,9 @@ typedef union
>>     int __align;
>>   } pthread_mutexattr_t;
>>
>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>> +#define __PTHREAD_SPINS 0
>> +
>>
>>   /* Data structure for conditional variable handling.  The structure of
>>      the attribute type is not exposed on purpose.  */
>>
>
>
Adhemerval Zanella Netto April 8, 2014, 5:13 p.m. UTC | #4
Thanks for checking it. I'm pinging again because without this patch or similar hack
(define __PTHREAD_MUTEX_HAVE_ELISION somewhere) 'make check' *fails* while
building nptl tests on platform that does not define __PTHREAD_MUTEX_HAVE_ELISION.

On 04-04-2014 10:09, Stefan Liebler wrote:
> Hi,
>
> this is okay on s390. I have tested the patch with s390/s390x.
> Thanks.
>
> On 04/02/2014 02:35 PM, Adhemerval Zanella wrote:
>> Ping for arch maintainers.
>>
>> On 26-03-2014 17:29, Adhemerval Zanella wrote:
>>> On 26-03-2014 14:50, Roland McGrath wrote:
>>>> I don't understand what "support for 64 bits" or "support for 32 bits"
>>>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>>>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>>>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>>>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.
>>>>
>>>> That can be a separate cleanup if you want, and others may want to kibitz.
>>>> But that might involve dropping the header you're adding here, so maybe you
>>>> want to just resolve it now.
>>>>
>>>> If you want to go ahead with this change, then it's OK with the other nits
>>>> above and this comment rewritten to describe concretely what the macro
>>>> means.  In actual usage so far, it doesn't actually mean anything about
>>>> elision support per se.  It just means something about how the fields of
>>>> pthread_mutex_t are structured and hence what the initializer must look like.
>>>> If that's all it's for, it should be made clear.
>>>>
>>> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
>>> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
>>> layout is also arch specific and does not make sense disassociate them.
>>> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.
>>>
>>> -- 
>>>
>>>     * nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION):
>>>     Remove macro usage.
>>>     (__PTHREAD_SPINS): Move definition to ...
>>>     * nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>>>     (__PTHREAD_SPINS): ... here.
>>>     * nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>     * sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Likewise.
>>>
>>> ports/ChangeLog.hppa
>>>     * ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>>>     (__PTHREAD_SPIN): Moved defintion from pthread.h.
>>>
>>> ---
>>>
>>> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
>>> index 1e0c5dc..40a3e21 100644
>>> --- a/nptl/sysdeps/pthread/pthread.h
>>> +++ b/nptl/sysdeps/pthread/pthread.h
>>> @@ -82,15 +82,6 @@ enum
>>>   #endif
>>>
>>>
>>> -/* Mutex initializers.  */
>>> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout.  */
>>> -#define __PTHREAD_SPINS 0, 0
>>> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout.  */
>>> -#define __PTHREAD_SPINS { 0, 0 }
>>> -#else
>>> -#define __PTHREAD_SPINS 0
>>> -#endif
>>> -
>>>   #ifdef __PTHREAD_MUTEX_HAVE_PREV
>>>   # define PTHREAD_MUTEX_INITIALIZER \
>>>     { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
>>> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>>> index 71bd3ae..20e5fc0 100644
>>> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>>> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
>>> @@ -106,6 +106,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>>> index 23a1698..716f151 100644
>>> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>>> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
>>> @@ -20,8 +20,6 @@
>>>
>>>   #include <bits/wordsize.h>
>>>
>>> -# define __PTHREAD_MUTEX_HAVE_ELISION   0
>>> -
>>>   #if __WORDSIZE == 64
>>>   # define __SIZEOF_PTHREAD_ATTR_T 56
>>>   # define __SIZEOF_PTHREAD_MUTEX_T 40
>>> @@ -107,6 +105,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>>> index e42d94e..f92958c 100644
>>> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>>> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
>>> @@ -77,6 +77,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>>> index be615b6..7b9edf2 100644
>>> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>>> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
>>> @@ -106,6 +106,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>>> index 28e5144..0ca10f2 100644
>>> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>>> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
>>> @@ -105,7 +105,8 @@ typedef union
>>>       short __elision;
>>>       __pthread_list_t __list;
>>>   # define __PTHREAD_MUTEX_HAVE_PREV    1
>>> -# define __PTHREAD_MUTEX_HAVE_ELISION   1
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +# define __PTHREAD_SPINS             0, 0
>>>   #else
>>>       unsigned int __nusers;
>>>       __extension__ union
>>> @@ -116,7 +117,7 @@ typedef union
>>>       short __elision;
>>>   # define __spins d.__espins
>>>   # define __elision d.__elision
>>> -# define __PTHREAD_MUTEX_HAVE_ELISION   2
>>> +# define __PTHREAD_SPINS         { 0, 0 }
>>>         } d;
>>>         __pthread_slist_t __list;
>>>       };
>>> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>>> index deec4da..621895b 100644
>>> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>>> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
>>> @@ -94,6 +94,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>>> index f11eeab..76b94b4 100644
>>> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
>>> @@ -74,6 +74,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>>> index 6f85eae..c633c6d 100644
>>> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
>>> @@ -73,6 +73,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>>> index 26edce5..0a0af56 100644
>>> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
>>> @@ -77,6 +77,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>>> index b77b80a..49969bb 100644
>>> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
>>> @@ -74,6 +74,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>>> index 283f240..b21f5f8 100644
>>> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
>>> @@ -77,6 +77,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>>> index ca053e3..d4c5600 100644
>>> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
>>> @@ -76,6 +76,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>>> index 9d9386b..712076c 100644
>>> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
>>> @@ -106,6 +106,9 @@ typedef union
>>>     long int __align;
>>>   } pthread_mutex_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>   typedef union
>>>   {
>>>     char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
>>> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>>> index f469352..17bb112 100644
>>> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>>> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
>>> @@ -112,6 +112,9 @@ typedef union
>>>     int __align;
>>>   } pthread_mutexattr_t;
>>>
>>> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
>>> +#define __PTHREAD_SPINS 0
>>> +
>>>
>>>   /* Data structure for conditional variable handling.  The structure of
>>>      the attribute type is not exposed on purpose.  */
>>>
>>
>>
>
>
Carlos O'Donell April 8, 2014, 8:30 p.m. UTC | #5
On 03/26/2014 04:29 PM, Adhemerval Zanella wrote:
> On 26-03-2014 14:50, Roland McGrath wrote:
>> I don't understand what "support for 64 bits" or "support for 32 bits"
>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
>>
>> That can be a separate cleanup if you want, and others may want to kibitz.
>> But that might involve dropping the header you're adding here, so maybe you
>> want to just resolve it now.
>>
>> If you want to go ahead with this change, then it's OK with the other nits
>> above and this comment rewritten to describe concretely what the macro
>> means.  In actual usage so far, it doesn't actually mean anything about
>> elision support per se.  It just means something about how the fields of
>> pthread_mutex_t are structured and hence what the initializer must look like.
>> If that's all it's for, it should be made clear.
>>
> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
> layout is also arch specific and does not make sense disassociate them.
> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.

My opinion is that you should *not* wait for arch maintainers to review this
patch. It's a fairly mechanical patch that should not break any configuration
and should not cause any undue ABI changes. A simple grep'ing shows you won't
have any problems.

If you want to be nice send a follow-up email and TO all of the arch
maintainers letting them know you made the change. This should cut through
the noise and let them test at their own time and pace.

I pulled the patch down, applied, and tested with it and it looks good
for x86-64, no regressions.

OK to checkin.

> --
> 
> 	* nptl/sysdeps/pthread/pthread.h (__PTHREAD_MUTEX_HAVE_ELISION):
> 	Remove macro usage.
> 	(__PTHREAD_SPINS): Move definition to ...
> 	* nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> 	(__PTHREAD_SPINS): ... here.
> 	* nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.
> 	* sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Likewise.

OK. These look good to me.

> ports/ChangeLog.hppa
> 	* ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> 	(__PTHREAD_SPIN): Moved defintion from pthread.h.

OK for hppa.

> ---
> 
> diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
> index 1e0c5dc..40a3e21 100644
> --- a/nptl/sysdeps/pthread/pthread.h
> +++ b/nptl/sysdeps/pthread/pthread.h
> @@ -82,15 +82,6 @@ enum
>  #endif
>  
>  
> -/* Mutex initializers.  */
> -#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout.  */
> -#define __PTHREAD_SPINS 0, 0
> -#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout.  */
> -#define __PTHREAD_SPINS { 0, 0 }
> -#else
> -#define __PTHREAD_SPINS 0
> -#endif
> -

OK.

>  #ifdef __PTHREAD_MUTEX_HAVE_PREV
>  # define PTHREAD_MUTEX_INITIALIZER \
>    { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> index 71bd3ae..20e5fc0 100644
> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> index 23a1698..716f151 100644
> --- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
> @@ -20,8 +20,6 @@
>  
>  #include <bits/wordsize.h>
>  
> -# define __PTHREAD_MUTEX_HAVE_ELISION   0
> -

OK.

>  #if __WORDSIZE == 64
>  # define __SIZEOF_PTHREAD_ATTR_T 56
>  # define __SIZEOF_PTHREAD_MUTEX_T 40
> @@ -107,6 +105,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> index e42d94e..f92958c 100644
> --- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> index be615b6..7b9edf2 100644
> --- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> index 28e5144..0ca10f2 100644
> --- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> +++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
> @@ -105,7 +105,8 @@ typedef union
>      short __elision;
>      __pthread_list_t __list;
>  # define __PTHREAD_MUTEX_HAVE_PREV	1
> -# define __PTHREAD_MUTEX_HAVE_ELISION   1
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +# define __PTHREAD_SPINS             0, 0

OK.

>  #else
>      unsigned int __nusers;
>      __extension__ union
> @@ -116,7 +117,7 @@ typedef union
>  	short __elision;
>  # define __spins d.__espins
>  # define __elision d.__elision
> -# define __PTHREAD_MUTEX_HAVE_ELISION   2
> +# define __PTHREAD_SPINS         { 0, 0 }

OK.

>        } d;
>        __pthread_slist_t __list;
>      };
> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> index deec4da..621895b 100644
> --- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> +++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
> @@ -94,6 +94,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> index f11eeab..76b94b4 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
> @@ -74,6 +74,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> index 6f85eae..c633c6d 100644
> --- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
> @@ -73,6 +73,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> index 26edce5..0a0af56 100644
> --- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> index b77b80a..49969bb 100644
> --- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
> @@ -74,6 +74,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> index 283f240..b21f5f8 100644
> --- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
> @@ -77,6 +77,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> index ca053e3..d4c5600 100644
> --- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
> @@ -76,6 +76,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> index 9d9386b..712076c 100644
> --- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
> @@ -106,6 +106,9 @@ typedef union
>    long int __align;
>  } pthread_mutex_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  typedef union
>  {
>    char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
> diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> index f469352..17bb112 100644
> --- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> +++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
> @@ -112,6 +112,9 @@ typedef union
>    int __align;
>  } pthread_mutexattr_t;
>  
> +/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
> +#define __PTHREAD_SPINS 0
> +

OK.

>  
>  /* Data structure for conditional variable handling.  The structure of
>     the attribute type is not exposed on purpose.  */
> 

Cheers,
Carlos.
Adhemerval Zanella Netto April 9, 2014, 2:01 p.m. UTC | #6
On 08-04-2014 17:30, Carlos O'Donell wrote:
> On 03/26/2014 04:29 PM, Adhemerval Zanella wrote:
>> On 26-03-2014 14:50, Roland McGrath wrote:
>>> I don't understand what "support for 64 bits" or "support for 32 bits"
>>> means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
>>> seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
>>> is really just about the x86-private layout of pthread_mutex_t.  It seems
>>> more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
>>>
>>> That can be a separate cleanup if you want, and others may want to kibitz.
>>> But that might involve dropping the header you're adding here, so maybe you
>>> want to just resolve it now.
>>>
>>> If you want to go ahead with this change, then it's OK with the other nits
>>> above and this comment rewritten to describe concretely what the macro
>>> means.  In actual usage so far, it doesn't actually mean anything about
>>> elision support per se.  It just means something about how the fields of
>>> pthread_mutex_t are structured and hence what the initializer must look like.
>>> If that's all it's for, it should be made clear.
>>>
>> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
>> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
>> layout is also arch specific and does not make sense disassociate them.
>> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.
> My opinion is that you should *not* wait for arch maintainers to review this
> patch. It's a fairly mechanical patch that should not break any configuration
> and should not cause any undue ABI changes. A simple grep'ing shows you won't
> have any problems.
>
> If you want to be nice send a follow-up email and TO all of the arch
> maintainers letting them know you made the change. This should cut through
> the noise and let them test at their own time and pace.
>
> I pulled the patch down, applied, and tested with it and it looks good
> for x86-64, no regressions.
>
> OK to checkin.
>
Pushed upstream as 01f8eac224421f07f28f91cc05db7459ea433ea4 and CCing all the arch
maintainers.

PS: sorry for the spam.
David Miller April 11, 2014, 5:30 p.m. UTC | #7
From: Adhemerval Zanella <azanella@linux.vnet.ibm.com>
Date: Wed, 09 Apr 2014 11:01:18 -0300

> Pushed upstream as 01f8eac224421f07f28f91cc05db7459ea433ea4 and CCing all the arch
> maintainers.
> 
> PS: sorry for the spam.

Thanks for taking care of this.
Torvald Riegel April 11, 2014, 8:08 p.m. UTC | #8
On Wed, 2014-03-26 at 17:29 -0300, Adhemerval Zanella wrote:
> On 26-03-2014 14:50, Roland McGrath wrote:
> > I don't understand what "support for 64 bits" or "support for 32 bits"
> > means.  OK, I've looked at bits/pthreadtypes.h so I do understand.  But it
> > seems pretty wrong to pretend this is a generic 32/64 sort of thing when it
> > is really just about the x86-private layout of pthread_mutex_t.  It seems
> > more proper to have bits/pthreadtypes.h just define __PTHREAD_SPINS.  
> >
> > That can be a separate cleanup if you want, and others may want to kibitz.
> > But that might involve dropping the header you're adding here, so maybe you
> > want to just resolve it now.
> >
> > If you want to go ahead with this change, then it's OK with the other nits
> > above and this comment rewritten to describe concretely what the macro
> > means.  In actual usage so far, it doesn't actually mean anything about
> > elision support per se.  It just means something about how the fields of
> > pthread_mutex_t are structured and hence what the initializer must look like.
> > If that's all it's for, it should be made clear.
> >
> Cleanup up the whole __PTHREAD_SPINS seems the appropriated measure.  This patch
> moves the __PTHREAD_SPINS definition to arch specific header since pthread_mutex_t
> layout is also arch specific and does not make sense disassociate them.
> This makes the definition of __PTHREAD_MUTEX_HAVE_ELISION not required.

I know I'm late with this comment, but I would have preferred if this
was called __PTHREAD_MUTEX_SPINS or something like this.  It's not a
generic spinning-related thing.
diff mbox

Patch

diff --git a/nptl/sysdeps/pthread/pthread.h b/nptl/sysdeps/pthread/pthread.h
index 1e0c5dc..40a3e21 100644
--- a/nptl/sysdeps/pthread/pthread.h
+++ b/nptl/sysdeps/pthread/pthread.h
@@ -82,15 +82,6 @@  enum
 #endif
 
 
-/* Mutex initializers.  */
-#if __PTHREAD_MUTEX_HAVE_ELISION == 1 /* 64bit layout.  */
-#define __PTHREAD_SPINS 0, 0
-#elif __PTHREAD_MUTEX_HAVE_ELISION == 2 /* 32bit layout.  */
-#define __PTHREAD_SPINS { 0, 0 }
-#else
-#define __PTHREAD_SPINS 0
-#endif
-
 #ifdef __PTHREAD_MUTEX_HAVE_PREV
 # define PTHREAD_MUTEX_INITIALIZER \
   { { 0, 0, 0, 0, 0, __PTHREAD_SPINS, { 0, 0 } } }
diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
index 71bd3ae..20e5fc0 100644
--- a/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h
@@ -106,6 +106,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
index 23a1698..716f151 100644
--- a/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/s390/bits/pthreadtypes.h
@@ -20,8 +20,6 @@ 
 
 #include <bits/wordsize.h>
 
-# define __PTHREAD_MUTEX_HAVE_ELISION   0
-
 #if __WORDSIZE == 64
 # define __SIZEOF_PTHREAD_ATTR_T 56
 # define __SIZEOF_PTHREAD_MUTEX_T 40
@@ -107,6 +105,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
index e42d94e..f92958c 100644
--- a/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/sh/bits/pthreadtypes.h
@@ -77,6 +77,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
index be615b6..7b9edf2 100644
--- a/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/sparc/bits/pthreadtypes.h
@@ -106,6 +106,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
index 28e5144..0ca10f2 100644
--- a/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
+++ b/nptl/sysdeps/unix/sysv/linux/x86/bits/pthreadtypes.h
@@ -105,7 +105,8 @@  typedef union
     short __elision;
     __pthread_list_t __list;
 # define __PTHREAD_MUTEX_HAVE_PREV	1
-# define __PTHREAD_MUTEX_HAVE_ELISION   1
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+# define __PTHREAD_SPINS             0, 0
 #else
     unsigned int __nusers;
     __extension__ union
@@ -116,7 +117,7 @@  typedef union
 	short __elision;
 # define __spins d.__espins
 # define __elision d.__elision
-# define __PTHREAD_MUTEX_HAVE_ELISION   2
+# define __PTHREAD_SPINS         { 0, 0 }
       } d;
       __pthread_slist_t __list;
     };
diff --git a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
index deec4da..621895b 100644
--- a/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
+++ b/ports/sysdeps/unix/sysv/linux/hppa/nptl/bits/pthreadtypes.h
@@ -94,6 +94,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
index f11eeab..76b94b4 100644
--- a/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/aarch64/nptl/bits/pthreadtypes.h
@@ -74,6 +74,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
index 6f85eae..c633c6d 100644
--- a/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/alpha/nptl/bits/pthreadtypes.h
@@ -73,6 +73,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
index 26edce5..0a0af56 100644
--- a/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/arm/nptl/bits/pthreadtypes.h
@@ -77,6 +77,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
index b77b80a..49969bb 100644
--- a/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/ia64/nptl/bits/pthreadtypes.h
@@ -74,6 +74,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
index 283f240..b21f5f8 100644
--- a/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/m68k/nptl/bits/pthreadtypes.h
@@ -77,6 +77,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
index ca053e3..d4c5600 100644
--- a/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/microblaze/nptl/bits/pthreadtypes.h
@@ -76,6 +76,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
index 9d9386b..712076c 100644
--- a/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/mips/nptl/bits/pthreadtypes.h
@@ -106,6 +106,9 @@  typedef union
   long int __align;
 } pthread_mutex_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 typedef union
 {
   char __size[__SIZEOF_PTHREAD_MUTEXATTR_T];
diff --git a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
index f469352..17bb112 100644
--- a/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
+++ b/sysdeps/unix/sysv/linux/tile/nptl/bits/pthreadtypes.h
@@ -112,6 +112,9 @@  typedef union
   int __align;
 } pthread_mutexattr_t;
 
+/* Mutex __spins initilizer used by PTHREAD_MUTEX_INITIALIZER.  */
+#define __PTHREAD_SPINS 0
+
 
 /* Data structure for conditional variable handling.  The structure of
    the attribute type is not exposed on purpose.  */