public inbox for libc-hacker@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] clean up PPC for private futex changes.
@ 2007-07-13 20:44 Steven Munroe
  2007-07-23 16:10 ` Ulrich Drepper
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Munroe @ 2007-07-13 20:44 UTC (permalink / raw)
  To: GNU libc hacker, Paul Mackerras; +Cc: Ryan Arnold, Alan Modra

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

PPC is broken in mainline due to missing private futex support. The
attached patch attempts to enable PPC for private futexes. I am not sure
if I reverse engineered the i386/x86_64 changes correctly so a close
inspection would be appreciated (especially for lll_futex_wake_unlock!).

With the attached patch powerpc32/powerpc64 makes/make checks cleanly on
a none-private futex eanbled kernel.

Paul what is the status of private futex support powerpc64 kernels?

thanks



[-- Attachment #2: ppc-private-futex-20070710.txt --]
[-- Type: text/plain, Size: 11114 bytes --]

2007-07-10  Steven Munroe  <sjmunroe@us.ibm.com>

	* pthread_rwlock_rdlock.c (__pthread_rwlock_rdlock): Add LLL_SHARED
	parameter to lll_futex_wait call.
	* pthread_rwlock_wrlock.c (__pthread_rwlock_wrlock): Likewise.

	* sysdeps/unix/sysv/linux/powerpc/pthread_once.c (__pthread_once):
	Replace lll_futex_wait with lll_private_futex_wait.
	* sysdeps/unix/sysv/linux/powerpc/sem_post.c (__new_sem_post):
	Add LLL_SHARED parameter to lll_futex_wake().

	* sysdeps/unix/sysv/linux/powerpc/lowlevellock.h: Define LLL_PRIVATE
	LLL_SHARED, lll_private_futex_wait, lll_private_futex_timed_wait and
	lll_private_futex_wake.
	(lll_futex_wait): Add private parameter. Adjust FUTEX_PRIVATE_FLAG
	bit from private parm before syscall.
	(lll_futex_timed_wait): Likewise.
	(lll_futex_wake): Likewise.
	(lll_futex_wake_unlock): Likewise.
	(lll_mutex_unlock): Add LLL_SHARED parm to lll_futex_wake call.
	(lll_robust_mutex_unlock): Likewise.
	(lll_mutex_unlock_force): Likewise.
	(lll_wait_tid): Add LLL_SHARED parm to lll_futex_wait call.

diff -urN libc25-cvstip-20070710/nptl/pthread_rwlock_rdlock.c libc25/nptl/pthread_rwlock_rdlock.c
--- libc25-cvstip-20070710/nptl/pthread_rwlock_rdlock.c	2007-05-26 17:11:31.000000000 -0500
+++ libc25/nptl/pthread_rwlock_rdlock.c	2007-07-11 15:46:33.775044328 -0500
@@ -77,7 +77,9 @@
       lll_mutex_unlock (rwlock->__data.__lock);
 
       /* Wait for the writer to finish.  */
-      lll_futex_wait (&rwlock->__data.__readers_wakeup, waitval);
+      lll_futex_wait (&rwlock->__data.__readers_wakeup, waitval, 
+		      // XYZ check mutex flag
+		      LLL_SHARED);
 
       /* Get the lock.  */
       lll_mutex_lock (rwlock->__data.__lock);
diff -urN libc25-cvstip-20070710/nptl/pthread_rwlock_wrlock.c libc25/nptl/pthread_rwlock_wrlock.c
--- libc25-cvstip-20070710/nptl/pthread_rwlock_wrlock.c	2006-10-28 00:11:15.000000000 -0500
+++ libc25/nptl/pthread_rwlock_wrlock.c	2007-07-13 15:45:04.763093928 -0500
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2007 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
 
@@ -68,7 +68,9 @@
       lll_mutex_unlock (rwlock->__data.__lock);
 
       /* Wait for the writer or reader(s) to finish.  */
-      lll_futex_wait (&rwlock->__data.__writer_wakeup, waitval);
+      lll_futex_wait (&rwlock->__data.__writer_wakeup, waitval,
+		      // XYZ check mutex flag
+		      LLL_SHARED);
 
       /* Get the lock.  */
       lll_mutex_lock (rwlock->__data.__lock);
diff -urN libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h libc25/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
--- libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h	2007-05-29 23:44:54.000000000 -0500
+++ libc25/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h	2007-07-13 14:41:43.810089800 -0500
@@ -39,37 +39,46 @@
 #define FUTEX_TRYLOCK_PI	8
 #define FUTEX_PRIVATE_FLAG	128
 
+/* Values for 'private' parameter of locking macros.  Yes, the
+   definition seems to be backwards.  But it is not.  The bit will be
+   reversed before passing to the system call.  */
+#define LLL_PRIVATE	0
+#define LLL_SHARED	FUTEX_PRIVATE_FLAG
+
 
 /* Initializer for compatibility lock.	*/
 #define LLL_MUTEX_LOCK_INITIALIZER (0)
 
-#define lll_futex_wait(futexp, val) \
+#define lll_futex_wait(futexp, val, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
+    long int opt_flags = (FUTEX_WAIT | LLL_SHARED) ^ private;		      \
     long int __ret;							      \
 									      \
     __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), FUTEX_WAIT, (val), 0);		      \
+			      (futexp), opt_flags, (val), 0);		      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
-#define lll_futex_timed_wait(futexp, val, timespec) \
+#define lll_futex_timed_wait(futexp, val, timespec, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
+    long int opt_flags = (FUTEX_WAIT | LLL_SHARED) ^ private;		      \
     long int __ret;							      \
 									      \
     __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), FUTEX_WAIT, (val), (timespec));	      \
+			      (futexp), opt_flags, (val), (timespec));	      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
-#define lll_futex_wake(futexp, nr) \
+#define lll_futex_wake(futexp, nr, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
+    long int opt_flags = (FUTEX_WAKE | LLL_SHARED) ^ private;		      \
     long int __ret;							      \
 									      \
     __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), FUTEX_WAKE, (nr), 0);		      \
+			      (futexp), opt_flags, (nr), 0);		      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
@@ -97,17 +106,69 @@
   })
 
 /* Returns non-zero if error happened, zero if success.  */
-#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2) \
+#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
+    long int opt_flags = (FUTEX_WAKE_OP | LLL_SHARED) ^ private;	      \
+    long int opt_flag2 = (FUTEX_OP_CLEAR_WAKE_IF_GT_ONE | LLL_SHARED)	      \
+                          ^ private;					      \
     long int __ret;							      \
 									      \
     __ret = INTERNAL_SYSCALL (futex, __err, 6,				      \
-			      (futexp), FUTEX_WAKE_OP, (nr_wake),	      \
+			      (futexp), opt_flags, (nr_wake),		      \
 			      (nr_wake2), (futexp2),			      \
-			      FUTEX_OP_CLEAR_WAKE_IF_GT_ONE);		      \
+			      opt_flag2);				      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err);				      \
   })
+  
+  
+#define lll_private_futex_wait(futexp, val) \
+  lll_private_futex_timed_wait (futexp, val, NULL)
+
+
+#ifdef __ASSUME_PRIVATE_FUTEX  
+# define lll_private_futex_timed_wait(futexp, val, timeout) \
+  ({									      \
+    INTERNAL_SYSCALL_DECL (__err);					      \
+    long int __ret;							      \
+									      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
+			      (futexp), (FUTEX_WAIT | FUTEX_PRIVATE_FLAG),    \
+			      (val), (timeout));			      \
+    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
+  })
+
+# define lll_private_futex_wake(futexp, val) \
+  ({									      \
+    INTERNAL_SYSCALL_DECL (__err);					      \
+    long int __ret;							      \
+									      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
+			      (futexp), (FUTEX_WAKE | FUTEX_PRIVATE_FLAG),    \
+			      (val), 0);				      \
+    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
+  })
+#else
+# define lll_private_futex_timed_wait(futexp, val, timeout) \
+  ({									      \
+    INTERNAL_SYSCALL_DECL (__err);					      \
+    long int __ret;							      \
+									      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
+			      (futexp), FUTEX_WAIT, (val), (timeout));	      \
+    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
+  })
+
+# define lll_private_futex_wake(futexp, val) \
+  ({									      \
+    INTERNAL_SYSCALL_DECL (__err);					      \
+    long int __ret;							      \
+									      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
+			      (futexp), FUTEX_WAKE, (val), 0);		      \
+    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
+  })
+#endif
 
 #ifdef UP
 # define __lll_acq_instr	""
@@ -230,7 +291,7 @@
     int *__futex = &(lock);						      \
     int __val = atomic_exchange_rel (__futex, 0);			      \
     if (__builtin_expect (__val > 1, 0))				      \
-      lll_futex_wake (__futex, 1);					      \
+      lll_futex_wake (__futex, 1, LLL_SHARED);				      \
   }))
 
 #define lll_robust_mutex_unlock(lock) \
@@ -238,7 +299,7 @@
     int *__futex = &(lock);						      \
     int __val = atomic_exchange_rel (__futex, 0);			      \
     if (__builtin_expect (__val & FUTEX_WAITERS, 0))			      \
-      lll_futex_wake (__futex, 1);					      \
+      lll_futex_wake (__futex, 1, LLL_SHARED);				      \
   }))
 
 #define lll_mutex_unlock_force(lock) \
@@ -246,7 +307,7 @@
     int *__futex = &(lock);						      \
     *__futex = 0;							      \
     __asm __volatile (__lll_rel_instr ::: "memory");			      \
-    lll_futex_wake (__futex, 1);					      \
+    lll_futex_wake (__futex, 1, LLL_SHARED);				      \
   }))
 
 #define lll_mutex_islocked(futex) \
@@ -281,7 +342,7 @@
   do {									      \
     __typeof (tid) __tid;						      \
     while ((__tid = (tid)) != 0)					      \
-      lll_futex_wait (&(tid), __tid);					      \
+      lll_futex_wait (&(tid), __tid, LLL_SHARED);			      \
   } while (0)
 
 extern int __lll_timedwait_tid (int *, const struct timespec *)
diff -urN libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c libc25/nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c
--- libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c	2004-09-08 01:07:35.000000000 -0500
+++ libc25/nptl/sysdeps/unix/sysv/linux/powerpc/pthread_once.c	2007-07-13 15:44:06.908102352 -0500
@@ -1,4 +1,4 @@
-/* Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Paul Mackerras <paulus@au.ibm.com>, 2003.
 
@@ -30,7 +30,7 @@
   pthread_once_t *once_control = (pthread_once_t *) arg;
 
   *once_control = 0;
-  lll_futex_wake (once_control, INT_MAX);
+  lll_private_futex_wake (once_control, INT_MAX);
 }
 
 
@@ -74,7 +74,7 @@
 	break;
 
       /* Same generation, some other thread was faster. Wait.  */
-      lll_futex_wait (once_control, oldval);
+      lll_private_futex_wait (once_control, oldval);
     }
 
 
@@ -92,7 +92,7 @@
   atomic_increment (once_control);
 
   /* Wake up all other threads.  */
-  lll_futex_wake (once_control, INT_MAX);
+  lll_private_futex_wake (once_control, INT_MAX);
 
   return 0;
 }
diff -urN libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/sem_post.c libc25/nptl/sysdeps/unix/sysv/linux/powerpc/sem_post.c
--- libc25-cvstip-20070710/nptl/sysdeps/unix/sysv/linux/powerpc/sem_post.c	2007-05-15 01:31:57.000000000 -0500
+++ libc25/nptl/sysdeps/unix/sysv/linux/powerpc/sem_post.c	2007-07-13 15:43:52.523158896 -0500
@@ -1,5 +1,5 @@
 /* sem_post -- post to a POSIX semaphore.  Powerpc version.
-   Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Paul Mackerras <paulus@au.ibm.com>, 2003.
 
@@ -33,7 +33,7 @@
 
   __asm __volatile (__lll_rel_instr ::: "memory");
   int nr = atomic_increment_val (futex);
-  int err = lll_futex_wake (futex, nr);
+  int err = lll_futex_wake (futex, nr, LLL_SHARED);
   if (__builtin_expect (err, 0) < 0)
     {
       __set_errno (-err);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clean up PPC for private futex changes.
  2007-07-13 20:44 [PATCH] clean up PPC for private futex changes Steven Munroe
@ 2007-07-23 16:10 ` Ulrich Drepper
  2007-07-23 20:43   ` Jakub Jelinek
  0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Drepper @ 2007-07-23 16:10 UTC (permalink / raw)
  To: Steven Munroe; +Cc: GNU libc hacker

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Applied.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQFGpNLz2ijCOnn/RHQRAmDXAKCLVzRHHtYOamRI0ZInYRNQrw+5jwCdEzUG
TgjOeA5emKjHjpzwLSJwBm4=
=dMth
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clean up PPC for private futex changes.
  2007-07-23 16:10 ` Ulrich Drepper
