This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Ping[3]: [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: Tue, 12 Feb 2013 14:44:14 +0530
- Subject: Ping[3]: [PATCH][s390][ppc] Add REQUEUE_PI support
- References: <20130108132702.GD27464@spoyarek.pnq.redhat.com><20130204114626.GI29187@spoyarek.pnq.redhat.com>
Ping!
On Mon, Feb 04, 2013 at 05:16:26PM +0530, Siddhesh Poyarekar wrote:
> 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