From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32132 invoked by alias); 23 Jul 2007 20:43:58 -0000 Received: (qmail 32116 invoked by uid 22791); 23 Jul 2007 20:43:58 -0000 X-Spam-Check-By: sourceware.org Received: from sunsite.ms.mff.cuni.cz (HELO sunsite.mff.cuni.cz) (195.113.15.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 23 Jul 2007 20:43:52 +0000 Received: from sunsite.mff.cuni.cz (localhost.localdomain [127.0.0.1]) by sunsite.mff.cuni.cz (8.13.8/8.13.8) with ESMTP id l6NKgcuQ006426; Mon, 23 Jul 2007 22:42:38 +0200 Received: (from jakub@localhost) by sunsite.mff.cuni.cz (8.13.8/8.13.8/Submit) id l6NKgbPk006425; Mon, 23 Jul 2007 22:42:37 +0200 Date: Mon, 23 Jul 2007 20:43:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Steven Munroe , GNU libc hacker Subject: Re: [PATCH] clean up PPC for private futex changes. Message-ID: <20070723204237.GE4603@sunsite.mff.cuni.cz> Reply-To: Jakub Jelinek References: <4697E92C.1010403@us.ibm.com> <46A4D2F3.7060005@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46A4D2F3.7060005@redhat.com> User-Agent: Mutt/1.4.2.2i Mailing-List: contact libc-hacker-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sourceware.org X-SW-Source: 2007-07/txt/msg00040.txt.bz2 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 * 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