@ 2007-07-23 20:43   ` Jakub Jelinek
  2007-07-23 22:09     ` Steven Munroe
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2007-07-23 20:43 UTC (permalink / raw)
  To: Ulrich Drepper; +Cc: Steven Munroe, GNU libc hacker

On Mon, Jul 23, 2007 at 09:10:27AM -0700, Ulrich Drepper wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Applied.

Unfortunately that patch contains several important bugs:
1) lll_futex_wait etc. had only correct definition when
   __ASSUME_PRIVATE_FUTEX, when that is not true, it would
   call futex syscall with bit 7 set even when libpthread.so
   init determined it is not supported
2) lll_private_futex_wait etc. macros were shared when
   not __ASSUME_PRIVATE_FUTEX
3) FUTEX_WAKE_OP had the wake operation argument also ored
   with 128 in some cases

Here is how it IMHO should look like (built and tested on ppc64-linux,
unfortunately not wiht a 2.6.23ish kernel).
The x86_64 changes are untested.

2007-07-23  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
	(__lll_private_flag): Define.
	(lll_futex_wait): Define as a wrapper around lll_futex_timed_wait.
	(lll_futex_timed_wait, lll_futex_wake, lll_futex_wake_unlock): Use
	__lll_private_flag.
	(lll_private_futex_wait, lll_private_futex_timedwait,
	lll_private_futex_wake): Define as wrapper around non-_private
	macros.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
	(__lll_private_flag): Define.
	(lll_futex_timed_wait, lll_futex_wake): Use __lll_private_flag.
	(lll_private_futex_wait, lll_private_futex_timedwait,
	lll_private_futex_wake): Define as wrapper around non-_private
	macros.

