* [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
@ 2017-10-15 15:11 H.J. Lu
2017-10-16 13:51 ` Adhemerval Zanella
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-15 15:11 UTC (permalink / raw)
To: GNU C Library; +Cc: Adhemerval Zanella
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?
H.J.
---
[BZ #22298]
* nptl/allocatestack.c (allocate_stack): Check if
__PTHREAD_MUTEX_HAVE_PREV is non-zero, instead if
__PTHREAD_MUTEX_HAVE_PREV is defined.
* nptl/descr.h (pthread): Likewise.
* nptl/nptl-init.c (__pthread_initialize_minimal_internal):
Likewise.
* nptl/pthread_create.c (START_THREAD_DEFN): Likewise.
* sysdeps/nptl/fork.c (__libc_fork): Likewise.
* sysdeps/nptl/pthread.h (PTHREAD_MUTEX_INITIALIZER): Likewise.
* sysdeps/nptl/bits/thread-shared-types.h
(__PTHREAD_MUTEX_HAVE_PREV): Define only if it is undefined.
(__pthread_internal_list): Check __pthread_internal_list instead
of __WORDSIZE.
(__PTHREAD_SPINS_DATA): Likewise.
(__PTHREAD_SPINS): Likewise.
(__pthread_mutex_s): Likewise.
* sysdeps/x86/nptl/bits/pthreadtypes-arch.h
(__PTHREAD_MUTEX_HAVE_PREV): Defined.
---
nptl/allocatestack.c | 2 +-
nptl/descr.h | 2 +-
nptl/nptl-init.c | 2 +-
nptl/pthread_create.c | 4 ++--
sysdeps/nptl/bits/thread-shared-types.h | 17 ++++++++++++-----
sysdeps/nptl/fork.c | 2 +-
sysdeps/nptl/pthread.h | 2 +-
sysdeps/x86/nptl/bits/pthreadtypes-arch.h | 2 ++
8 files changed, 21 insertions(+), 12 deletions(-)
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
--
2.13.5
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-15 15:11 [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298] H.J. Lu
@ 2017-10-16 13:51 ` Adhemerval Zanella
2017-10-16 14:32 ` Andreas Schwab
2017-10-16 15:00 ` H.J. Lu
0 siblings, 2 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2017-10-16 13:51 UTC (permalink / raw)
To: H.J. Lu, GNU C Library
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 13:51 ` Adhemerval Zanella
@ 2017-10-16 14:32 ` Andreas Schwab
2017-10-16 14:58 ` H.J. Lu
2017-10-16 15:00 ` H.J. Lu
1 sibling, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-16 14:32 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: H.J. Lu, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 14:32 ` Andreas Schwab
@ 2017-10-16 14:58 ` H.J. Lu
2017-10-16 15:25 ` Andreas Schwab
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-16 14:58 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Adhemerval Zanella, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 13:51 ` Adhemerval Zanella
2017-10-16 14:32 ` Andreas Schwab
@ 2017-10-16 15:00 ` H.J. Lu
2017-10-16 15:14 ` Adhemerval Zanella
1 sibling, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-16 15:00 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 15:00 ` H.J. Lu
@ 2017-10-16 15:14 ` Adhemerval Zanella
0 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2017-10-16 15:14 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 14:58 ` H.J. Lu
@ 2017-10-16 15:25 ` Andreas Schwab
2017-10-16 15:51 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-16 15:25 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 15:25 ` Andreas Schwab
@ 2017-10-16 15:51 ` H.J. Lu
2017-10-16 16:05 ` Andreas Schwab
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-16 15:51 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Adhemerval Zanella, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 15:51 ` H.J. Lu
@ 2017-10-16 16:05 ` Andreas Schwab
2017-10-16 16:07 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-16 16:05 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 16:05 ` Andreas Schwab
@ 2017-10-16 16:07 ` H.J. Lu
2017-10-16 17:39 ` Andreas Schwab
2017-10-17 7:21 ` Andreas Schwab
0 siblings, 2 replies; 20+ messages in thread
From: H.J. Lu @ 2017-10-16 16:07 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Adhemerval Zanella, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 16:07 ` H.J. Lu
@ 2017-10-16 17:39 ` Andreas Schwab
2017-10-16 18:00 ` H.J. Lu
2017-10-17 7:21 ` Andreas Schwab
1 sibling, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-16 17:39 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 17:39 ` Andreas Schwab
@ 2017-10-16 18:00 ` H.J. Lu
2017-10-16 19:17 ` Adhemerval Zanella
2017-10-17 7:20 ` Andreas Schwab
0 siblings, 2 replies; 20+ messages in thread
From: H.J. Lu @ 2017-10-16 18:00 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Adhemerval Zanella, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 18:00 ` H.J. Lu
@ 2017-10-16 19:17 ` Adhemerval Zanella
2017-10-17 7:20 ` Andreas Schwab
1 sibling, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2017-10-16 19:17 UTC (permalink / raw)
To: H.J. Lu, Andreas Schwab; +Cc: GNU C Library
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 18:00 ` H.J. Lu
2017-10-16 19:17 ` Adhemerval Zanella
@ 2017-10-17 7:20 ` Andreas Schwab
2017-10-17 15:23 ` H.J. Lu
1 sibling, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-17 7:20 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-16 16:07 ` H.J. Lu
2017-10-16 17:39 ` Andreas Schwab
@ 2017-10-17 7:21 ` Andreas Schwab
1 sibling, 0 replies; 20+ messages in thread
From: Andreas Schwab @ 2017-10-17 7:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-17 7:20 ` Andreas Schwab
@ 2017-10-17 15:23 ` H.J. Lu
2017-10-18 8:21 ` Andreas Schwab
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-17 15:23 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Adhemerval Zanella, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-17 15:23 ` H.J. Lu
@ 2017-10-18 8:21 ` Andreas Schwab
2017-10-18 11:06 ` Adhemerval Zanella
0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2017-10-18 8:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
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.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-18 8:21 ` Andreas Schwab
@ 2017-10-18 11:06 ` Adhemerval Zanella
2017-10-18 12:35 ` H.J. Lu
0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2017-10-18 11:06 UTC (permalink / raw)
To: Andreas Schwab, H.J. Lu; +Cc: GNU C Library
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-18 11:06 ` Adhemerval Zanella
@ 2017-10-18 12:35 ` H.J. Lu
2017-10-18 13:02 ` Adhemerval Zanella
0 siblings, 1 reply; 20+ messages in thread
From: H.J. Lu @ 2017-10-18 12:35 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: Andreas Schwab, GNU C Library
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.
--
H.J.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298]
2017-10-18 12:35 ` H.J. Lu
@ 2017-10-18 13:02 ` Adhemerval Zanella
0 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2017-10-18 13:02 UTC (permalink / raw)
To: H.J. Lu; +Cc: Andreas Schwab, GNU C Library
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).
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-10-18 13:02 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-15 15:11 [PATCH] Define __PTHREAD_MUTEX_HAVE_PREV only if undefined [BZ #22298] H.J. Lu
2017-10-16 13:51 ` Adhemerval Zanella
2017-10-16 14:32 ` Andreas Schwab
2017-10-16 14:58 ` H.J. Lu
2017-10-16 15:25 ` Andreas Schwab
2017-10-16 15:51 ` H.J. Lu
2017-10-16 16:05 ` Andreas Schwab
2017-10-16 16:07 ` H.J. Lu
2017-10-16 17:39 ` Andreas Schwab
2017-10-16 18:00 ` H.J. Lu
2017-10-16 19:17 ` Adhemerval Zanella
2017-10-17 7:20 ` Andreas Schwab
2017-10-17 15:23 ` H.J. Lu
2017-10-18 8:21 ` Andreas Schwab
2017-10-18 11:06 ` Adhemerval Zanella
2017-10-18 12:35 ` H.J. Lu
2017-10-18 13:02 ` Adhemerval Zanella
2017-10-17 7:21 ` Andreas Schwab
2017-10-16 15:00 ` H.J. Lu
2017-10-16 15:14 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).