This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC PATCH 1/2] Add generic arch support for Requeue PI
- From: Dinakar Guniguntala <dino at in dot ibm dot com>
- To: libc-alpha at sources dot redhat dot com
- Cc: Darren Hart <dvhltc at us dot ibm dot com>, Thomas Gleixner <tglx at linutronix dot de>, Ulrich Drepper <drepper at redhat dot com>, Ingo Molnar <mingo at elte dot hu>, Jakub Jelinek <jakub at redhat dot com>, Steven Rostedt <rostedt at goodmis dot org>, Sripathi Kodi <sripathik at in dot ibm dot com>
- Date: Mon, 18 May 2009 21:52:41 +0530
- Subject: Re: [RFC PATCH 1/2] Add generic arch support for Requeue PI
- References: <20090505195003.GA23727@in.ibm.com>
- Reply-to: dino at in dot ibm dot com
Hi Ulrich/Jakub,
Just a gentle reminder, Are you planning on reviewing these
patches ? Would appreciate if you can take a look a them
and comment on the general requeue-PI design and also the
patches below, Thanks
-Dinakar
On Wed, May 06, 2009 at 01:20:03AM +0530, Dinakar Guniguntala wrote:
>
> Hello,
>
> Please refer to the following mail from Darren Hart on lkml that
> adds support for requeue PI implementation in the kernel
> http://marc.info/?l=linux-kernel&m=123879118200773&w=2
> and some documentation regarding the same
> http://marc.info/?l=linux-kernel&m=123922906632577&w=2
>
> The following 2 patches adds glibc support for requeue'ing
> from condvars to PI mutexes. This currently has support
> only for x86_64 architecture and has been tested pretty
> heavily with java and a make subdirs=nptl check
>
> The kernel feature is currently only available in the RT tree,
> and I have used the 2.6.29.2-rt11 kernel to test the feature
>
> http://www.kernel.org/pub/linux/kernel/v2.6/linux-2.6.29.2.tar.bz2
> http://www.kernel.org/pub/linux/kernel/projects/rt/patch-2.6.29.2-rt11.bz2
>
> I would really appreciate any feedback on this, Thanks!
>
> -Dinakar
>
>
> 2009-05-05 Dinakar Guniguntala <dino@in.ibm.com>
>
> * pthread_cond_broadcast.c: Add support for using
> FUTEX_CMP_REQUEUE_PI for PI mutexes.
> * pthread_cond_signal.c: Likewise.
> * pthread_cond_timedwait.c: Add support for using
> FUTEX_WAIT_REQUEUE_PI for PI mutexes.
> * pthread_cond_wait.c: Likewise.
>
>
> diff -Nurp libc-20090427/nptl/pthread_cond_broadcast.c libc-20090427-1/nptl/pthread_cond_broadcast.c
> --- libc-20090427/nptl/pthread_cond_broadcast.c 2007-08-11 14:44:39.000000000 -0400
> +++ libc-20090427-1/nptl/pthread_cond_broadcast.c 2009-05-05 15:15:38.000000000 -0400
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2003, 2004, 2006, 2007 Free Software Foundation, Inc.
> +/* Copyright (C) 2003, 2004, 2006, 2007, 2009 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
> Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
>
> @@ -58,19 +58,28 @@ __pthread_cond_broadcast (cond)
> /* 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). */
> + /* We don't requeue for pshared condvars */
> if (__builtin_expect (mut->__data.__kind
> - & (PTHREAD_MUTEX_PRIO_INHERIT_NP
> - | PTHREAD_MUTEX_PSHARED_BIT), 0))
> + & PTHREAD_MUTEX_PSHARED_BIT, 0))
> 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 (__builtin_expect (mut->__data.__kind
> + & PTHREAD_MUTEX_PRIO_INHERIT_NP, 0))
> + {
> + /* lll_futex_requeue_pi returns 0 for success and non-zero
> + for errors. */
> + if (__builtin_expect (lll_futex_requeue_pi (&cond->__data.__futex,
> + 1, INT_MAX,
> + &mut->__data.__lock,
> + futex_val, LLL_PRIVATE),
> + 0))
> + /* The requeue_pi functionality is not available. */
> + goto wake_all;
> + }
> + /* lll_futex_requeue returns 0 for success and non-zero for errors. */
> + else if (__builtin_expect (lll_futex_requeue (&cond->__data.__futex, 1,
> + INT_MAX, &mut->__data.__lock,
> + futex_val, LLL_PRIVATE), 0))
> {
> /* The requeue functionality is not available. */
> wake_all:
> diff -Nurp libc-20090427/nptl/pthread_cond_signal.c libc-20090427-1/nptl/pthread_cond_signal.c
> --- libc-20090427/nptl/pthread_cond_signal.c 2007-08-11 14:46:16.000000000 -0400
> +++ libc-20090427-1/nptl/pthread_cond_signal.c 2009-05-05 15:16:09.000000000 -0400
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
> +/* Copyright (C) 2003, 2004, 2007, 2009 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
> Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
>
> @@ -32,8 +32,8 @@ int
> __pthread_cond_signal (cond)
> pthread_cond_t *cond;
> {
> - int pshared = (cond->__data.__mutex == (void *) ~0l)
> - ? LLL_SHARED : LLL_PRIVATE;
> + pthread_mutex_t *mutex = (pthread_mutex_t *) cond->__data.__mutex;
> + int pshared = (mutex == (void *) ~0l) ? LLL_SHARED : LLL_PRIVATE;
>
> /* Make sure we are alone. */
> lll_lock (cond->__data.__lock, pshared);
> @@ -44,16 +44,29 @@ __pthread_cond_signal (cond)
> /* Yes. Mark one of them as woken. */
> ++cond->__data.__wakeup_seq;
> ++cond->__data.__futex;
> + int futex_val = cond->__data.__futex;
>
> + /* XXX Do we need to ensure that we are not a pshared mutex? */
> + if (mutex->__data.__kind & PTHREAD_MUTEX_PRIO_INHERIT_NP)
> + {
> + /* lll_futex_requeue_pi returns 0 for success and non-zero
> + for errors. */
> + if (! __builtin_expect (lll_futex_requeue_pi (&cond->__data.__futex,
> + 1, 0, &mutex->__data.__lock, futex_val,
> + pshared), 0))
> + goto done;
> + }
> /* Wake one. */
> - if (! __builtin_expect (lll_futex_wake_unlock (&cond->__data.__futex, 1,
> - 1, &cond->__data.__lock,
> - pshared), 0))
> + else if (! __builtin_expect (lll_futex_wake_unlock (&cond->__data.__futex,
> + 1, 1,
> + &cond->__data.__lock,
> + pshared), 0))
> return 0;
>
> lll_futex_wake (&cond->__data.__futex, 1, pshared);
> }
>
> +done:
> /* We are done. */
> lll_unlock (cond->__data.__lock, pshared);
>
> diff -Nurp libc-20090427/nptl/pthread_cond_timedwait.c libc-20090427-1/nptl/pthread_cond_timedwait.c
> --- libc-20090427/nptl/pthread_cond_timedwait.c 2007-08-13 14:32:59.000000000 -0400
> +++ libc-20090427-1/nptl/pthread_cond_timedwait.c 2009-05-05 15:16:21.000000000 -0400
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2003, 2004, 2007 Free Software Foundation, Inc.
> +/* Copyright (C) 2003, 2004, 2007, 2009 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
> Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
>
> @@ -49,6 +49,7 @@ __pthread_cond_timedwait (cond, mutex, a
> struct _pthread_cleanup_buffer buffer;
> struct _condvar_cleanup_buffer cbuffer;
> int result = 0;
> + int pi_requeued;
>
> /* Catch invalid parameters. */
> if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
> @@ -97,6 +98,7 @@ __pthread_cond_timedwait (cond, mutex, a
>
> while (1)
> {
> + pi_requeued = 0;
> struct timespec rt;
> {
> #ifdef __NR_clock_gettime
> @@ -155,10 +157,26 @@ __pthread_cond_timedwait (cond, mutex, a
> /* Enable asynchronous cancellation. Required by the standard. */
> cbuffer.oldtype = __pthread_enable_asynccancel ();
>
> + if (mutex->__data.__kind & PTHREAD_MUTEX_PRIO_INHERIT_NP)
> + {
> + /* Try requeueing to the PI mutex, if no support in the kernel
> + try the non-requeue syscall. */
> + if (! __builtin_expect ((err = lll_futex_timed_wait_requeue_pi
> + (&cond->__data.__futex,
> + futex_val, &rt,
> + &mutex->__data.__lock,
> + pshared)), 0))
> + {
> + pi_requeued = 1;
> + goto woken;
> + }
> + }
> +
> /* Wait until woken by signal or broadcast. */
> err = lll_futex_timed_wait (&cond->__data.__futex,
> futex_val, &rt, pshared);
>
> +woken:
> /* Disable asynchronous cancellation. */
> __pthread_disable_asynccancel (cbuffer.oldtype);
>
> @@ -208,8 +226,10 @@ __pthread_cond_timedwait (cond, mutex, a
> /* The cancellation handling is back to normal, remove the handler. */
> __pthread_cleanup_pop (&buffer, 0);
>
> - /* Get the mutex before returning. */
> - err = __pthread_mutex_cond_lock (mutex);
> + /* Get the mutex before returning. If the requeue_pi call above was successful,
> + the lock is already held in the kernel, so just return to the application. */
> + if (!pi_requeued)
> + err = __pthread_mutex_cond_lock (mutex);
>
> return err ?: result;
> }
> diff -Nurp libc-20090427/nptl/pthread_cond_wait.c libc-20090427-1/nptl/pthread_cond_wait.c
> --- libc-20090427/nptl/pthread_cond_wait.c 2007-08-11 14:44:27.000000000 -0400
> +++ libc-20090427-1/nptl/pthread_cond_wait.c 2009-05-05 15:16:30.000000000 -0400
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 2003, 2004, 2006, 2007 Free Software Foundation, Inc.
> +/* Copyright (C) 2003, 2004, 2006, 2007, 2009 Free Software Foundation, Inc.
> This file is part of the GNU C Library.
> Contributed by Martin Schwidefsky <schwidefsky@de.ibm.com>, 2003.
>
> @@ -100,6 +100,7 @@ __pthread_cond_wait (cond, mutex)
> int err;
> int pshared = (cond->__data.__mutex == (void *) ~0l)
> ? LLL_SHARED : LLL_PRIVATE;
> + int pi_requeued;
>
> /* Make sure we are along. */
> lll_lock (cond->__data.__lock, pshared);
> @@ -142,6 +143,7 @@ __pthread_cond_wait (cond, mutex)
> do
> {
> unsigned int futex_val = cond->__data.__futex;
> + pi_requeued = 0;
>
> /* Prepare to wait. Release the condvar futex. */
> lll_unlock (cond->__data.__lock, pshared);
> @@ -149,9 +151,25 @@ __pthread_cond_wait (cond, mutex)
> /* Enable asynchronous cancellation. Required by the standard. */
> cbuffer.oldtype = __pthread_enable_asynccancel ();
>
> - /* Wait until woken by signal or broadcast. */
> + if (mutex->__data.__kind & PTHREAD_MUTEX_PRIO_INHERIT_NP)
> + {
> + /* Try requeueing to the PI mutex, if no support in the kernel
> + try the non-requeue syscall. */
> + if (! __builtin_expect ((lll_futex_wait_requeue_pi
> + (&cond->__data.__futex,
> + futex_val,
> + &mutex->__data.__lock,
> + pshared)), 0))
> + {
> + pi_requeued = 1;
> + goto woken;
> + }
> + }
> +
> + /* Wait until woken by signal or broadcast. */
> lll_futex_wait (&cond->__data.__futex, futex_val, pshared);
>
> +woken:
> /* Disable asynchronous cancellation. */
> __pthread_disable_asynccancel (cbuffer.oldtype);
>
> @@ -187,7 +205,11 @@ __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. */
> + /* Get the mutex before returning. If the requeue_pi call above was successful,
> + the lock is already held in the kernel, so just return 0 to application. */
> + if (pi_requeued)
> + return 0;
> +
> return __pthread_mutex_cond_lock (mutex);
> }
>
> diff -Nurp libc-20090427/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h libc-20090427-1/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
> --- libc-20090427/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h 2009-01-02 22:44:45.000000000 -0500
> +++ libc-20090427-1/nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h 2009-05-04 05:41:38.000000000 -0400
> @@ -54,6 +54,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
>
> @@ -221,6 +223,27 @@ LLL_STUB_UNWIND_INFO_END
> })
>
>
> +#define lll_futex_wait_requeue_pi(futex, val, mutex, private) \
> + lll_futex_timed_wait_requeue_pi(futex, val, NULL, mutex, private)
> +
> +
> +#define lll_futex_timed_wait_requeue_pi(futex, val, timeout, mutex, private) \
> + ({ \
> + register const struct timespec *__to __asm ("r10") = timeout; \
> + register void *__mutex __asm ("r8") = mutex; \
> + int __status; \
> + register __typeof (val) _val __asm ("edx") = (val); \
> + __asm __volatile ("syscall" \
> + : "=a" (__status) \
> + : "0" (SYS_futex), "D" (futex), \
> + "S" (__lll_private_flag (FUTEX_WAIT_REQUEUE_PI, private)), \
> + "d" (_val), "r" (__to), \
> + "r" (__mutex) \
> + : "memory", "cc", "r11", "cx"); \
> + __status; \
> + })
> +
> +
> #define lll_futex_wake(futex, nr, private) \
> do { \
> int __ignore; \
> @@ -552,6 +575,21 @@ LLL_STUB_UNWIND_INFO_END
> : "cx", "r11", "cc", "memory"); \
> __res < 0; })
>
> +/* Returns non-zero if error happened, zero if success. */
> +#define lll_futex_requeue_pi(ftx, nr_wake, nr_move, mutex, val, private) \
> + ({ int __res; \
> + register int __nr_move __asm ("r10") = nr_move; \
> + register void *__mutex __asm ("r8") = mutex; \
> + register int __val __asm ("r9") = val; \
> + __asm __volatile ("syscall" \
> + : "=a" (__res) \
> + : "0" (__NR_futex), "D" ((void *) ftx), \
> + "S" (__lll_private_flag (FUTEX_CMP_REQUEUE_PI, \
> + private)), "d" (nr_wake), \
> + "r" (__nr_move), "r" (__mutex), "r" (__val) \
> + : "cx", "r11", "cc", "memory"); \
> + __res < 0; })
> +
> #define lll_islocked(futex) \
> (futex != LLL_LOCK_INITIALIZER)