--- libc/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h.jj	2007-07-23 19:36:30.000000000 +0200
+++ libc/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h	2007-07-23 21:36:34.000000000 +0200
@@ -45,40 +45,55 @@
 #define LLL_PRIVATE	0
 #define LLL_SHARED	FUTEX_PRIVATE_FLAG
 
+#if !defined NOT_IN_libc || defined IS_IN_rtld
+/* In libc.so or ld.so all futexes are private.  */
+# ifdef __ASSUME_PRIVATE_FUTEX
+#  define __lll_private_flag(fl, private) \
+  ((fl) | FUTEX_PRIVATE_FLAG)
+# else
+#  define __lll_private_flag(fl, private) \
+  ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))
+# endif
+#else
+# ifdef __ASSUME_PRIVATE_FUTEX
+#  define __lll_private_flag(fl, private) \
+  (((fl) | FUTEX_PRIVATE_FLAG) ^ (private))
+# else
+#  define __lll_private_flag(fl, private) \
+  (__builtin_constant_p (private)					      \
+   ? ((private) == 0							      \
+      ? ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))	      \
+      : (fl))								      \
+   : ((fl) | (((private) ^ FUTEX_PRIVATE_FLAG)				      \
+	      & THREAD_GETMEM (THREAD_SELF, header.private_futex))))
+# endif	      
+#endif
 
 /* Initializer for compatibility lock.	*/
 #define LLL_MUTEX_LOCK_INITIALIZER (0)
 
 #define lll_futex_wait(futexp, val, private) \
