This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
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.