Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]

["H.J. Lu" <hjl dot tools at gmail dot com>]

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



-- 
H.J.