-  ({									      \
-    INTERNAL_SYSCALL_DECL (__err);					      \
-    long int opt_flags = (FUTEX_WAIT | LLL_SHARED) ^ private;		      \
-    long int __ret;							      \
-									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), opt_flags, (val), 0);		      \
-    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
-  })
+  lll_futex_timed_wait (futexp, val, NULL, private)
 
 #define lll_futex_timed_wait(futexp, val, timespec, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
-    long int opt_flags = (FUTEX_WAIT | LLL_SHARED) ^ private;		      \
     long int __ret;							      \
 									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), opt_flags, (val), (timespec));	      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4, (futexp),		      \
+			      __lll_private_flag (FUTEX_WAIT, private),	      \
+			      (val), (timespec));			      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
 #define lll_futex_wake(futexp, nr, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
-    long int opt_flags = (FUTEX_WAKE | LLL_SHARED) ^ private;		      \
     long int __ret;							      \
 									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), opt_flags, (nr), 0);		      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 4, (futexp),		      \
+			      __lll_private_flag (FUTEX_WAKE, private),	      \
+			      (nr), 0);					      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
   })
 
@@ -109,66 +124,24 @@
 #define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
   ({									      \
     INTERNAL_SYSCALL_DECL (__err);					      \
-    long int opt_flags = (FUTEX_WAKE_OP | LLL_SHARED) ^ private;	      \
-    long int opt_flag2 = (FUTEX_OP_CLEAR_WAKE_IF_GT_ONE | LLL_SHARED)	      \
-                          ^ private;					      \
     long int __ret;							      \
 									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 6,				      \
-			      (futexp), opt_flags, (nr_wake),		      \
-			      (nr_wake2), (futexp2),			      \
-			      opt_flag2);				      \
+    __ret = INTERNAL_SYSCALL (futex, __err, 6, (futexp),		      \
+			      __lll_private_flag (FUTEX_WAKE_OP, private),    \
+			      (nr_wake), (nr_wake2), (futexp2),		      \
+			      FUTEX_OP_CLEAR_WAKE_IF_GT_ONE);		      \
     INTERNAL_SYSCALL_ERROR_P (__ret, __err);				      \
   })
   
   
 #define lll_private_futex_wait(futexp, val) \
