[PATCH] New condvar implementation that provides stronger ordering guarantees.

Tue Jun 14 20:54:00 GMT 2016

I've now tested this patch successfully using our existing tests on ppc,
ppc64, ppc64le, s390x, and aarch64.  I couldn't test on s390 due to
https://www.sourceware.org/ml/libc-alpha/2016-06/msg00545.html.

The attached patch is a minor revision that just fixes some formatting
and adds an include (revealing by testing on s390x).

I think this patch is ready for wider testing, for example in Rawhide or
perhaps even by committing it on trunk.
Some more testing of the overflow case in the atomic monotonic 64b
counters we use on archs that just have 32b atomic ops (see
pthread_cond_common.c) would be good; I plan to do this later, as it
requires creating new tests and doesn't affect the core condvar
algorithm really.

I'd still appreciate further reviews, even if this is just about whether
the algorithm is accessible enough and the comments do a good-enough job
of explaining it.

Thoughts?

On Thu, 2016-05-26 at 15:21 +0200, Torvald Riegel wrote:
> This replaces the current condvar with a new algorithm that's different
> that the one I proposed last year.  I wasn't able to fix the
> futex-spurious-wake-up-related issue in last year's algorithm without
> the risk of decreasing performance significantly.
> 
> I would appreciate testing on weak memory model architectures such as
> ARM and POWER.  I have tested on x86 and x86_64.
> I would of course also appreciate more sets of eyes on the algorithm.
> I'm not aware of any issues, but reviews never hurt; even if you just
> would like to see more detailed comments, please let me know. 
> 
> Once we have that, testing in Rawhide should be the next step.  We still
> have a month until the feature freeze deadline, so I'd like to get this
> done this cycle if possible.
> 
> pre-v9 sparc and hppa are currently broken by this patch.
> For sparc, I'd like to get feedback by sparc maintainers regarding how
> they would like to deal with the lack of proper atomics support in
> pre-v9 sparc; it's a recurring theme, so maybe we should tackle that in
> a more foundational way than just creating custom pre-v9 sparc for all
> synchronization algorithms.
> Regarding hppa, I might leave it to the hppa maintainer to use the new
> condvar (it has to support old LinuxThreads initializers).  Or I'll do
> it later if he tells me I should do it myself ;)
> 
> This patch doesn't do anything in terms of support for PI and real time.
> I believe we'd need a significantly adapted implementation (and perhaps
> algorithm) to support it; for example, we're short on space in
> pthread_cond_t, so even sticking in a PI mutex as replacement for the
> currently used condvar-internal lock would require jumping through
> hoops.
> 
> 
> 2016-05-26  Torvald Riegel  <triegel@redhat.com>
> 
> 	[BZ #13165]
> 	* nptl/pthread_cond_broadcast.c (__pthread_cond_broadcast): Rewrite to
> 	use new algorithm.
> 	* nptl/pthread_cond_destroy.c (__pthread_cond_destroy): Likewise.
> 	* nptl/pthread_cond_init.c (__pthread_cond_init): Likewise.
> 	* nptl/pthread_cond_signal.c (__pthread_cond_signal): Likewise.
> 	* nptl/pthread_cond_wait.c (__pthread_cond_wait): Likewise.
> 	(__pthread_cond_timedwait): Move here from pthread_cond_timedwait.c.
> 	(__condvar_confirm_wakeup, __condvar_cancel_waiting,
> 	__condvar_cleanup_waiting, __condvar_dec_grefs,
> 	__pthread_cond_wait_common): New.
> 	(__condvar_cleanup): Remove.
> 	* npt/pthread_condattr_getclock.c (pthread_condattr_getclock): Adapt.
> 	* npt/pthread_condattr_setclock.c (pthread_condattr_setclock):
> 	Likewise.
> 	* npt/pthread_condattr_getpshared.c (pthread_condattr_getpshared):
> 	Likewise.
> 	* npt/pthread_condattr_init.c (pthread_condattr_init): Likewise.
> 	* nptl/tst-cond1.c: Add comment.
> 	* nptl/tst-cond20.c (do_test): Adapt.
> 	* nptl/tst-cond22.c (do_test): Likewise.
> 	* sysdeps/aarch64/nptl/bits/pthreadtypes.h (pthread_cond_t): Adapt
> 	structure.
> 	* sysdeps/arm/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/ia64/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/m68k/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/microblaze/nptl/bits/pthreadtypes.h (pthread_cond_t):
> 	Likewise.
> 	* sysdeps/mips/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/nios2/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/s390/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/sh/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/tile/nptl/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/unix/sysv/linux/alpha/bits/pthreadtypes.h (pthread_cond_t):
> 	Likewise.
> 	* sysdeps/unix/sysv/linux/powerpc/bits/pthreadtypes.h (pthread_cond_t):
> 	Likewise.
> 	* sysdeps/x86/bits/pthreadtypes.h (pthread_cond_t): Likewise.
> 	* sysdeps/nptl/internaltypes.h (COND_NWAITERS_SHIFT): Remove.
> 	(COND_CLOCK_BITS): Adapt.
> 	* sysdeps/nptl/pthread.h (PTHREAD_COND_INITIALIZER): Adapt.
> 	* sysdeps/unix/sysv/linux/hppa/internaltypes.h (cond_compat_clear,
> 	cond_compat_check_and_clear): Adapt.
> 	* sysdeps/unix/sysv/linux/hppa/pthread_cond_timedwait.c: Remove file ...
> 	* sysdeps/unix/sysv/linux/hppa/pthread_cond_wait.c
> 	(__pthread_cond_timedwait): ... and move here.
> 	* nptl/DESIGN-condvar.txt: Remove file.
> 	* nptl/lowlevelcond.sym: Likewise.
> 	* nptl/pthread_cond_timedwait.c: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_broadcast.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_signal.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_timedwait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i486/pthread_cond_wait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i586/pthread_cond_broadcast.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i586/pthread_cond_signal.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i586/pthread_cond_timedwait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i586/pthread_cond_wait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i686/pthread_cond_broadcast.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i686/pthread_cond_signal.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i686/pthread_cond_timedwait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/i386/i686/pthread_cond_wait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_broadcast.S: Likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_signal.S: Likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S: Likewise.
> 	* sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Likewise.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: condvar.patch
Type: text/x-patch
Size: 209043 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20160614/9dc8466c/attachment.bin>