* [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
@ 2014-07-02 23:14 Roland McGrath
2014-07-03 13:40 ` Torvald Riegel
2014-07-07 11:26 ` Bernard Ogden
0 siblings, 2 replies; 13+ messages in thread
From: Roland McGrath @ 2014-07-02 23:14 UTC (permalink / raw)
To: GNU C. Library; +Cc: Bernard Ogden
This makes lowlevellock.h generic, and splits out the Linux-specific bits
into a new file lowlevellock-futex.h. Aside from the split, the generic
file is very similar to Bernie's linux-generic version. (I actually
started with the ARM file and worked from there, but then looked at the
differences from Bernie's file.)
I think Bernie's version introduced a regression that's hard to test for.
In his file, all the macros return the "result" of the syscall, i.e. either
a negated errno code or the non-error return value of the syscall.
However, at least one or two places rely on the non-error return value of
at least some of these macros being exactly zero. For example,
lll_futex_requeue is expected to return 0, but the syscall when successful
will usually return a positive number. The failure mode of this regression
is that the code will fall back to thundering-herd wakeups, which is just a
performance regression (but a very important one). So we need to take
great care in looking for those kinds of changes to the compiled code, not
simply verify that the test suite passes.
Unfortunately, the status quo ante is rather haphazard treatment of the
syscall return value in the different macros. So cleaning the macros up
causes more changes to the generated code than I'd like.
I'm still looking at the testing (on arm-linux-gnueabihf). There are some
nontrivial differences I wasn't expecting, and I need to examine them all
more thoroughly. Some are just mysterious, where the compiler made a
different code-generation choice for no apparent reason (like swapping the
order of operands in a comparison and inverting the associated conditional
branch). Other are understandable but stupid, where it does more checking
on the syscall return value than it actually cares about (an example of
this costs three or four extra instructions and an extra branch, but that's
after doing a blocking syscall, so it's lost deeply in the noise). Others
are probably from the new uniformity of the syscall return value handling
but should be harmless. I need to examine them all before claiming they
don't matter, and I won't get to that until next week.
Joseph is away and I don't expect him to approve the change for ARM very
soon. After I've finished the testing and if Bernie and others are happy
with the new code, I figure I'll commit the new files next week but not
remove the ARM file.
I note that the AAarch64 file is well nigh identical to the ARM file and
several others probably are too. So those machines' maintainers could (and
should) remove their files and test the new generic code as soon as they
manage.
2014-07-02 Roland McGrath <roland@hack.frob.com>
* sysdeps/nptl/lowlevellock.h: New file.
* sysdeps/unix/sysv/linux/lowlevellock-futex.h: New file.
* sysdeps/nptl/lowlevellock-futex.h: New file.
* sysdeps/unix/sysv/linux/arm/lowlevellock.h: File removed.
diff --git a/sysdeps/nptl/lowlevellock-futex.h b/sysdeps/nptl/lowlevellock-futex.h
new file mode 100644
index 0000000..12f3876
--- /dev/null
+++ b/sysdeps/nptl/lowlevellock-futex.h
@@ -0,0 +1,86 @@
+/* Low-level locking access to futex facilities. Stub version.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_FUTEX_H
+#define _LOWLEVELLOCK_FUTEX_H 1
+
+#include <errno.h>
+
+
+/* Values for 'private' parameter of locking macros. Note pthreadP.h
+ optimizes for these exact values, though they are not required. */
+#define LLL_PRIVATE 0
+#define LLL_SHARED 128
+
+
+/* For most of these macros, the return value is never really used.
+ Nevertheless, the protocol is that each one returns a negated errno
+ code for failure or zero for success. (Note that the corresponding
+ Linux system calls can sometimes return positive values for success
+ cases too. We never use those values.) */
+
+
+/* Wait while *FUTEXP == VAL for an lll_futex_wake call on FUTEXP. */
+#define lll_futex_wait(futexp, val, private) \
+ lll_futex_timed_wait (futexp, val, NULL, private)
+
+/* Wait until a lll_futex_wake call on FUTEXP, or TIMEOUT elapses. */
+#define lll_futex_timed_wait(futexp, val, timeout, private) \
+ -ENOSYS
+
+/* This macro should be defined only if FUTEX_CLOCK_REALTIME is also defined.
+ If CLOCKBIT is zero, this is identical to lll_futex_timed_wait.
+ If CLOCKBIT has FUTEX_CLOCK_REALTIME set, then it's the same but
+ TIMEOUT is counted by CLOCK_REALTIME rather than CLOCK_MONOTONIC. */
+#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
+ -ENOSYS
+
+/* Wake up up to NR waiters on FUTEXP. */
+#define lll_futex_wake(futexp, nr, private) \
+ -ENOSYS
+
+/* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of the
+ rest from waiting on FUTEXP to waiting on MUTEX (a different futex). */
+#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
+ -ENOSYS
+
+/* Wake up up to NR_WAKE waiters on FUTEXP and NR_WAKE2 on FUTEXP2. */
+#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
+ -ENOSYS
+
+
+/* Like lll_futex_wait (FUTEXP, VAL, PRIVATE) but with the expectation
+ that lll_futex_cmp_requeue_pi (FUTEXP, _, _, MUTEX, _, PRIVATE) will
+ be used to do the wakeup. Confers priority-inheritance behavior on
+ the waiter. */
+#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
+ lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
+
+/* Like lll_futex_wait_requeue_pi, but with a timeout. */
+#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
+ mutex, private) \
+ -ENOSYS
+
+/* Like lll_futex_requeue, but pairs with lll_futex_wait_requeue_pi
+ and inherits priority from the waiter. */
+#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
+ val, private) \
+ -ENOSYS
+
+
+#endif /* lowlevellock-futex.h */
diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
new file mode 100644
index 0000000..e440bb4
--- /dev/null
+++ b/sysdeps/nptl/lowlevellock.h
@@ -0,0 +1,201 @@
+/* Low-level lock implementation. Generic futex-based version.
+ Copyright (C) 2005-2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_H
+#define _LOWLEVELLOCK_H 1
+
+#include <atomic.h>
+#include <lowlevellock-futex.h>
+
+#define lll_robust_dead(futexv, private) \
+ do \
+ { \
+ int *__futexp = &(futexv); \
+ atomic_or (__futexp, FUTEX_OWNER_DIED); \
+ lll_futex_wake (__futexp, 1, private); \
+ } \
+ while (0)
+
+#define lll_trylock(lock) \
+ atomic_compare_and_exchange_bool_acq (&(lock), 1, 0)
+
+#define lll_cond_trylock(lock) \
+ atomic_compare_and_exchange_bool_acq (&(lock), 2, 0)
+
+#define __lll_robust_trylock(futex, id) \
+ (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
+#define lll_robust_trylock(lock, id) \
+ __lll_robust_trylock (&(lock), id)
+
+extern void __lll_lock_wait_private (int *futex) attribute_hidden;
+extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
+extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_lock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
+ { \
+ if (__builtin_constant_p (private) && (private) == LLL_PRIVATE) \
+ __lll_lock_wait_private (__futex); \
+ else \
+ __lll_lock_wait (__futex, private); \
+ } \
+ }))
+#define lll_lock(futex, private) \
+ __lll_lock (&(futex), private)
+
+
+#define __lll_robust_lock(futex, id, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
+ __val = __lll_robust_lock_wait (__futex, private); \
+ __val; \
+ })
+#define lll_robust_lock(futex, id, private) \
+ __lll_robust_lock (&(futex), id, private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_cond_lock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ if (__glibc_unlikely (atomic_exchange_acq (__futex, 2))) \
+ __lll_lock_wait (__futex, private); \
+ }))
+#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
+
+
+#define lll_robust_cond_lock(futex, id, private) \
+ __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
+
+
+extern int __lll_timedlock_wait (int *futex, const struct timespec *,
+ int private) attribute_hidden;
+extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
+ int private) attribute_hidden;
+
+#define __lll_timedlock(futex, abstime, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
+ __val = __lll_timedlock_wait (__futex, abstime, private); \
+ __val; \
+ })
+#define lll_timedlock(futex, abstime, private) \
+ __lll_timedlock (&(futex), abstime, private)
+
+
+#define __lll_robust_timedlock(futex, abstime, id, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
+ __val = __lll_robust_timedlock_wait (__futex, abstime, private); \
+ __val; \
+ })
+#define lll_robust_timedlock(futex, abstime, id, private) \
+ __lll_robust_timedlock (&(futex), abstime, id, private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_unlock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ int __oldval = atomic_exchange_rel (__futex, 0); \
+ if (__glibc_unlikely (__oldval > 1)) \
+ lll_futex_wake (__futex, 1, private); \
+ }))
+#define lll_unlock(futex, private) \
+ __lll_unlock (&(futex), private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_robust_unlock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ int __oldval = atomic_exchange_rel (__futex, 0); \
+ if (__glibc_unlikely (__oldval & FUTEX_WAITERS)) \
+ lll_futex_wake (__futex, 1, private); \
+ }))
+#define lll_robust_unlock(futex, private) \
+ __lll_robust_unlock (&(futex), private)
+
+
+#define lll_islocked(futex) \
+ ((futex) != LLL_LOCK_INITIALIZER)
+
+
+/* Our internal lock implementation is identical to the binary-compatible
+ mutex implementation. */
+
+/* Initializers for lock. */
+#define LLL_LOCK_INITIALIZER (0)
+#define LLL_LOCK_INITIALIZER_LOCKED (1)
+
+/* The states of a lock are:
+ 0 - untaken
+ 1 - taken by one user
+ >1 - taken by more users */
+
+/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
+ wakeup when the clone terminates. The memory location contains the
+ thread ID while the clone is running and is reset to zero
+ afterwards. */
+#define lll_wait_tid(tid) \
+ do { \
+ __typeof (tid) __tid; \
+ while ((__tid = (tid)) != 0) \
+ lll_futex_wait (&(tid), __tid, LLL_SHARED);\
+ } while (0)
+
+extern int __lll_timedwait_tid (int *, const struct timespec *)
+ attribute_hidden;
+
+#define lll_timedwait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ if ((tid) != 0) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ __res; \
+ })
+
+
+#endif /* lowlevellock.h */
diff --git a/sysdeps/unix/sysv/linux/arm/lowlevellock.h b/sysdeps/unix/sysv/linux/arm/lowlevellock.h
deleted file mode 100644
index 5d19434..0000000
--- a/sysdeps/unix/sysv/linux/arm/lowlevellock.h
+++ /dev/null
@@ -1,321 +0,0 @@
-/* Copyright (C) 2005-2014 Free Software Foundation, Inc.
- This file is part of the GNU C Library.
-
- The GNU C Library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- The GNU C Library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with the GNU C Library. If not, see
- <http://www.gnu.org/licenses/>. */
-
-#ifndef _LOWLEVELLOCK_H
-#define _LOWLEVELLOCK_H 1
-
-#include <time.h>
-#include <sys/param.h>
-#include <bits/pthreadtypes.h>
-#include <atomic.h>
-#include <sysdep.h>
-#include <kernel-features.h>
-
-#define FUTEX_WAIT 0
-#define FUTEX_WAKE 1
-#define FUTEX_REQUEUE 3
-#define FUTEX_CMP_REQUEUE 4
-#define FUTEX_WAKE_OP 5
-#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
-#define FUTEX_LOCK_PI 6
-#define FUTEX_UNLOCK_PI 7
-#define FUTEX_TRYLOCK_PI 8
-#define FUTEX_WAIT_BITSET 9
-#define FUTEX_WAKE_BITSET 10
-#define FUTEX_WAIT_REQUEUE_PI 11
-#define FUTEX_CMP_REQUEUE_PI 12
-#define FUTEX_PRIVATE_FLAG 128
-#define FUTEX_CLOCK_REALTIME 256
-
-#define FUTEX_BITSET_MATCH_ANY 0xffffffff
-
-/* 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
-
-
-#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
-
-
-#define lll_futex_wait(futexp, val, private) \
- lll_futex_timed_wait(futexp, val, NULL, private)
-
-#define lll_futex_timed_wait(futexp, val, timespec, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- __ret = INTERNAL_SYSCALL (futex, __err, 4, (futexp), \
- __lll_private_flag (FUTEX_WAIT, private), \
- (val), (timespec)); \
- __ret; \
- })
-
-#define lll_futex_timed_wait_bitset(futexp, val, timespec, clockbit, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- int __op = FUTEX_WAIT_BITSET | clockbit; \
- __ret = INTERNAL_SYSCALL (futex, __err, 6, (futexp), \
- __lll_private_flag (__op, private), \
- (val), (timespec), NULL /* Unused. */, \
- FUTEX_BITSET_MATCH_ANY); \
- __ret; \
- })
-
-#define lll_futex_wake(futexp, nr, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- __ret = INTERNAL_SYSCALL (futex, __err, 4, (futexp), \
- __lll_private_flag (FUTEX_WAKE, private), \
- (nr), 0); \
- __ret; \
- })
-
-#define lll_robust_dead(futexv, private) \
- do \
- { \
- int *__futexp = &(futexv); \
- atomic_or (__futexp, FUTEX_OWNER_DIED); \
- lll_futex_wake (__futexp, 1, private); \
- } \
- while (0)
-
-/* Returns non-zero if error happened, zero if success. */
-#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- __ret = INTERNAL_SYSCALL (futex, __err, 6, (futexp), \
- __lll_private_flag (FUTEX_CMP_REQUEUE, private),\
- (nr_wake), (nr_move), (mutex), (val)); \
- INTERNAL_SYSCALL_ERROR_P (__ret, __err); \
- })
-
-
-/* Returns non-zero if error happened, zero if success. */
-#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- __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); \
- })
-
-/* Priority Inheritance support. */
-#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
- lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
-
-#define lll_futex_timed_wait_requeue_pi(futexp, val, timespec, clockbit, \
- mutex, private) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- int __op = FUTEX_WAIT_REQUEUE_PI | clockbit; \
- \
- INTERNAL_SYSCALL (futex, __err, 5, (futexp), \
- __lll_private_flag (__op, private), \
- (val), (timespec), mutex); \
- })
-
-#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, val, priv) \
- ({ \
- INTERNAL_SYSCALL_DECL (__err); \
- long int __ret; \
- \
- __ret = INTERNAL_SYSCALL (futex, __err, 6, (futexp), \
- __lll_private_flag (FUTEX_CMP_REQUEUE_PI, priv),\
- (nr_wake), (nr_move), (mutex), (val)); \
- INTERNAL_SYSCALL_ERROR_P (__ret, __err); \
- })
-
-
-#define lll_trylock(lock) \
- atomic_compare_and_exchange_val_acq(&(lock), 1, 0)
-
-#define lll_cond_trylock(lock) \
- atomic_compare_and_exchange_val_acq(&(lock), 2, 0)
-
-#define __lll_robust_trylock(futex, id) \
- (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
-#define lll_robust_trylock(lock, id) \
- __lll_robust_trylock (&(lock), id)
-
-extern void __lll_lock_wait_private (int *futex) attribute_hidden;
-extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
-extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
-
-#define __lll_lock(futex, private) \
- ((void) ({ \
- int *__futex = (futex); \
- if (__builtin_expect (atomic_compare_and_exchange_val_acq (__futex, \
- 1, 0), 0)) \
- { \
- if (__builtin_constant_p (private) && (private) == LLL_PRIVATE) \
- __lll_lock_wait_private (__futex); \
- else \
- __lll_lock_wait (__futex, private); \
- } \
- }))
-#define lll_lock(futex, private) __lll_lock (&(futex), private)
-
-
-#define __lll_robust_lock(futex, id, private) \
- ({ \
- int *__futex = (futex); \
- int __val = 0; \
- \
- if (__builtin_expect (atomic_compare_and_exchange_bool_acq (__futex, id, \
- 0), 0)) \
- __val = __lll_robust_lock_wait (__futex, private); \
- __val; \
- })
-#define lll_robust_lock(futex, id, private) \
- __lll_robust_lock (&(futex), id, private)
-
-
-#define __lll_cond_lock(futex, private) \
- ((void) ({ \
- int *__futex = (futex); \
- if (__builtin_expect (atomic_exchange_acq (__futex, 2), 0)) \
- __lll_lock_wait (__futex, private); \
- }))
-#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
-
-
-#define lll_robust_cond_lock(futex, id, private) \
- __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
-
-
-extern int __lll_timedlock_wait (int *futex, const struct timespec *,
- int private) attribute_hidden;
-extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
- int private) attribute_hidden;
-
-#define __lll_timedlock(futex, abstime, private) \
- ({ \
- int *__futex = (futex); \
- int __val = 0; \
- \
- if (__builtin_expect (atomic_exchange_acq (__futex, 1), 0)) \
- __val = __lll_timedlock_wait (__futex, abstime, private); \
- __val; \
- })
-#define lll_timedlock(futex, abstime, private) \
- __lll_timedlock (&(futex), abstime, private)
-
-
-#define __lll_robust_timedlock(futex, abstime, id, private) \
- ({ \
- int *__futex = (futex); \
- int __val = 0; \
- \
- if (__builtin_expect (atomic_compare_and_exchange_bool_acq (__futex, id, \
- 0), 0)) \
- __val = __lll_robust_timedlock_wait (__futex, abstime, private); \
- __val; \
- })
-#define lll_robust_timedlock(futex, abstime, id, private) \
- __lll_robust_timedlock (&(futex), abstime, id, private)
-
-
-#define __lll_unlock(futex, private) \
- (void) \
- ({ int *__futex = (futex); \
- int __oldval = atomic_exchange_rel (__futex, 0); \
- if (__builtin_expect (__oldval > 1, 0)) \
- lll_futex_wake (__futex, 1, private); \
- })
-#define lll_unlock(futex, private) __lll_unlock(&(futex), private)
-
-
-#define __lll_robust_unlock(futex, private) \
- (void) \
- ({ int *__futex = (futex); \
- int __oldval = atomic_exchange_rel (__futex, 0); \
- if (__builtin_expect (__oldval & FUTEX_WAITERS, 0)) \
- lll_futex_wake (__futex, 1, private); \
- })
-#define lll_robust_unlock(futex, private) \
- __lll_robust_unlock(&(futex), private)
-
-
-#define lll_islocked(futex) \
- (futex != 0)
-
-
-/* Our internal lock implementation is identical to the binary-compatible
- mutex implementation. */
-
-/* Initializers for lock. */
-#define LLL_LOCK_INITIALIZER (0)
-#define LLL_LOCK_INITIALIZER_LOCKED (1)
-
-/* The states of a lock are:
- 0 - untaken
- 1 - taken by one user
- >1 - taken by more users */
-
-/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
- wakeup when the clone terminates. The memory location contains the
- thread ID while the clone is running and is reset to zero
- afterwards. */
-#define lll_wait_tid(tid) \
- do { \
- __typeof (tid) __tid; \
- while ((__tid = (tid)) != 0) \
- lll_futex_wait (&(tid), __tid, LLL_SHARED);\
- } while (0)
-
-extern int __lll_timedwait_tid (int *, const struct timespec *)
- attribute_hidden;
-
-#define lll_timedwait_tid(tid, abstime) \
- ({ \
- int __res = 0; \
- if ((tid) != 0) \
- __res = __lll_timedwait_tid (&(tid), (abstime)); \
- __res; \
- })
-
-#endif /* lowlevellock.h */
diff --git a/sysdeps/unix/sysv/linux/lowlevellock-futex.h b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
new file mode 100644
index 0000000..343afd6
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
@@ -0,0 +1,137 @@
+/* Low-level locking access to futex facilities. Linux version.
+ Copyright (C) 2005-2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_FUTEX_H
+#define _LOWLEVELLOCK_FUTEX_H 1
+
+#include <sysdep.h>
+#include <tls.h>
+#include <kernel-features.h>
+
+
+#define FUTEX_WAIT 0
+#define FUTEX_WAKE 1
+#define FUTEX_REQUEUE 3
+#define FUTEX_CMP_REQUEUE 4
+#define FUTEX_WAKE_OP 5
+#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
+#define FUTEX_LOCK_PI 6
+#define FUTEX_UNLOCK_PI 7
+#define FUTEX_TRYLOCK_PI 8
+#define FUTEX_WAIT_BITSET 9
+#define FUTEX_WAKE_BITSET 10
+#define FUTEX_WAIT_REQUEUE_PI 11
+#define FUTEX_CMP_REQUEUE_PI 12
+#define FUTEX_PRIVATE_FLAG 128
+#define FUTEX_CLOCK_REALTIME 256
+
+#define FUTEX_BITSET_MATCH_ANY 0xffffffff
+
+/* 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
+
+
+#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
+
+#define lll_futex_syscall(nargs, futexp, op, ...) \
+ ({ \
+ INTERNAL_SYSCALL_DECL (__err); \
+ long int __ret = INTERNAL_SYSCALL (futex, __err, nargs, futexp, op, \
+ __VA_ARGS__); \
+ (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret, __err)) \
+ ? -INTERNAL_SYSCALL_ERRNO (__ret, __err) : 0); \
+ })
+
+#define lll_futex_wait(futexp, val, private) \
+ lll_futex_timed_wait (futexp, val, NULL, private)
+
+#define lll_futex_timed_wait(futexp, val, timeout, private) \
+ lll_futex_syscall (4, futexp, \
+ __lll_private_flag (FUTEX_WAIT, private), \
+ val, timeout)
+
+#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_WAIT_BITSET | (clockbit), \
+ private), \
+ val, timeout, NULL /* Unused. */, \
+ FUTEX_BITSET_MATCH_ANY)
+
+#define lll_futex_wake(futexp, nr, private) \
+ lll_futex_syscall (4, futexp, \
+ __lll_private_flag (FUTEX_WAKE, private), nr, 0)
+
+/* Returns non-zero if error happened, zero if success. */
+#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_CMP_REQUEUE, private), \
+ nr_wake, nr_move, mutex, val)
+
+/* Returns non-zero if error happened, zero if success. */
+#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_WAKE_OP, private), \
+ nr_wake, nr_wake2, futexp2, \
+ FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)
+
+/* Priority Inheritance support. */
+#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
+ lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
+
+#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
+ mutex, private) \
+ lll_futex_syscall (5, futexp, \
+ __lll_private_flag (FUTEX_WAIT_REQUEUE_PI \
+ | (clockbit), private), \
+ val, timeout, mutex)
+
+
+#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
+ val, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_CMP_REQUEUE_PI, \
+ private), \
+ nr_wake, nr_move, mutex, val)
+
+
+#endif /* lowlevellock-futex.h */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-02 23:14 [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one Roland McGrath
@ 2014-07-03 13:40 ` Torvald Riegel
2014-07-04 3:10 ` Roland McGrath
2014-07-07 11:26 ` Bernard Ogden
1 sibling, 1 reply; 13+ messages in thread
From: Torvald Riegel @ 2014-07-03 13:40 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library, Bernard Ogden
On Wed, 2014-07-02 at 16:14 -0700, Roland McGrath wrote:
> diff --git a/sysdeps/nptl/lowlevellock-futex.h b/sysdeps/nptl/lowlevellock-futex.h
> new file mode 100644
> index 0000000..12f3876
> --- /dev/null
> +++ b/sysdeps/nptl/lowlevellock-futex.h
> @@ -0,0 +1,86 @@
> +/* Low-level locking access to futex facilities. Stub version.
> + Copyright (C) 2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _LOWLEVELLOCK_FUTEX_H
> +#define _LOWLEVELLOCK_FUTEX_H 1
> +
> +#include <errno.h>
> +
> +
> +/* Values for 'private' parameter of locking macros. Note pthreadP.h
> + optimizes for these exact values, though they are not required. */
> +#define LLL_PRIVATE 0
> +#define LLL_SHARED 128
> +
> +
> +/* For most of these macros, the return value is never really used.
> + Nevertheless, the protocol is that each one returns a negated errno
> + code for failure or zero for success. (Note that the corresponding
> + Linux system calls can sometimes return positive values for success
> + cases too. We never use those values.) */
> +
> +
> +/* Wait while *FUTEXP == VAL for an lll_futex_wake call on FUTEXP. */
> +#define lll_futex_wait(futexp, val, private) \
> + lll_futex_timed_wait (futexp, val, NULL, private)
> +
> +/* Wait until a lll_futex_wake call on FUTEXP, or TIMEOUT elapses. */
> +#define lll_futex_timed_wait(futexp, val, timeout, private) \
> + -ENOSYS
I'd prefer if the comment would explicitly mention spurious wake-ups;
what you write could be implementable but is not precisely how we use
futexes. Maybe just say that it waits until a real or a spurious
wake-up occurs?
> +/* This macro should be defined only if FUTEX_CLOCK_REALTIME is also defined.
> + If CLOCKBIT is zero, this is identical to lll_futex_timed_wait.
> + If CLOCKBIT has FUTEX_CLOCK_REALTIME set, then it's the same but
> + TIMEOUT is counted by CLOCK_REALTIME rather than CLOCK_MONOTONIC. */
> +#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
> + -ENOSYS
> +
> +/* Wake up up to NR waiters on FUTEXP. */
> +#define lll_futex_wake(futexp, nr, private) \
> + -ENOSYS
> +
> +/* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of the
> + rest from waiting on FUTEXP to waiting on MUTEX (a different futex). */
> +#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
> + -ENOSYS
Call mutex futexp2 (or something else) instead? (It's just a parameter
name, but it's also not one of our mutexes, really...)
> +
> +/* Wake up up to NR_WAKE waiters on FUTEXP and NR_WAKE2 on FUTEXP2. */
> +#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
> + -ENOSYS
> +
> +
> +/* Like lll_futex_wait (FUTEXP, VAL, PRIVATE) but with the expectation
> + that lll_futex_cmp_requeue_pi (FUTEXP, _, _, MUTEX, _, PRIVATE) will
> + be used to do the wakeup. Confers priority-inheritance behavior on
> + the waiter. */
> +#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
> + lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
> +
> +/* Like lll_futex_wait_requeue_pi, but with a timeout. */
> +#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
> + mutex, private) \
> + -ENOSYS
> +
> +/* Like lll_futex_requeue, but pairs with lll_futex_wait_requeue_pi
> + and inherits priority from the waiter. */
> +#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
> + val, private) \
> + -ENOSYS
> +
> +
> +#endif /* lowlevellock-futex.h */
> diff --git a/sysdeps/nptl/lowlevellock.h b/sysdeps/nptl/lowlevellock.h
> new file mode 100644
> index 0000000..e440bb4
> --- /dev/null
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -0,0 +1,201 @@
> +/* Low-level lock implementation. Generic futex-based version.
> + Copyright (C) 2005-2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _LOWLEVELLOCK_H
> +#define _LOWLEVELLOCK_H 1
> +
> +#include <atomic.h>
> +#include <lowlevellock-futex.h>
I believe it would be good if we could document these later on; now that
there's a generic version, there's (more or less) just one place we'd
need documentation at.
> +#define lll_robust_dead(futexv, private) \
> + do \
> + { \
> + int *__futexp = &(futexv); \
> + atomic_or (__futexp, FUTEX_OWNER_DIED); \
> + lll_futex_wake (__futexp, 1, private); \
> + } \
> + while (0)
> +
> +#define lll_trylock(lock) \
> + atomic_compare_and_exchange_bool_acq (&(lock), 1, 0)
> +
> +#define lll_cond_trylock(lock) \
> + atomic_compare_and_exchange_bool_acq (&(lock), 2, 0)
> +
> +#define __lll_robust_trylock(futex, id) \
> + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
Use atomic_compare_and_exchange_bool_acq (or clean that up later)?
> +#define lll_robust_trylock(lock, id) \
> + __lll_robust_trylock (&(lock), id)
> +
> +extern void __lll_lock_wait_private (int *futex) attribute_hidden;
> +extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
> +extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_lock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
> + { \
> + if (__builtin_constant_p (private) && (private) == LLL_PRIVATE) \
> + __lll_lock_wait_private (__futex); \
> + else \
> + __lll_lock_wait (__futex, private); \
> + } \
> + }))
> +#define lll_lock(futex, private) \
> + __lll_lock (&(futex), private)
> +
> +
> +#define __lll_robust_lock(futex, id, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
> + __val = __lll_robust_lock_wait (__futex, private); \
> + __val; \
> + })
> +#define lll_robust_lock(futex, id, private) \
> + __lll_robust_lock (&(futex), id, private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_cond_lock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 2))) \
> + __lll_lock_wait (__futex, private); \
> + }))
> +#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
> +
> +
> +#define lll_robust_cond_lock(futex, id, private) \
> + __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
> +
> +
> +extern int __lll_timedlock_wait (int *futex, const struct timespec *,
> + int private) attribute_hidden;
> +extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
> + int private) attribute_hidden;
> +
> +#define __lll_timedlock(futex, abstime, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
> + __val = __lll_timedlock_wait (__futex, abstime, private); \
> + __val; \
> + })
> +#define lll_timedlock(futex, abstime, private) \
> + __lll_timedlock (&(futex), abstime, private)
This differs from __lll_lock (e.g., CAS vs. atomic_exchange). I suppose
that resembles the current code we have; nonetheless, I find it
surprising (e.g., lll_lock will not overwrite a value of 2 in the lock
(ie, contended), whereas timedlock will). That changes whether a
subsequent lll_unlock will do a lll_futex_wake or not.
> +
> +
> +#define __lll_robust_timedlock(futex, abstime, id, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
> + __val = __lll_robust_timedlock_wait (__futex, abstime, private); \
> + __val; \
> + })
> +#define lll_robust_timedlock(futex, abstime, id, private) \
> + __lll_robust_timedlock (&(futex), abstime, id, private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_unlock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + int __oldval = atomic_exchange_rel (__futex, 0); \
> + if (__glibc_unlikely (__oldval > 1)) \
> + lll_futex_wake (__futex, 1, private); \
> + }))
> +#define lll_unlock(futex, private) \
> + __lll_unlock (&(futex), private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_robust_unlock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + int __oldval = atomic_exchange_rel (__futex, 0); \
> + if (__glibc_unlikely (__oldval & FUTEX_WAITERS)) \
> + lll_futex_wake (__futex, 1, private); \
> + }))
> +#define lll_robust_unlock(futex, private) \
> + __lll_robust_unlock (&(futex), private)
> +
> +
> +#define lll_islocked(futex) \
> + ((futex) != LLL_LOCK_INITIALIZER)
> +
> +
> +/* Our internal lock implementation is identical to the binary-compatible
> + mutex implementation. */
> +
> +/* Initializers for lock. */
> +#define LLL_LOCK_INITIALIZER (0)
> +#define LLL_LOCK_INITIALIZER_LOCKED (1)
> +
> +/* The states of a lock are:
> + 0 - untaken
> + 1 - taken by one user
I guess "(not) acquired" is a better term here?
> + >1 - taken by more users */
A mutex is never acquired by more than one thread. IIRC, this state
means that the lock is acquired by one thread, and other threads _might_
be trying to acquire the lock as well (including being blocked on the
futex).
> +
> +/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
> + wakeup when the clone terminates. The memory location contains the
> + thread ID while the clone is running and is reset to zero
> + afterwards. */
> +#define lll_wait_tid(tid) \
> + do { \
> + __typeof (tid) __tid; \
> + while ((__tid = (tid)) != 0) \
> + lll_futex_wait (&(tid), __tid, LLL_SHARED);\
> + } while (0)
> +
> +extern int __lll_timedwait_tid (int *, const struct timespec *)
> + attribute_hidden;
> +
> +#define lll_timedwait_tid(tid, abstime) \
> + ({ \
> + int __res = 0; \
> + if ((tid) != 0) \
> + __res = __lll_timedwait_tid (&(tid), (abstime)); \
> + __res; \
> + })
> +
> +
> +#endif /* lowlevellock.h */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-03 13:40 ` Torvald Riegel
@ 2014-07-04 3:10 ` Roland McGrath
0 siblings, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2014-07-04 3:10 UTC (permalink / raw)
To: Torvald Riegel; +Cc: GNU C. Library, Bernard Ogden
> I'd prefer if the comment would explicitly mention spurious wake-ups;
> what you write could be implementable but is not precisely how we use
> futexes. Maybe just say that it waits until a real or a spurious
> wake-up occurs?
I made a modicum of effort to document the macros' protocols in the stub
file because it's the generally right thing to do. But I was just barely
cobbling together rough and terse descriptions of what I wasn't quite sure
things actually did. I'd love it if you could provide detailed replacement
comments for the stub file that explain the contract the genericish NPTL
source code expects from these macros. Separately, if there's something
useful to say about how the Linux implementation bridges the gap between
that contract and the kernel's exact contract to put into comments in the
Linux file, I'd appreciate that very much as well.
> Call mutex futexp2 (or something else) instead? (It's just a parameter
> name, but it's also not one of our mutexes, really...)
I took the old ARM file as the model and didn't change most names.
But I can change these however you think looks best.
> I believe it would be good if we could document these later on; now that
> there's a generic version, there's (more or less) just one place we'd
> need documentation at.
Feel free! :-)
> > +#define __lll_robust_trylock(futex, id) \
> > + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
>
> Use atomic_compare_and_exchange_bool_acq (or clean that up later)?
I changed a couple of cases that were val in the old ARM file to be bool
where it was obvious to me that the intended semantics were clearly bool
(which did not affect the generated code for ARM). This one I was less
sure about, but I guess the semantics of that value test are exactly the
same here as the bool version so I should change it too.
> This differs from __lll_lock (e.g., CAS vs. atomic_exchange). I suppose
> that resembles the current code we have; nonetheless, I find it
> surprising (e.g., lll_lock will not overwrite a value of 2 in the lock
> (ie, contended), whereas timedlock will). That changes whether a
> subsequent lll_unlock will do a lll_futex_wake or not.
This change is intended not to materially change the compiled code at all
if possible. Once we have many more machines consolidated on using a
generic implementation, then you should by all means go to town on
substantive cleanups like this.
> > +/* The states of a lock are:
> > + 0 - untaken
> > + 1 - taken by one user
>
> I guess "(not) acquired" is a better term here?
>
> > + >1 - taken by more users */
>
> A mutex is never acquired by more than one thread. IIRC, this state
> means that the lock is acquired by one thread, and other threads _might_
> be trying to acquire the lock as well (including being blocked on the
> futex).
Here I just preserved the existing comments from the old code. Once my
change lands, please send your own changes to do comment cleanups and the
like. (A pure comment change like this that is indisputably clarifying
things can go in as "obvious enough" without needing formal review.)
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-02 23:14 [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one Roland McGrath
2014-07-03 13:40 ` Torvald Riegel
@ 2014-07-07 11:26 ` Bernard Ogden
2014-07-07 12:03 ` Andreas Schwab
2014-07-09 21:54 ` Roland McGrath
1 sibling, 2 replies; 13+ messages in thread
From: Bernard Ogden @ 2014-07-07 11:26 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library
Ugh - yes, I missed that some syscalls would return non-zero for success.
I have a few comments, 2 about other things I missed before, and 1 about the bug I was trying to fix.
On 3 Jul 2014, at 00:14, Roland McGrath <roland@hack.frob.com> wrote:
> +#define __lll_robust_trylock(futex, id) \
> + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
My first thought was that the != 0 is superfluous. Either way you return 0 if you changed the memory, and non-zero if you didn't.
But looking at the call in pthread_mutex_trylock.c, I wonder if both patches have this wrong. Should we be returning the old value here? That's what (at least) m68k does, so there's a difference that'll need resolving.
> +#define __lll_cond_lock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 2))) \
> + __lll_lock_wait (__futex, private); \
> + }))
This is unconditionally setting the futex to 2, where my patch (based on Tile) sets it to 2 only if it was previously 0. Again, the platforms are inconsistent so it'll need resolving.
> +#define __lll_timedlock(futex, abstime, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
> + __val = __lll_timedlock_wait (__futex, abstime, private); \
> + __val; \
> + })
This'll spread BZ 16892 (and be a binary difference on the currently unaffected platforms).
Should be something like:
if (__glibc_unlikely (atomic_compare_and_exchange_bool_acq (__futex, 1, 0)))
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-07 11:26 ` Bernard Ogden
@ 2014-07-07 12:03 ` Andreas Schwab
2014-07-09 21:54 ` Roland McGrath
1 sibling, 0 replies; 13+ messages in thread
From: Andreas Schwab @ 2014-07-07 12:03 UTC (permalink / raw)
To: Bernard Ogden; +Cc: Roland McGrath, GNU C. Library
Bernard Ogden <bernie.ogden@linaro.org> writes:
> On 3 Jul 2014, at 00:14, Roland McGrath <roland@hack.frob.com> wrote:
>
>> +#define __lll_robust_trylock(futex, id) \
>> + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
>
> My first thought was that the != 0 is superfluous. Either way you return 0
> if you changed the memory, and non-zero if you didn't.
>
> But looking at the call in pthread_mutex_trylock.c, I wonder if both
> patches have this wrong. Should we be returning the old value here? That's
> what (at least) m68k does, so there's a difference that'll need resolving.
Looks like x86 and powerpc do that too, and all other archs got this
wrong.
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] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-07 11:26 ` Bernard Ogden
2014-07-07 12:03 ` Andreas Schwab
@ 2014-07-09 21:54 ` Roland McGrath
2014-07-14 14:56 ` Bernard Ogden
1 sibling, 1 reply; 13+ messages in thread
From: Roland McGrath @ 2014-07-09 21:54 UTC (permalink / raw)
To: Bernard Ogden; +Cc: GNU C. Library, marcus.shawcroft
> I have a few comments, 2 about other things I missed before, and 1 about
> the bug I was trying to fix.
Thanks very much for the review.
My inclination is to start out with a version like mine that doesn't change
any code from what it does now on at least one platform. (I chose ARM just
because that's what I'm working on first for the non-Linux port where this
refactoring matters.) Once it's in and being used by ARM (and/or one or
more other machines where the code matches today), then we can do these
other fixes incrementally. That is: first refactor without change; then fix.
> On 3 Jul 2014, at 00:14, Roland McGrath <roland@hack.frob.com> wrote:
>
> > +#define __lll_robust_trylock(futex, id) \
> > + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
>
> My first thought was that the != 0 is superfluous. Either way you return
> 0 if you changed the memory, and non-zero if you didn't.
>
> But looking at the call in pthread_mutex_trylock.c, I wonder if both
> patches have this wrong. Should we be returning the old value here?
> That's what (at least) m68k does, so there's a difference that'll need
> resolving.
Indeed this seems clearly wrong. Unfortunately it looks to me like the
scenario where it makes a difference can only come up in a race. The logic
in pthread_mutex_trylock is somewhat intricate, so I'm not completely sure.
Can you see a way to come up with a (non-racy) regression test for this?
I've looked at all the machines' implementations of lll_robust_trylock.
Most look just like ARM, i.e. using atomic_compare_and_exchange_val_acq
and comparing against zero.
s390 open-codes in assembly exactly what the
atomic_compare_and_exchange_val_acq would do, and then does the
erroneous comparison in C.
sh open-codes in assembly exactly what the
atomic_compare_and_exchange_val_acq along with the erroneous comparison
would do.
i386, x86_64, and powerpc open-code in assembly exactly what the
atomic_compare_and_exchange_val_acq would do, and don't do the erroneous
comparison.
m68k uses atomic_compare_and_exchange_val_acq without the erroneous
comparison.
The sole use of lll_robust_trylock is in pthread_mutex_trylock, right
after code that just uses atomic_compare_and_exchange_val_acq directly
on the same memory location.
So I think what we should really do is just remove lll_robust_trylock
entirely. It doesn't let the machine encapsulate any additional magic,
because the generic code already assumes exactly what the use of the
location has to be. This could be done either before or after my patch.
> > +#define __lll_cond_lock(futex, private) \
> > + ((void) \
> > + ({ \
> > + int *__futex = (futex); \
> > + if (__glibc_unlikely (atomic_exchange_acq (__futex, 2))) \
> > + __lll_lock_wait (__futex, private); \
> > + }))
>
> This is unconditionally setting the futex to 2, where my patch (based
> on Tile) sets it to 2 only if it was previously 0. Again, the
> platforms are inconsistent so it'll need resolving.
The lay of the land is pretty similar for this case. Many have exactly
equivalent code. Many others have code that uses one of
atomic_compare_and_exchange_{bool,val}_acq and so match the behavior of
yours rather than mine. A few others open-code assembly that is
equivalent to one or the other.
Unlike lll_robust_trylock, there is one outlier: sparc is almost the
same (as yours), but uses atomic_compare_and_exchange_val_24_acq (a
special sparcism). So it's not a candidate for removing entirely even
though it too has only one user (pthread_mutex_cond_lock.c).
Off hand I don't understand the use here well enough to be completely
sure which is correct. In the contended case, they all get to
__lll_lock_wait. The generic C and the x86 assembly versions of that do
the equivalent of the unconditional atomic_exchange_acq. I suspect that
is actually what's correct: if it was 0 before that's the
unlocked/uncontended case; if it was 1 before, your code leaves it 1
(until it gets into __lll_lock_wait, anyway) but there is now
contention, so it should be 2; when it gets into __lll_lock_wait, that
will do an unconditional atomic_exchange_acq anyway.
Given this control flow, I think it's impossible to write any test that
can tell the difference. Your version is just going to be slightly
suboptimal in the contended case, because it will skip installing 2 and
then do an additional atomic operation in __lll_lock_wait to install it
before going into the kernel.
So we just have to reason it out. Do you agree with my reasoning above?
I've updated my version on the branch to have an explicit != 0 in the
comparison to make it slightly more clear. (All this stuff could use
vastly more comments.)
> > +#define __lll_timedlock(futex, abstime, private) \
> > + ({ \
> > + int *__futex = (futex); \
> > + int __val = 0; \
> > + \
> > + if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
> > + __val = __lll_timedlock_wait (__futex, abstime, private); \
> > + __val; \
> > + })
>
> This'll spread BZ 16892 (and be a binary difference on the currently
> unaffected platforms).
>
> Should be something like:
>
> if (__glibc_unlikely (atomic_compare_and_exchange_bool_acq (__futex, 1, 0)))
OK. I concur with the rationale described in the bug. But as I said
above, I'm inclined to separate that fix to be done after the pure
refactoring has gone in.
I think what I'll do now is proceed with working up a change to
eliminate lll_robust_trylock in the status quo ante. I'm going on the
assumption that we'll conclude it's impossible to test for that issue,
but please do put your mind to coming up with a useful test case (or
else contributing to the belief that it can't be done).
That will leave just the latter two issues to be resolved after the
refactoring goes in. For lll_cond_lock, I think what we want is no
change from my version (and the status quo ante for ARM), but that needs
to be confirmed by other people contemplating the logic. For
lll_timedlock, I do think your change is correct and we'll do it after.
Actually changing ARM has to wait for Joseph to come back next week and
approve it. By the time he's back, I'll be travelling for Cauldron and
then I can't be relied on to do anything before the 28th (though I might
well do some things while travelling).
I pointed out earlier that the aarch64 file is nearly identical to the
arm file. So if Marcus is around now and able to review and approve
before Joseph is back, we could make aarch64 the first to use the new
generic file instead of arm. (I can do a build for aarch64 and verify
that it has no effect on the compiled code.) Then I think we could
proceed with the follow-on fixes and let ARM and other machines drop
their now-redundant files at their maintainers' leisure to pick up those
fixes.
Assuming nobody tells me why any of this is a bad idea, I'll proceed in
that direction now.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-09 21:54 ` Roland McGrath
@ 2014-07-14 14:56 ` Bernard Ogden
2014-07-15 7:38 ` Bernie Ogden
2014-07-15 22:38 ` [COMMITTED PATCH] " Roland McGrath
0 siblings, 2 replies; 13+ messages in thread
From: Bernard Ogden @ 2014-07-14 14:56 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library, marcus.shawcroft
On 9 Jul 2014, at 22:54, Roland McGrath <roland@hack.frob.com> wrote:
>> On 3 Jul 2014, at 00:14, Roland McGrath <roland@hack.frob.com> wrote:
>>
>>> +#define __lll_robust_trylock(futex, id) \
>>> + (atomic_compare_and_exchange_val_acq (futex, id, 0) != 0)
>>
>> My first thought was that the != 0 is superfluous. Either way you return
>> 0 if you changed the memory, and non-zero if you didn't.
>>
>> But looking at the call in pthread_mutex_trylock.c, I wonder if both
>> patches have this wrong. Should we be returning the old value here?
>> That's what (at least) m68k does, so there's a difference that'll need
>> resolving.
>
> Indeed this seems clearly wrong. Unfortunately it looks to me like the
> scenario where it makes a difference can only come up in a race. The logic
> in pthread_mutex_trylock is somewhat intricate, so I'm not completely sure.
> Can you see a way to come up with a (non-racy) regression test for this?
To test that particular case, it looks like we'd have to force a thread to die in an unclean way in the middle of the function. I don't think we can do that.
Perhaps there should be a test that we get EOWNERDEAD if a lock holder dies in an unclean way before the call to __pthread_mutex_trylock - I didn't see one and I guess that would be doable with a pair of threads. But that's getting further away from issues relating to this patch.
>
>>> +#define __lll_cond_lock(futex, private) \
>>> + ((void) \
>>> + ({ \
>>> + int *__futex = (futex); \
>>> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 2))) \
>>> + __lll_lock_wait (__futex, private); \
>>> + }))
>>
>> This is unconditionally setting the futex to 2, where my patch (based
>> on Tile) sets it to 2 only if it was previously 0. Again, the
>> platforms are inconsistent so it'll need resolving.
>
> The lay of the land is pretty similar for this case. Many have exactly
> equivalent code. Many others have code that uses one of
> atomic_compare_and_exchange_{bool,val}_acq and so match the behavior of
> yours rather than mine. A few others open-code assembly that is
> equivalent to one or the other.
>
> Unlike lll_robust_trylock, there is one outlier: sparc is almost the
> same (as yours), but uses atomic_compare_and_exchange_val_24_acq (a
> special sparcism). So it's not a candidate for removing entirely even
> though it too has only one user (pthread_mutex_cond_lock.c).
>
> Off hand I don't understand the use here well enough to be completely
> sure which is correct. In the contended case, they all get to
> __lll_lock_wait. The generic C and the x86 assembly versions of that do
> the equivalent of the unconditional atomic_exchange_acq. I suspect that
> is actually what's correct: if it was 0 before that's the
> unlocked/uncontended case; if it was 1 before, your code leaves it 1
> (until it gets into __lll_lock_wait, anyway) but there is now
> contention, so it should be 2; when it gets into __lll_lock_wait, that
> will do an unconditional atomic_exchange_acq anyway.
>
> Given this control flow, I think it's impossible to write any test that
> can tell the difference. Your version is just going to be slightly
> suboptimal in the contended case, because it will skip installing 2 and
> then do an additional atomic operation in __lll_lock_wait to install it
> before going into the kernel.
>
> So we just have to reason it out. Do you agree with my reasoning above?
I only learned about condition variables at all through looking at this patch, but I think your reasoning is correct.
>
>>> +#define __lll_timedlock(futex, abstime, private) \
>>> + ({ \
>>> + int *__futex = (futex); \
>>> + int __val = 0; \
>>> + \
>>> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
>>> + __val = __lll_timedlock_wait (__futex, abstime, private); \
>>> + __val; \
>>> + })
>>
>> This'll spread BZ 16892 (and be a binary difference on the currently
>> unaffected platforms).
>>
>> Should be something like:
>>
>> if (__glibc_unlikely (atomic_compare_and_exchange_bool_acq (__futex, 1, 0)))
>
> OK. I concur with the rationale described in the bug. But as I said
> above, I'm inclined to separate that fix to be done after the pure
> refactoring has gone in.
That makes sense.
So in summary:
__lll_robust_trylock: Has already been removed, we can't test.
__lll_cond_lock: Your patch looks fine.
__lll_timedlock: To be fixed once the refactoring has gone in.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-14 14:56 ` Bernard Ogden
@ 2014-07-15 7:38 ` Bernie Ogden
2014-07-15 22:38 ` [COMMITTED PATCH] " Roland McGrath
1 sibling, 0 replies; 13+ messages in thread
From: Bernie Ogden @ 2014-07-15 7:38 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library, Marcus Shawcroft
> Perhaps there should be a test that we get EOWNERDEAD if a lock holder dies in an unclean way before the call to __pthread_mutex_trylock - I didn't see one and I guess that would be doable with a pair of threads. But that's getting further away from issues relating to this patch.
Actually there is already a test for EOWNERDEAD (tst-robust2 and
tst-robust5). Just not in the racy case.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [COMMITTED PATCH] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-14 14:56 ` Bernard Ogden
2014-07-15 7:38 ` Bernie Ogden
@ 2014-07-15 22:38 ` Roland McGrath
2014-07-16 8:07 ` Bernie Ogden
2014-07-18 21:42 ` Andreas Schwab
1 sibling, 2 replies; 13+ messages in thread
From: Roland McGrath @ 2014-07-15 22:38 UTC (permalink / raw)
To: GNU C. Library; +Cc: Bernard Ogden
I've committed the new files that were previously posted on the
roland/lll-futex branch. This does not affect any existing configuration
yet. Each machine can simply remove its lowlevellock.h file to start using
the generic code and see how it compares.
This "generic" code is based on the existing ARM and AArch64 code. It's
known to have the BZ#16892 bug but we'd like to fix that only after some
machine(s) have switched to actually using this code so that there can be
separate tree states for converting the machine to use the generic code,
which is intended to have zero semantic effect and expected to have minimal
effect on the compiled code; and then fixing BZ#16892, which changes the
semantics slightly.
I've build-tested this on arm-linux-gnueabihf and aarch64-linux-gnu, by
removing sysdeps/unix/sysv/linux/CPU/lowlevellock.h. On both those
machines, there were a lot more changes to generated code than I was
expecting, but in everything I examined the differences seemed to be in
arbitrary code-generation choices by the compiler and not directly related
to the lowlevellock.h differences. I don't understand why those
differences appeared. Someone who groks the compiler well could look at
them and try to make sense of it for our edification.
I've also run the test suite on arm-linux-gnueabihf to compare the status
quo ante with this code (and arm/lowlevellock.h removed, which is not
committed yet). There were no regressions.
Thanks,
Roland
* sysdeps/nptl/lowlevellock.h: New file.
* sysdeps/unix/sysv/linux/lowlevellock-futex.h: New file.
* sysdeps/nptl/lowlevellock-futex.h: New file.
--- /dev/null
+++ b/sysdeps/nptl/lowlevellock-futex.h
@@ -0,0 +1,86 @@
+/* Low-level locking access to futex facilities. Stub version.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_FUTEX_H
+#define _LOWLEVELLOCK_FUTEX_H 1
+
+#include <errno.h>
+
+
+/* Values for 'private' parameter of locking macros. Note pthreadP.h
+ optimizes for these exact values, though they are not required. */
+#define LLL_PRIVATE 0
+#define LLL_SHARED 128
+
+
+/* For most of these macros, the return value is never really used.
+ Nevertheless, the protocol is that each one returns a negated errno
+ code for failure or zero for success. (Note that the corresponding
+ Linux system calls can sometimes return positive values for success
+ cases too. We never use those values.) */
+
+
+/* Wait while *FUTEXP == VAL for an lll_futex_wake call on FUTEXP. */
+#define lll_futex_wait(futexp, val, private) \
+ lll_futex_timed_wait (futexp, val, NULL, private)
+
+/* Wait until a lll_futex_wake call on FUTEXP, or TIMEOUT elapses. */
+#define lll_futex_timed_wait(futexp, val, timeout, private) \
+ -ENOSYS
+
+/* This macro should be defined only if FUTEX_CLOCK_REALTIME is also defined.
+ If CLOCKBIT is zero, this is identical to lll_futex_timed_wait.
+ If CLOCKBIT has FUTEX_CLOCK_REALTIME set, then it's the same but
+ TIMEOUT is counted by CLOCK_REALTIME rather than CLOCK_MONOTONIC. */
+#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
+ -ENOSYS
+
+/* Wake up up to NR waiters on FUTEXP. */
+#define lll_futex_wake(futexp, nr, private) \
+ -ENOSYS
+
+/* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of the
+ rest from waiting on FUTEXP to waiting on MUTEX (a different futex). */
+#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
+ -ENOSYS
+
+/* Wake up up to NR_WAKE waiters on FUTEXP and NR_WAKE2 on FUTEXP2. */
+#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
+ -ENOSYS
+
+
+/* Like lll_futex_wait (FUTEXP, VAL, PRIVATE) but with the expectation
+ that lll_futex_cmp_requeue_pi (FUTEXP, _, _, MUTEX, _, PRIVATE) will
+ be used to do the wakeup. Confers priority-inheritance behavior on
+ the waiter. */
+#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
+ lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
+
+/* Like lll_futex_wait_requeue_pi, but with a timeout. */
+#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
+ mutex, private) \
+ -ENOSYS
+
+/* Like lll_futex_requeue, but pairs with lll_futex_wait_requeue_pi
+ and inherits priority from the waiter. */
+#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
+ val, private) \
+ -ENOSYS
+
+
+#endif /* lowlevellock-futex.h */
--- /dev/null
+++ b/sysdeps/nptl/lowlevellock.h
@@ -0,0 +1,187 @@
+/* Low-level lock implementation. Generic futex-based version.
+ Copyright (C) 2005-2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_H
+#define _LOWLEVELLOCK_H 1
+
+#include <atomic.h>
+#include <lowlevellock-futex.h>
+
+#define lll_trylock(lock) \
+ atomic_compare_and_exchange_bool_acq (&(lock), 1, 0)
+
+#define lll_cond_trylock(lock) \
+ atomic_compare_and_exchange_bool_acq (&(lock), 2, 0)
+
+extern void __lll_lock_wait_private (int *futex) attribute_hidden;
+extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
+extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_lock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
+ { \
+ if (__builtin_constant_p (private) && (private) == LLL_PRIVATE) \
+ __lll_lock_wait_private (__futex); \
+ else \
+ __lll_lock_wait (__futex, private); \
+ } \
+ }))
+#define lll_lock(futex, private) \
+ __lll_lock (&(futex), private)
+
+
+#define __lll_robust_lock(futex, id, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
+ __val = __lll_robust_lock_wait (__futex, private); \
+ __val; \
+ })
+#define lll_robust_lock(futex, id, private) \
+ __lll_robust_lock (&(futex), id, private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_cond_lock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ if (__glibc_unlikely (atomic_exchange_acq (__futex, 2) != 0)) \
+ __lll_lock_wait (__futex, private); \
+ }))
+#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
+
+
+#define lll_robust_cond_lock(futex, id, private) \
+ __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
+
+
+extern int __lll_timedlock_wait (int *futex, const struct timespec *,
+ int private) attribute_hidden;
+extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
+ int private) attribute_hidden;
+
+#define __lll_timedlock(futex, abstime, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
+ __val = __lll_timedlock_wait (__futex, abstime, private); \
+ __val; \
+ })
+#define lll_timedlock(futex, abstime, private) \
+ __lll_timedlock (&(futex), abstime, private)
+
+
+#define __lll_robust_timedlock(futex, abstime, id, private) \
+ ({ \
+ int *__futex = (futex); \
+ int __val = 0; \
+ \
+ if (__glibc_unlikely \
+ (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
+ __val = __lll_robust_timedlock_wait (__futex, abstime, private); \
+ __val; \
+ })
+#define lll_robust_timedlock(futex, abstime, id, private) \
+ __lll_robust_timedlock (&(futex), abstime, id, private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_unlock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ int __oldval = atomic_exchange_rel (__futex, 0); \
+ if (__glibc_unlikely (__oldval > 1)) \
+ lll_futex_wake (__futex, 1, private); \
+ }))
+#define lll_unlock(futex, private) \
+ __lll_unlock (&(futex), private)
+
+
+/* This is an expression rather than a statement even though its value is
+ void, so that it can be used in a comma expression or as an expression
+ that's cast to void. */
+#define __lll_robust_unlock(futex, private) \
+ ((void) \
+ ({ \
+ int *__futex = (futex); \
+ int __oldval = atomic_exchange_rel (__futex, 0); \
+ if (__glibc_unlikely (__oldval & FUTEX_WAITERS)) \
+ lll_futex_wake (__futex, 1, private); \
+ }))
+#define lll_robust_unlock(futex, private) \
+ __lll_robust_unlock (&(futex), private)
+
+
+#define lll_islocked(futex) \
+ ((futex) != LLL_LOCK_INITIALIZER)
+
+
+/* Our internal lock implementation is identical to the binary-compatible
+ mutex implementation. */
+
+/* Initializers for lock. */
+#define LLL_LOCK_INITIALIZER (0)
+#define LLL_LOCK_INITIALIZER_LOCKED (1)
+
+/* The states of a lock are:
+ 0 - untaken
+ 1 - taken by one user
+ >1 - taken by more users */
+
+/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
+ wakeup when the clone terminates. The memory location contains the
+ thread ID while the clone is running and is reset to zero
+ afterwards. */
+#define lll_wait_tid(tid) \
+ do { \
+ __typeof (tid) __tid; \
+ while ((__tid = (tid)) != 0) \
+ lll_futex_wait (&(tid), __tid, LLL_SHARED);\
+ } while (0)
+
+extern int __lll_timedwait_tid (int *, const struct timespec *)
+ attribute_hidden;
+
+#define lll_timedwait_tid(tid, abstime) \
+ ({ \
+ int __res = 0; \
+ if ((tid) != 0) \
+ __res = __lll_timedwait_tid (&(tid), (abstime)); \
+ __res; \
+ })
+
+
+#endif /* lowlevellock.h */
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
@@ -0,0 +1,137 @@
+/* Low-level locking access to futex facilities. Linux version.
+ Copyright (C) 2005-2014 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _LOWLEVELLOCK_FUTEX_H
+#define _LOWLEVELLOCK_FUTEX_H 1
+
+#include <sysdep.h>
+#include <tls.h>
+#include <kernel-features.h>
+
+
+#define FUTEX_WAIT 0
+#define FUTEX_WAKE 1
+#define FUTEX_REQUEUE 3
+#define FUTEX_CMP_REQUEUE 4
+#define FUTEX_WAKE_OP 5
+#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
+#define FUTEX_LOCK_PI 6
+#define FUTEX_UNLOCK_PI 7
+#define FUTEX_TRYLOCK_PI 8
+#define FUTEX_WAIT_BITSET 9
+#define FUTEX_WAKE_BITSET 10
+#define FUTEX_WAIT_REQUEUE_PI 11
+#define FUTEX_CMP_REQUEUE_PI 12
+#define FUTEX_PRIVATE_FLAG 128
+#define FUTEX_CLOCK_REALTIME 256
+
+#define FUTEX_BITSET_MATCH_ANY 0xffffffff
+
+/* 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
+
+
+#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
+
+#define lll_futex_syscall(nargs, futexp, op, ...) \
+ ({ \
+ INTERNAL_SYSCALL_DECL (__err); \
+ long int __ret = INTERNAL_SYSCALL (futex, __err, nargs, futexp, op, \
+ __VA_ARGS__); \
+ (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret, __err)) \
+ ? -INTERNAL_SYSCALL_ERRNO (__ret, __err) : 0); \
+ })
+
+#define lll_futex_wait(futexp, val, private) \
+ lll_futex_timed_wait (futexp, val, NULL, private)
+
+#define lll_futex_timed_wait(futexp, val, timeout, private) \
+ lll_futex_syscall (4, futexp, \
+ __lll_private_flag (FUTEX_WAIT, private), \
+ val, timeout)
+
+#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_WAIT_BITSET | (clockbit), \
+ private), \
+ val, timeout, NULL /* Unused. */, \
+ FUTEX_BITSET_MATCH_ANY)
+
+#define lll_futex_wake(futexp, nr, private) \
+ lll_futex_syscall (4, futexp, \
+ __lll_private_flag (FUTEX_WAKE, private), nr, 0)
+
+/* Returns non-zero if error happened, zero if success. */
+#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_CMP_REQUEUE, private), \
+ nr_wake, nr_move, mutex, val)
+
+/* Returns non-zero if error happened, zero if success. */
+#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_WAKE_OP, private), \
+ nr_wake, nr_wake2, futexp2, \
+ FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)
+
+/* Priority Inheritance support. */
+#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
+ lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
+
+#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
+ mutex, private) \
+ lll_futex_syscall (5, futexp, \
+ __lll_private_flag (FUTEX_WAIT_REQUEUE_PI \
+ | (clockbit), private), \
+ val, timeout, mutex)
+
+
+#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
+ val, private) \
+ lll_futex_syscall (6, futexp, \
+ __lll_private_flag (FUTEX_CMP_REQUEUE_PI, \
+ private), \
+ nr_wake, nr_move, mutex, val)
+
+
+#endif /* lowlevellock-futex.h */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [COMMITTED PATCH] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-15 22:38 ` [COMMITTED PATCH] " Roland McGrath
@ 2014-07-16 8:07 ` Bernie Ogden
2014-07-16 16:45 ` Roland McGrath
2014-07-18 21:42 ` Andreas Schwab
1 sibling, 1 reply; 13+ messages in thread
From: Bernie Ogden @ 2014-07-16 8:07 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library
There is only zero semantic/minimal codegen effect for those machines
currently affected by BZ#16892 - those being m68k, aarch64, arm and
sh/sh4. So perhaps the thing to do is have those machines switch
first, confirm the effect is as expected and then fix the bug before
switching the others.
On 15 July 2014 23:38, Roland McGrath <roland@hack.frob.com> wrote:
> I've committed the new files that were previously posted on the
> roland/lll-futex branch. This does not affect any existing configuration
> yet. Each machine can simply remove its lowlevellock.h file to start using
> the generic code and see how it compares.
>
> This "generic" code is based on the existing ARM and AArch64 code. It's
> known to have the BZ#16892 bug but we'd like to fix that only after some
> machine(s) have switched to actually using this code so that there can be
> separate tree states for converting the machine to use the generic code,
> which is intended to have zero semantic effect and expected to have minimal
> effect on the compiled code; and then fixing BZ#16892, which changes the
> semantics slightly.
>
> I've build-tested this on arm-linux-gnueabihf and aarch64-linux-gnu, by
> removing sysdeps/unix/sysv/linux/CPU/lowlevellock.h. On both those
> machines, there were a lot more changes to generated code than I was
> expecting, but in everything I examined the differences seemed to be in
> arbitrary code-generation choices by the compiler and not directly related
> to the lowlevellock.h differences. I don't understand why those
> differences appeared. Someone who groks the compiler well could look at
> them and try to make sense of it for our edification.
>
> I've also run the test suite on arm-linux-gnueabihf to compare the status
> quo ante with this code (and arm/lowlevellock.h removed, which is not
> committed yet). There were no regressions.
>
>
> Thanks,
> Roland
>
>
> * sysdeps/nptl/lowlevellock.h: New file.
> * sysdeps/unix/sysv/linux/lowlevellock-futex.h: New file.
> * sysdeps/nptl/lowlevellock-futex.h: New file.
>
> --- /dev/null
> +++ b/sysdeps/nptl/lowlevellock-futex.h
> @@ -0,0 +1,86 @@
> +/* Low-level locking access to futex facilities. Stub version.
> + Copyright (C) 2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _LOWLEVELLOCK_FUTEX_H
> +#define _LOWLEVELLOCK_FUTEX_H 1
> +
> +#include <errno.h>
> +
> +
> +/* Values for 'private' parameter of locking macros. Note pthreadP.h
> + optimizes for these exact values, though they are not required. */
> +#define LLL_PRIVATE 0
> +#define LLL_SHARED 128
> +
> +
> +/* For most of these macros, the return value is never really used.
> + Nevertheless, the protocol is that each one returns a negated errno
> + code for failure or zero for success. (Note that the corresponding
> + Linux system calls can sometimes return positive values for success
> + cases too. We never use those values.) */
> +
> +
> +/* Wait while *FUTEXP == VAL for an lll_futex_wake call on FUTEXP. */
> +#define lll_futex_wait(futexp, val, private) \
> + lll_futex_timed_wait (futexp, val, NULL, private)
> +
> +/* Wait until a lll_futex_wake call on FUTEXP, or TIMEOUT elapses. */
> +#define lll_futex_timed_wait(futexp, val, timeout, private) \
> + -ENOSYS
> +
> +/* This macro should be defined only if FUTEX_CLOCK_REALTIME is also defined.
> + If CLOCKBIT is zero, this is identical to lll_futex_timed_wait.
> + If CLOCKBIT has FUTEX_CLOCK_REALTIME set, then it's the same but
> + TIMEOUT is counted by CLOCK_REALTIME rather than CLOCK_MONOTONIC. */
> +#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
> + -ENOSYS
> +
> +/* Wake up up to NR waiters on FUTEXP. */
> +#define lll_futex_wake(futexp, nr, private) \
> + -ENOSYS
> +
> +/* Wake up up to NR_WAKE waiters on FUTEXP. Move up to NR_MOVE of the
> + rest from waiting on FUTEXP to waiting on MUTEX (a different futex). */
> +#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
> + -ENOSYS
> +
> +/* Wake up up to NR_WAKE waiters on FUTEXP and NR_WAKE2 on FUTEXP2. */
> +#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
> + -ENOSYS
> +
> +
> +/* Like lll_futex_wait (FUTEXP, VAL, PRIVATE) but with the expectation
> + that lll_futex_cmp_requeue_pi (FUTEXP, _, _, MUTEX, _, PRIVATE) will
> + be used to do the wakeup. Confers priority-inheritance behavior on
> + the waiter. */
> +#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
> + lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
> +
> +/* Like lll_futex_wait_requeue_pi, but with a timeout. */
> +#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
> + mutex, private) \
> + -ENOSYS
> +
> +/* Like lll_futex_requeue, but pairs with lll_futex_wait_requeue_pi
> + and inherits priority from the waiter. */
> +#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
> + val, private) \
> + -ENOSYS
> +
> +
> +#endif /* lowlevellock-futex.h */
> --- /dev/null
> +++ b/sysdeps/nptl/lowlevellock.h
> @@ -0,0 +1,187 @@
> +/* Low-level lock implementation. Generic futex-based version.
> + Copyright (C) 2005-2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _LOWLEVELLOCK_H
> +#define _LOWLEVELLOCK_H 1
> +
> +#include <atomic.h>
> +#include <lowlevellock-futex.h>
> +
> +#define lll_trylock(lock) \
> + atomic_compare_and_exchange_bool_acq (&(lock), 1, 0)
> +
> +#define lll_cond_trylock(lock) \
> + atomic_compare_and_exchange_bool_acq (&(lock), 2, 0)
> +
> +extern void __lll_lock_wait_private (int *futex) attribute_hidden;
> +extern void __lll_lock_wait (int *futex, int private) attribute_hidden;
> +extern int __lll_robust_lock_wait (int *futex, int private) attribute_hidden;
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_lock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, 1, 0))) \
> + { \
> + if (__builtin_constant_p (private) && (private) == LLL_PRIVATE) \
> + __lll_lock_wait_private (__futex); \
> + else \
> + __lll_lock_wait (__futex, private); \
> + } \
> + }))
> +#define lll_lock(futex, private) \
> + __lll_lock (&(futex), private)
> +
> +
> +#define __lll_robust_lock(futex, id, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
> + __val = __lll_robust_lock_wait (__futex, private); \
> + __val; \
> + })
> +#define lll_robust_lock(futex, id, private) \
> + __lll_robust_lock (&(futex), id, private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_cond_lock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 2) != 0)) \
> + __lll_lock_wait (__futex, private); \
> + }))
> +#define lll_cond_lock(futex, private) __lll_cond_lock (&(futex), private)
> +
> +
> +#define lll_robust_cond_lock(futex, id, private) \
> + __lll_robust_lock (&(futex), (id) | FUTEX_WAITERS, private)
> +
> +
> +extern int __lll_timedlock_wait (int *futex, const struct timespec *,
> + int private) attribute_hidden;
> +extern int __lll_robust_timedlock_wait (int *futex, const struct timespec *,
> + int private) attribute_hidden;
> +
> +#define __lll_timedlock(futex, abstime, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely (atomic_exchange_acq (__futex, 1))) \
> + __val = __lll_timedlock_wait (__futex, abstime, private); \
> + __val; \
> + })
> +#define lll_timedlock(futex, abstime, private) \
> + __lll_timedlock (&(futex), abstime, private)
> +
> +
> +#define __lll_robust_timedlock(futex, abstime, id, private) \
> + ({ \
> + int *__futex = (futex); \
> + int __val = 0; \
> + \
> + if (__glibc_unlikely \
> + (atomic_compare_and_exchange_bool_acq (__futex, id, 0))) \
> + __val = __lll_robust_timedlock_wait (__futex, abstime, private); \
> + __val; \
> + })
> +#define lll_robust_timedlock(futex, abstime, id, private) \
> + __lll_robust_timedlock (&(futex), abstime, id, private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_unlock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + int __oldval = atomic_exchange_rel (__futex, 0); \
> + if (__glibc_unlikely (__oldval > 1)) \
> + lll_futex_wake (__futex, 1, private); \
> + }))
> +#define lll_unlock(futex, private) \
> + __lll_unlock (&(futex), private)
> +
> +
> +/* This is an expression rather than a statement even though its value is
> + void, so that it can be used in a comma expression or as an expression
> + that's cast to void. */
> +#define __lll_robust_unlock(futex, private) \
> + ((void) \
> + ({ \
> + int *__futex = (futex); \
> + int __oldval = atomic_exchange_rel (__futex, 0); \
> + if (__glibc_unlikely (__oldval & FUTEX_WAITERS)) \
> + lll_futex_wake (__futex, 1, private); \
> + }))
> +#define lll_robust_unlock(futex, private) \
> + __lll_robust_unlock (&(futex), private)
> +
> +
> +#define lll_islocked(futex) \
> + ((futex) != LLL_LOCK_INITIALIZER)
> +
> +
> +/* Our internal lock implementation is identical to the binary-compatible
> + mutex implementation. */
> +
> +/* Initializers for lock. */
> +#define LLL_LOCK_INITIALIZER (0)
> +#define LLL_LOCK_INITIALIZER_LOCKED (1)
> +
> +/* The states of a lock are:
> + 0 - untaken
> + 1 - taken by one user
> + >1 - taken by more users */
> +
> +/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
> + wakeup when the clone terminates. The memory location contains the
> + thread ID while the clone is running and is reset to zero
> + afterwards. */
> +#define lll_wait_tid(tid) \
> + do { \
> + __typeof (tid) __tid; \
> + while ((__tid = (tid)) != 0) \
> + lll_futex_wait (&(tid), __tid, LLL_SHARED);\
> + } while (0)
> +
> +extern int __lll_timedwait_tid (int *, const struct timespec *)
> + attribute_hidden;
> +
> +#define lll_timedwait_tid(tid, abstime) \
> + ({ \
> + int __res = 0; \
> + if ((tid) != 0) \
> + __res = __lll_timedwait_tid (&(tid), (abstime)); \
> + __res; \
> + })
> +
> +
> +#endif /* lowlevellock.h */
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
> @@ -0,0 +1,137 @@
> +/* Low-level locking access to futex facilities. Linux version.
> + Copyright (C) 2005-2014 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library. If not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#ifndef _LOWLEVELLOCK_FUTEX_H
> +#define _LOWLEVELLOCK_FUTEX_H 1
> +
> +#include <sysdep.h>
> +#include <tls.h>
> +#include <kernel-features.h>
> +
> +
> +#define FUTEX_WAIT 0
> +#define FUTEX_WAKE 1
> +#define FUTEX_REQUEUE 3
> +#define FUTEX_CMP_REQUEUE 4
> +#define FUTEX_WAKE_OP 5
> +#define FUTEX_OP_CLEAR_WAKE_IF_GT_ONE ((4 << 24) | 1)
> +#define FUTEX_LOCK_PI 6
> +#define FUTEX_UNLOCK_PI 7
> +#define FUTEX_TRYLOCK_PI 8
> +#define FUTEX_WAIT_BITSET 9
> +#define FUTEX_WAKE_BITSET 10
> +#define FUTEX_WAIT_REQUEUE_PI 11
> +#define FUTEX_CMP_REQUEUE_PI 12
> +#define FUTEX_PRIVATE_FLAG 128
> +#define FUTEX_CLOCK_REALTIME 256
> +
> +#define FUTEX_BITSET_MATCH_ANY 0xffffffff
> +
> +/* 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
> +
> +
> +#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
> +
> +#define lll_futex_syscall(nargs, futexp, op, ...) \
> + ({ \
> + INTERNAL_SYSCALL_DECL (__err); \
> + long int __ret = INTERNAL_SYSCALL (futex, __err, nargs, futexp, op, \
> + __VA_ARGS__); \
> + (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (__ret, __err)) \
> + ? -INTERNAL_SYSCALL_ERRNO (__ret, __err) : 0); \
> + })
> +
> +#define lll_futex_wait(futexp, val, private) \
> + lll_futex_timed_wait (futexp, val, NULL, private)
> +
> +#define lll_futex_timed_wait(futexp, val, timeout, private) \
> + lll_futex_syscall (4, futexp, \
> + __lll_private_flag (FUTEX_WAIT, private), \
> + val, timeout)
> +
> +#define lll_futex_timed_wait_bitset(futexp, val, timeout, clockbit, private) \
> + lll_futex_syscall (6, futexp, \
> + __lll_private_flag (FUTEX_WAIT_BITSET | (clockbit), \
> + private), \
> + val, timeout, NULL /* Unused. */, \
> + FUTEX_BITSET_MATCH_ANY)
> +
> +#define lll_futex_wake(futexp, nr, private) \
> + lll_futex_syscall (4, futexp, \
> + __lll_private_flag (FUTEX_WAKE, private), nr, 0)
> +
> +/* Returns non-zero if error happened, zero if success. */
> +#define lll_futex_requeue(futexp, nr_wake, nr_move, mutex, val, private) \
> + lll_futex_syscall (6, futexp, \
> + __lll_private_flag (FUTEX_CMP_REQUEUE, private), \
> + nr_wake, nr_move, mutex, val)
> +
> +/* Returns non-zero if error happened, zero if success. */
> +#define lll_futex_wake_unlock(futexp, nr_wake, nr_wake2, futexp2, private) \
> + lll_futex_syscall (6, futexp, \
> + __lll_private_flag (FUTEX_WAKE_OP, private), \
> + nr_wake, nr_wake2, futexp2, \
> + FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)
> +
> +/* Priority Inheritance support. */
> +#define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
> + lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
> +
> +#define lll_futex_timed_wait_requeue_pi(futexp, val, timeout, clockbit, \
> + mutex, private) \
> + lll_futex_syscall (5, futexp, \
> + __lll_private_flag (FUTEX_WAIT_REQUEUE_PI \
> + | (clockbit), private), \
> + val, timeout, mutex)
> +
> +
> +#define lll_futex_cmp_requeue_pi(futexp, nr_wake, nr_move, mutex, \
> + val, private) \
> + lll_futex_syscall (6, futexp, \
> + __lll_private_flag (FUTEX_CMP_REQUEUE_PI, \
> + private), \
> + nr_wake, nr_move, mutex, val)
> +
> +
> +#endif /* lowlevellock-futex.h */
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [COMMITTED PATCH] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-16 8:07 ` Bernie Ogden
@ 2014-07-16 16:45 ` Roland McGrath
0 siblings, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2014-07-16 16:45 UTC (permalink / raw)
To: Bernie Ogden; +Cc: GNU C. Library
> There is only zero semantic/minimal codegen effect for those machines
> currently affected by BZ#16892 - those being m68k, aarch64, arm and
> sh/sh4. So perhaps the thing to do is have those machines switch
> first, confirm the effect is as expected and then fix the bug before
> switching the others.
That seems reasonble. But of course it is up to each machine maintainer.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [COMMITTED PATCH] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-15 22:38 ` [COMMITTED PATCH] " Roland McGrath
2014-07-16 8:07 ` Bernie Ogden
@ 2014-07-18 21:42 ` Andreas Schwab
2014-07-19 23:55 ` Roland McGrath
1 sibling, 1 reply; 13+ messages in thread
From: Andreas Schwab @ 2014-07-18 21:42 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C. Library, Bernard Ogden
I've now removed sysdeps/unix/sysv/linux/m68k/lowlevellock.h to use the
generic version. There were some expected code changes (due to the use
of lll_futex_syscall throughout), but no regression.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [COMMITTED PATCH] Separate Linuxisms from lowlevellock.h, make a generic one
2014-07-18 21:42 ` Andreas Schwab
@ 2014-07-19 23:55 ` Roland McGrath
0 siblings, 0 replies; 13+ messages in thread
From: Roland McGrath @ 2014-07-19 23:55 UTC (permalink / raw)
To: Andreas Schwab; +Cc: GNU C. Library, Bernard Ogden
> I've now removed sysdeps/unix/sysv/linux/m68k/lowlevellock.h to use the
> generic version. There were some expected code changes (due to the use
> of lll_futex_syscall throughout), but no regression.
Great! Do you want to try fixing BZ#16892 in the generic code now? I'm
not sure if we should wait for another machine (perhaps a more heavily used
one, such as ARM) to switch as well.
Thanks,
Roland
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-07-19 23:55 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-02 23:14 [PATCH roland/lll-futex] Separate Linuxisms from lowlevellock.h, make a generic one Roland McGrath
2014-07-03 13:40 ` Torvald Riegel
2014-07-04 3:10 ` Roland McGrath
2014-07-07 11:26 ` Bernard Ogden
2014-07-07 12:03 ` Andreas Schwab
2014-07-09 21:54 ` Roland McGrath
2014-07-14 14:56 ` Bernard Ogden
2014-07-15 7:38 ` Bernie Ogden
2014-07-15 22:38 ` [COMMITTED PATCH] " Roland McGrath
2014-07-16 8:07 ` Bernie Ogden
2014-07-16 16:45 ` Roland McGrath
2014-07-18 21:42 ` Andreas Schwab
2014-07-19 23:55 ` Roland McGrath
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).