-  lll_private_futex_timed_wait (futexp, val, NULL)
-
-
-#ifdef __ASSUME_PRIVATE_FUTEX  
-# define lll_private_futex_timed_wait(futexp, val, timeout) \
-  ({									      \
-    INTERNAL_SYSCALL_DECL (__err);					      \
-    long int __ret;							      \
-									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), (FUTEX_WAIT | FUTEX_PRIVATE_FLAG),    \
-			      (val), (timeout));			      \
-    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
-  })
+  lll_futex_timed_wait (futexp, val, NULL, LLL_PRIVATE)
 
-# define lll_private_futex_wake(futexp, val) \
-  ({									      \
-    INTERNAL_SYSCALL_DECL (__err);					      \
-    long int __ret;							      \
-									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), (FUTEX_WAKE | FUTEX_PRIVATE_FLAG),    \
-			      (val), 0);				      \
-    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
-  })
-#else
-# define lll_private_futex_timed_wait(futexp, val, timeout) \
-  ({									      \
-    INTERNAL_SYSCALL_DECL (__err);					      \
-    long int __ret;							      \
-									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), FUTEX_WAIT, (val), (timeout));	      \
-    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
-  })
+#define lll_private_futex_timed_wait(futexp, val, timeout) \
+  lll_futex_timed_wait (futexp, val, timeout, LLL_PRIVATE)
 
-# define lll_private_futex_wake(futexp, val) \
-  ({									      \
-    INTERNAL_SYSCALL_DECL (__err);					      \
-    long int __ret;							      \
-									      \
-    __ret = INTERNAL_SYSCALL (futex, __err, 4,				      \
-			      (futexp), FUTEX_WAKE, (val), 0);		      \
-    INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret;		      \
-  })
-#endif
+#define lll_private_futex_wake(futexp, val) \
+  lll_futex_wake (futexp, val, LLL_PRIVATE)
 
 #ifdef UP
 # define __lll_acq_instr	""
--- libc/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h.jj	2007-06-19 13:10:21.000000000 +0200
+++ libc/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h	2007-07-23 21:56:00.000000000 +0200
@@ -50,6 +50,31 @@
 #define LLL_PRIVATE	0
 #define LLL_SHARED	FUTEX_PRIVATE_FLAG
 
