This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Ping: [PATCH][s390][ppc] Add REQUEUE_PI support
- From: Siddhesh Poyarekar <siddhesh at redhat dot com>
- To: libc-alpha at sourceware dot org
- Cc: krebbel at linux dot vnet dot ibm dot com, rsa at us dot ibm dot com
- Date: Mon, 4 Feb 2013 17:16:26 +0530
- Subject: Ping: [PATCH][s390][ppc] Add REQUEUE_PI support
- References: <20130108132702.GD27464@spoyarek.pnq.redhat.com>
Ping!
On Tue, Jan 08, 2013 at 06:57:02PM +0530, Siddhesh Poyarekar wrote:
> Hi,
>
> Support for priority inheritance in pthread condition variables using
> FUTEX_WAIT_REQUEUE_PI and FUTEX_CMP_REQUEUE_PI have been in x86 code
> since some time, but the generic code was never updated to make this
> available for other architectures. Attached patch modifies the
> generic code to make use of FUTEX_*REQUEUE_PI for condition variables
> backed by PI-aware mutexes whenever the underlying architecture
> supports it and also adds support for powerpc and s390. I have
> verified that there are no regressions in the testsuite due to this
> patch on s390x and ppc64. Once this goes in, other architectures can
> add this support by simply defining lll_futex_wait_requeue_pi,
> lll_futex_timed_wait_requeue_pi and lll_futex_cmp_requeue_pi, similar
> to the way I have done in s390 and ppc and compiling against a kernel
> newer than 2.6.31. I'll send out a separate email on ports once the
> patch is in.
>
> Siddhesh
>
> nptl/ChangeLog:
>
> * pthreadP.h (USE_REQUEUE_PI): New macro to check if mutex is
> PI-aware.
> * pthread_cond_broadcast.c (__pthread_cond_broadcast): Use
> PI-aware futex operations if available and mutex is PI-aware.
> * pthread_cond_signal.c (__pthread_cond_signal): Likewise.
> * nptl/pthread_cond_timedwait.c (__pthread_cond_timedwait):
> Likewise.
> * pthread_cond_wait.c (__condvar_cleanup): Adjust lock if
> cancellation occurred just after futex returned successfully
> from a PI operation with the mutex held.
> (__pthread_cond_wait): Use PI-aware futex operations if
> available and mutex is PI-aware.
> * sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
> (FUTEX_WAIT_REQUEUE_PI): Define.
> (FUTEX_CMP_REQUEUE_PI): Likewise.
> (lll_futex_wait_requeue_pi): Likewise.
> (lll_futex_timed_wait_requeue_pi): Likewise.
> (lll_futex_cmp_requeue_pi): Likewise.
> * nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
> (FUTEX_WAIT_REQUEUE_PI): Define.
> (FUTEX_CMP_REQUEUE_PI): Likewise.
> (lll_futex_wait_requeue_pi): Likewise.
> (lll_futex_timed_wait_requeue_pi): Likewise.
> (lll_futex_cmp_requeue_pi): Likewise.
> * sysdeps/unix/sysv/linux/kernel-features.h: Define
> __ASSUME_REQUEUE_PI for Linux version higher than 2.6.31.
>
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 993a79e..d08b219 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -577,4 +577,16 @@ extern void __wait_lookup_done (void) attribute_hidden;
> # define PTHREAD_STATIC_FN_REQUIRE(name) __asm (".globl " #name);
> #endif
>
> +/* Test if the mutex is suitable for the FUTEX_WAIT_REQUEUE_PI operation. */
> +#if (defined lll_futex_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> +# define USE_REQUEUE_PI(mut) \
> + ((mut) && (mut) != (void *) ~0l \
> + && (((mut)->__data.__kind \
> + & (PTHREAD_MUTEX_PRIO_INHERIT_NP | PTHREAD_MUTEX_ROBUST_NORMAL_NP)) \
> + == PTHREAD_MUTEX_PRIO_INHERIT_NP))
> +#else
> +# define USE_REQUEUE_PI(mut) 0
> +#endif
> +
> #endif /* pthreadP.h */
> diff --git a/nptl/pthread_cond_broadcast.c b/nptl/pthread_cond_broadcast.c
> index 968ee03..0702ec0 100644
> --- a/nptl/pthread_cond_broadcast.c
> +++ b/nptl/pthread_cond_broadcast.c
> @@ -53,34 +53,37 @@ __pthread_cond_broadcast (cond)
> /* We are done. */
> lll_unlock (cond->__data.__lock, pshared);
>
> - /* Do not use requeue for pshared condvars. */
> - if (cond->__data.__mutex == (void *) ~0l)
> - goto wake_all;
> -
> /* Wake everybody. */
> pthread_mutex_t *mut = (pthread_mutex_t *) cond->__data.__mutex;
>
> - /* XXX: Kernel so far doesn't support requeue to PI futex. */
> - /* XXX: Kernel so far can only requeue to the same type of futex,
> - in this case private (we don't requeue for pshared condvars). */
> - if (__builtin_expect (mut->__data.__kind
> - & (PTHREAD_MUTEX_PRIO_INHERIT_NP
> - | PTHREAD_MUTEX_PSHARED_BIT), 0))
> + /* Do not use requeue for pshared condvars. */
> + if (mut == (void *) ~0l
> + || PTHREAD_MUTEX_PSHARED (mut) & PTHREAD_MUTEX_PSHARED_BIT)
> goto wake_all;
>
> - /* lll_futex_requeue returns 0 for success and non-zero
> - for errors. */
> - if (__builtin_expect (lll_futex_requeue (&cond->__data.__futex, 1,
> - INT_MAX, &mut->__data.__lock,
> - futex_val, LLL_PRIVATE), 0))
> +#if (defined lll_futex_cmp_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + int pi_flag = PTHREAD_MUTEX_PRIO_INHERIT_NP | PTHREAD_MUTEX_ROBUST_NP;
> + pi_flag &= mut->__data.__kind;
> +
> + if (pi_flag == PTHREAD_MUTEX_PRIO_INHERIT_NP)
> {
> - /* The requeue functionality is not available. */
> - wake_all:
> - lll_futex_wake (&cond->__data.__futex, INT_MAX, pshared);
> + if (lll_futex_cmp_requeue_pi (&cond->__data.__futex, 1, INT_MAX,
> + &mut->__data.__lock, futex_val,
> + LLL_PRIVATE) == 0)
> + return 0;
> }
> -
> - /* That's all. */
> - return 0;
> + else
> +#endif
> + /* lll_futex_requeue returns 0 for success and non-zero
> + for errors. */
> + if (!__builtin_expect (lll_futex_requeue (&cond->__data.__futex, 1,
> + INT_MAX, &mut->__data.__lock,
> + futex_val, LLL_PRIVATE), 0))
> + return 0;
> +
> +wake_all:
> + lll_futex_wake (&cond->__data.__futex, INT_MAX, pshared);
> }
>
> /* We are done. */
> diff --git a/nptl/pthread_cond_signal.c b/nptl/pthread_cond_signal.c
> index 908a2ac..102d0b3 100644
> --- a/nptl/pthread_cond_signal.c
> +++ b/nptl/pthread_cond_signal.c
> @@ -47,12 +47,35 @@ __pthread_cond_signal (cond)
> ++cond->__data.__wakeup_seq;
> ++cond->__data.__futex;
>
> - /* Wake one. */
> - if (! __builtin_expect (lll_futex_wake_unlock (&cond->__data.__futex, 1,
> - 1, &cond->__data.__lock,
> - pshared), 0))
> - return 0;
> +#if (defined lll_futex_cmp_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + int pi_flag = PTHREAD_MUTEX_PRIO_INHERIT_NP | PTHREAD_MUTEX_ROBUST_NP;
> + pthread_mutex_t *mut = cond->__data.__mutex;
>
> + /* Do not use requeue for pshared condvars. */
> + if (mut != (void *) ~0l)
> + pi_flag &= mut->__data.__kind;
> +
> + if (__builtin_expect (pi_flag == PTHREAD_MUTEX_PRIO_INHERIT_NP, 0)
> + /* This can only really fail with a ENOSYS, since nobody can modify
> + futex while we have the cond_lock. */
> + && lll_futex_cmp_requeue_pi (&cond->__data.__futex, 1, 0,
> + &mut->__data.__lock,
> + cond->__data.__futex, pshared) == 0)
> + {
> + lll_unlock (cond->__data.__lock, pshared);
> + return 0;
> + }
> + else
> +#endif
> + /* Wake one. */
> + if (! __builtin_expect (lll_futex_wake_unlock (&cond->__data.__futex,
> + 1, 1,
> + &cond->__data.__lock,
> + pshared), 0))
> + return 0;
> +
> + /* Fallback if neither of them work. */
> lll_futex_wake (&cond->__data.__futex, 1, pshared);
> }
>
> diff --git a/nptl/pthread_cond_timedwait.c b/nptl/pthread_cond_timedwait.c
> index 0f52bd8..0a2d092 100644
> --- a/nptl/pthread_cond_timedwait.c
> +++ b/nptl/pthread_cond_timedwait.c
> @@ -64,6 +64,11 @@ __pthread_cond_timedwait (cond, mutex, abstime)
> int pshared = (cond->__data.__mutex == (void *) ~0l)
> ? LLL_SHARED : LLL_PRIVATE;
>
> +#if (defined lll_futex_timed_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + int pi_flag = 0;
> +#endif
> +
> /* Make sure we are alone. */
> lll_lock (cond->__data.__lock, pshared);
>
> @@ -155,17 +160,46 @@ __pthread_cond_timedwait (cond, mutex, abstime)
> /* Enable asynchronous cancellation. Required by the standard. */
> cbuffer.oldtype = __pthread_enable_asynccancel ();
>
> +/* REQUEUE_PI was implemented after FUTEX_CLOCK_REALTIME, so it is sufficient
> + to check just the former. */
> +#if (defined lll_futex_timed_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + /* If pi_flag remained 1 then it means that we had the lock and the mutex
> + but a spurious waker raced ahead of us. Give back the mutex before
> + going into wait again. */
> + if (pi_flag)
> + {
> + __pthread_mutex_cond_lock_adjust (mutex);
> + __pthread_mutex_unlock_usercnt (mutex, 0);
> + }
> + pi_flag = USE_REQUEUE_PI (mutex);
> +
> + if (pi_flag)
> + {
> + unsigned int clockbit = (cond->__data.__nwaiters & 1
> + ? 0 : FUTEX_CLOCK_REALTIME);
> + err = lll_futex_timed_wait_requeue_pi (&cond->__data.__futex,
> + futex_val, abstime, clockbit,
> + &mutex->__data.__lock,
> + pshared);
> + pi_flag = (err == 0);
> + }
> + else
> +#endif
> +
> + {
> #if (!defined __ASSUME_FUTEX_CLOCK_REALTIME \
> || !defined lll_futex_timed_wait_bitset)
> - /* Wait until woken by signal or broadcast. */
> - err = lll_futex_timed_wait (&cond->__data.__futex,
> - futex_val, &rt, pshared);
> + /* Wait until woken by signal or broadcast. */
> + err = lll_futex_timed_wait (&cond->__data.__futex,
> + futex_val, &rt, pshared);
> #else
> - unsigned int clockbit = (cond->__data.__nwaiters & 1
> - ? 0 : FUTEX_CLOCK_REALTIME);
> - err = lll_futex_timed_wait_bitset (&cond->__data.__futex, futex_val,
> - abstime, clockbit, pshared);
> + unsigned int clockbit = (cond->__data.__nwaiters & 1
> + ? 0 : FUTEX_CLOCK_REALTIME);
> + err = lll_futex_timed_wait_bitset (&cond->__data.__futex, futex_val,
> + abstime, clockbit, pshared);
> #endif
> + }
>
> /* Disable asynchronous cancellation. */
> __pthread_disable_asynccancel (cbuffer.oldtype);
> @@ -217,7 +251,16 @@ __pthread_cond_timedwait (cond, mutex, abstime)
> __pthread_cleanup_pop (&buffer, 0);
>
> /* Get the mutex before returning. */
> - err = __pthread_mutex_cond_lock (mutex);
> +#if (defined lll_futex_timed_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + if (pi_flag)
> + {
> + __pthread_mutex_cond_lock_adjust (mutex);
> + err = 0;
> + }
> + else
> +#endif
> + err = __pthread_mutex_cond_lock (mutex);
>
> return err ?: result;
> }
> diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
> index 0ae320c..01d42d7 100644
> --- a/nptl/pthread_cond_wait.c
> +++ b/nptl/pthread_cond_wait.c
> @@ -26,7 +26,6 @@
> #include <shlib-compat.h>
> #include <stap-probe.h>
>
> -
> struct _condvar_cleanup_buffer
> {
> int oldtype;
> @@ -85,8 +84,15 @@ __condvar_cleanup (void *arg)
> lll_futex_wake (&cbuffer->cond->__data.__futex, INT_MAX, pshared);
>
> /* Get the mutex before returning unless asynchronous cancellation
> - is in effect. */
> - __pthread_mutex_cond_lock (cbuffer->mutex);
> + is in effect. We don't try to get the mutex if we already own it. */
> + if (!(USE_REQUEUE_PI (cbuffer->mutex))
> + || ((cbuffer->mutex->__data.__lock & FUTEX_TID_MASK)
> + != THREAD_GETMEM (THREAD_SELF, tid)))
> + {
> + __pthread_mutex_cond_lock (cbuffer->mutex);
> + }
> + else
> + __pthread_mutex_cond_lock_adjust (cbuffer->mutex);
> }
>
>
> @@ -101,6 +107,11 @@ __pthread_cond_wait (cond, mutex)
> int pshared = (cond->__data.__mutex == (void *) ~0l)
> ? LLL_SHARED : LLL_PRIVATE;
>
> +#if (defined lll_futex_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + int pi_flag = 0;
> +#endif
> +
> LIBC_PROBE (cond_wait, 2, cond, mutex);
>
> /* Make sure we are alone. */
> @@ -144,15 +155,36 @@ __pthread_cond_wait (cond, mutex)
> do
> {
> unsigned int futex_val = cond->__data.__futex;
> -
> /* Prepare to wait. Release the condvar futex. */
> lll_unlock (cond->__data.__lock, pshared);
>
> /* Enable asynchronous cancellation. Required by the standard. */
> cbuffer.oldtype = __pthread_enable_asynccancel ();
>
> - /* Wait until woken by signal or broadcast. */
> - lll_futex_wait (&cond->__data.__futex, futex_val, pshared);
> +#if (defined lll_futex_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + /* If pi_flag remained 1 then it means that we had the lock and the mutex
> + but a spurious waker raced ahead of us. Give back the mutex before
> + going into wait again. */
> + if (pi_flag)
> + {
> + __pthread_mutex_cond_lock_adjust (mutex);
> + __pthread_mutex_unlock_usercnt (mutex, 0);
> + }
> + pi_flag = USE_REQUEUE_PI (mutex);
> +
> + if (pi_flag)
> + {
> + err = lll_futex_wait_requeue_pi (&cond->__data.__futex,
> + futex_val, &mutex->__data.__lock,
> + pshared);
> +
> + pi_flag = (err == 0);
> + }
> + else
> +#endif
> + /* Wait until woken by signal or broadcast. */
> + lll_futex_wait (&cond->__data.__futex, futex_val, pshared);
>
> /* Disable asynchronous cancellation. */
> __pthread_disable_asynccancel (cbuffer.oldtype);
> @@ -189,8 +221,17 @@ __pthread_cond_wait (cond, mutex)
> /* The cancellation handling is back to normal, remove the handler. */
> __pthread_cleanup_pop (&buffer, 0);
>
> - /* Get the mutex before returning. */
> - return __pthread_mutex_cond_lock (mutex);
> + /* Get the mutex before returning. Not needed for PI. */
> +#if (defined lll_futex_wait_requeue_pi \
> + && defined __ASSUME_REQUEUE_PI)
> + if (pi_flag)
> + {
> + __pthread_mutex_cond_lock_adjust (mutex);
> + return 0;
> + }
> + else
> +#endif
> + return __pthread_mutex_cond_lock (mutex);
> }
>
> versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
> diff --git a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
> index b4b1fd4..f33f703 100644
> --- a/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
> +++ b/nptl/sysdeps/unix/sysv/linux/powerpc/lowlevellock.h
> @@ -39,6 +39,8 @@
> #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
>
> @@ -149,6 +151,34 @@
> 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); \
> + long int __ret; \
> + int __op = FUTEX_WAIT_REQUEUE_PI | clockbit; \
> + \
> + __ret = INTERNAL_SYSCALL (futex, __err, 5, (futexp), \
> + __lll_private_flag (__op, private), \
> + (val), (timespec), mutex); \
> + INTERNAL_SYSCALL_ERROR_P (__ret, __err) ? -__ret : __ret; \
> + })
> +
> +#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); \
> + })
> +
>
> #ifdef UP
> # define __lll_acq_instr ""
> diff --git a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
> index a0163d6..9f7391b 100644
> --- a/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
> +++ b/nptl/sysdeps/unix/sysv/linux/s390/lowlevellock.h
> @@ -37,6 +37,8 @@
> #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
>
> @@ -141,6 +143,31 @@
> 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) \
> + ({ \
> + 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_compare_and_swap(futex, oldval, newval, operation) \
> do { \
> __typeof (futex) __futex = (futex); \
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index 21eef43..8fdff7e 100644
> --- a/sysdeps/unix/sysv/linux/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/kernel-features.h
> @@ -187,6 +187,11 @@
> # define __ASSUME_PWRITEV 1
> #endif
>
> +/* Support for FUTEX_*_REQUEUE_PI was added in 2.6.31. */
> +#if __LINUX_KERNEL_VERSION >= 0x02061f
> +# define __ASSUME_REQUEUE_PI 1
> +#endif
> +
> /* Support for F_GETOWN_EX was introduced in 2.6.32. */
> #if __LINUX_KERNEL_VERSION >= 0x020620
> # define __ASSUME_F_GETOWN_EX 1