+#if !defined NOT_IN_libc || defined IS_IN_rtld
+/* In libc.so or ld.so all futexes are private.  */
+# ifdef __ASSUME_PRIVATE_FUTEX
+#  define __lll_private_flag(fl, private) \
+  ((fl) | FUTEX_PRIVATE_FLAG)
+# else
+#  define __lll_private_flag(fl, private) \
+  ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))
+# endif
+#else
+# ifdef __ASSUME_PRIVATE_FUTEX
+#  define __lll_private_flag(fl, private) \
+  (((fl) | FUTEX_PRIVATE_FLAG) ^ (private))
+# else
+#  define __lll_private_flag(fl, private) \
+  (__builtin_constant_p (private)					      \
+   ? ((private) == 0							      \
+      ? ((fl) | THREAD_GETMEM (THREAD_SELF, header.private_futex))	      \
+      : (fl))								      \
+   : ({ unsigned int __fl = ((private) ^ FUTEX_PRIVATE_FLAG);		      \
+	asm ("andl %%fs:%P1, %0" : "+r" (__fl)				      \
+	     : "i" offsetof (struct pthread, header.private_futex));	      \
+	__fl | (fl); })
+# endif	      
+#endif
 
 /* Initializer for compatibility lock.  */
 #define LLL_MUTEX_LOCK_INITIALIZER		(0)
@@ -169,7 +194,8 @@ LLL_STUB_UNWIND_INFO_END
     register __typeof (val) _val __asm ("edx") = (val);			      \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__status)					      \
-		      : "0" (SYS_futex), "D" (futex), "S" (FUTEX_WAIT),	      \
+		      : "0" (SYS_futex), "D" (futex),			      \
+			"S" (__lll_private_flag (FUTEX_WAIT, private)),	      \
 		        "d" (_val), "r" (__to)				      \
 		      : "memory", "cc", "r11", "cx");			      \
     __status;								      \
@@ -182,73 +208,21 @@ LLL_STUB_UNWIND_INFO_END
     register __typeof (nr) _nr __asm ("edx") = (nr);			      \
     __asm __volatile ("syscall"						      \
 		      : "=a" (__ignore)					      \
-		      : "0" (SYS_futex), "D" (futex), "S" (FUTEX_WAKE),	      \
+		      : "0" (SYS_futex), "D" (futex),			      \
+			"S" (__lll_private_flag (FUTEX_WAKE, private)),	      \
 			"d" (_nr)					      \
 		      : "memory", "cc", "r10", "r11", "cx");		      \
   } while (0)
 
 
 #define lll_private_futex_wait(futex, val) \
-  lll_private_futex_timed_wait (futex, val, NULL)
-
-
-#ifdef __ASSUME_PRIVATE_FUTEX
-# define lll_private_futex_timed_wait(futex, val, timeout) \
-  ({									      \
-    register const struct timespec *__to __asm ("r10") = timeout;	      \
-    int __status;							      \
-    register __typeof (val) _val __asm ("edx") = (val);			      \
-    __asm __volatile ("syscall"						      \
-		      : "=a" (__status)					      \
-		      : "0" (SYS_futex), "D" (futex),			      \
-			"S" (FUTEX_WAIT | FUTEX_PRIVATE_FLAG),		      \
-		        "d" (_val), "r" (__to)				      \
-		      : "memory", "cc", "r11", "cx");			      \
-    __status;								      \
-  })
-
-
-# define lll_private_futex_wake(futex, nr) \
-  do {									      \
-    int __ignore;							      \
-    register __typeof (nr) _nr __asm ("edx") = (nr);			      \
-    __asm __volatile ("syscall"						      \
-		      : "=a" (__ignore)					      \
-		      : "0" (SYS_futex), "D" (futex),			      \
-			"S" (FUTEX_WAKE | FUTEX_PRIVATE_FLAG),		      \
-			"d" (_nr)					      \
-		      : "memory", "cc", "r10", "r11", "cx");		      \
-  } while (0)
-#else
-# define lll_private_futex_timed_wait(futex, val, timeout) \
-  ({									      \
-    register const struct timespec *__to __asm ("r10") = timeout;	      \
-    int __status;							      \
-    int __ignore;							      \
-    register __typeof (val) _val __asm ("edx") = (val);			      \
-    __asm __volatile ("movl %%fs:%P3, %%esi\n\t"			      \
-		      "syscall"						      \
-		      : "=a" (__status), "=S" (__ignore)		      \
-		      : "0" (SYS_futex), "i" (PRIVATE_FUTEX), "D" (futex),    \
-		        "d" (_val), "r" (__to)				      \
-		      : "memory", "cc", "r11", "cx");			      \
-    __status;								      \
-  })
+  lll_futex_timed_wait (futex, val, NULL, LLL_PRIVATE)
 
+#define lll_private_futex_timed_wait(futex, val, timeout) \
+  lll_futex_timed_wait (futex, val, timeout, LLL_PRIVATE)
 
-# define lll_private_futex_wake(futex, nr) \
-  do {									      \
-    int __ignore;							      \
-    int __ignore2;							      \
-    register __typeof (nr) _nr __asm ("edx") = (nr);			      \
-    __asm __volatile ("orl %%fs:%P3, %%esi\n\t"				      \
-		      "syscall"						      \
-		      : "=a" (__ignore), "=S" (__ignore2)		      \
-		      : "0" (SYS_futex), "i" (PRIVATE_FUTEX), "D" (futex),    \
-			"1" (FUTEX_WAKE), "d" (_nr)			      \
-		      : "memory", "cc", "r10", "r11", "cx");		      \
-  } while (0)
-#endif
+#define lll_private_futex_wake(futex, nr) \
+  lll_futex_wake (futex, nr, LLL_PRIVATE)
 
 
 /* Does not preserve %eax and %ecx.  */

	Jakub

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clean up PPC for private futex changes.
  2007-07-23 20:43   ` Jakub Jelinek
@ 2007-07-23 22:09     ` Steven Munroe
  2007-07-24  7:11       ` Jakub Jelinek
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Munroe @ 2007-07-23 22:09 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ulrich Drepper, GNU libc hacker

Jakub Jelinek wrote:

>On Mon, Jul 23, 2007 at 09:10:27AM -0700, Ulrich Drepper wrote:
>  
>
>>-----BEGIN PGP SIGNED MESSAGE-----
>>Hash: SHA1
>>
>>Applied.
>>    
>>
>
>Unfortunately that patch contains several important bugs:
>1) lll_futex_wait etc. had only correct definition when
>   __ASSUME_PRIVATE_FUTEX, when that is not true, it would
>   call futex syscall with bit 7 set even when libpthread.so
>   init determined it is not supported
>  
>
Ya, I was looking at the x86_64 code which did seem incomplete to me. So 
I tried to go one step better for PPC.

But how can header.private_futex be set in the absence of  
__ASSUME_PRIVATE_FUTEX?

>2) lll_private_futex_wait etc. macros were shared when
>   not __ASSUME_PRIVATE_FUTEX
>3) FUTEX_WAKE_OP had the wake operation argument also ored
>   with 128 in some cases
>  
>

Ya, I was concerned about that, but the examples from i386 and x86_64 
where not clear to me...

>Here is how it IMHO should look like (built and tested on ppc64-linux,
>unfortunately not wiht a 2.6.23ish kernel).
>  
>

This looks better. Thanks


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] clean up PPC for private futex changes.
  2007-07-23 22:09     ` Steven Munroe
@ 2007-07-24  7:11       ` Jakub Jelinek
  0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2007-07-24  7:11 UTC (permalink / raw)
  To: Steven Munroe; +Cc: Ulrich Drepper, GNU libc hacker

On Mon, Jul 23, 2007 at 05:09:04PM -0500, Steven Munroe wrote:
> But how can header.private_futex be set in the absence of  
> __ASSUME_PRIVATE_FUTEX?

header.private_futex is only ever present when
!defined __ASSUME_PRIVATE_FUTEX
and contains either 0 (when private futexes are not supported)
or FUTEX_PRIVATE_FLAG (== 128, when private futexes are supported).
This is not needed when __ASSUME_PRIVATE_FUTEX is defined - we
assume private futexes are supported and thus header.private_futex
would be always 128.

	Jakub

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-07-24  7:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-13 20:44 [PATCH] clean up PPC for private futex changes Steven Munroe
2007-07-23 16:10 ` Ulrich Drepper
2007-07-23 20:43   ` Jakub Jelinek
2007-07-23 22:09     ` Steven Munroe
2007-07-24  7:11       ` Jakub Jelinek

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).