Module Name: src Committed By: ad Date: Sat May 16 22:53:37 UTC 2020
Modified Files: src/lib/libpthread: pthread.c pthread_barrier.c pthread_cond.c pthread_int.h pthread_mutex.c pthread_rwlock.c Log Message: - Try to eliminate a hang in "parked" I've been seeing while stress testing. Centralise wakeup of deferred waiters in pthread__clear_waiters() and use throughout libpthread. Make fewer assumptions. Be more conservative in pthread_mutex when dealing with pending waiters. - Remove the "hint" argument everywhere since the kernel doesn't use it any more. To generate a diff of this commit: cvs rdiff -u -r1.169 -r1.170 src/lib/libpthread/pthread.c cvs rdiff -u -r1.21 -r1.22 src/lib/libpthread/pthread_barrier.c cvs rdiff -u -r1.68 -r1.69 src/lib/libpthread/pthread_cond.c cvs rdiff -u -r1.103 -r1.104 src/lib/libpthread/pthread_int.h cvs rdiff -u -r1.76 -r1.77 src/lib/libpthread/pthread_mutex.c cvs rdiff -u -r1.39 -r1.40 src/lib/libpthread/pthread_rwlock.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/lib/libpthread/pthread.c diff -u src/lib/libpthread/pthread.c:1.169 src/lib/libpthread/pthread.c:1.170 --- src/lib/libpthread/pthread.c:1.169 Fri May 15 14:30:23 2020 +++ src/lib/libpthread/pthread.c Sat May 16 22:53:37 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread.c,v 1.169 2020/05/15 14:30:23 joerg Exp $ */ +/* $NetBSD: pthread.c,v 1.170 2020/05/16 22:53:37 ad Exp $ */ /*- * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020 @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread.c,v 1.169 2020/05/15 14:30:23 joerg Exp $"); +__RCSID("$NetBSD: pthread.c,v 1.170 2020/05/16 22:53:37 ad Exp $"); #define __EXPOSE_STACK 1 @@ -105,7 +105,7 @@ static int pthread__diagassert; int pthread__concurrency; int pthread__nspins; -int pthread__unpark_max = PTHREAD__UNPARK_MAX; +size_t pthread__unpark_max = PTHREAD__UNPARK_MAX; int pthread__dbg; /* set by libpthread_dbg if active */ /* @@ -191,9 +191,9 @@ pthread__init(void) { pthread_t first; char *p; - int i; int mib[2]; unsigned int value; + ssize_t slen; size_t len; extern int __isthreaded; @@ -223,16 +223,16 @@ pthread__init(void) /* Initialize locks first; they're needed elsewhere. */ pthread__lockprim_init(); - for (i = 0; i < NHASHLOCK; i++) { + for (int i = 0; i < NHASHLOCK; i++) { pthread_mutex_init(&hashlocks[i].mutex, NULL); } /* Fetch parameters. */ - i = (int)_lwp_unpark_all(NULL, 0, NULL); - if (i == -1) + slen = _lwp_unpark_all(NULL, 0, NULL); + if (slen < 0) err(EXIT_FAILURE, "_lwp_unpark_all"); - if (i < pthread__unpark_max) - pthread__unpark_max = i; + if ((size_t)slen < pthread__unpark_max) + pthread__unpark_max = slen; /* Basic data structure setup */ pthread_attr_init(&pthread_default_attr); @@ -604,16 +604,57 @@ pthread_resume_np(pthread_t thread) * awoken (because cancelled, for instance) make sure we have no waiters * left. */ -static void +void pthread__clear_waiters(pthread_t self) { + int rv; - if (self->pt_nwaiters != 0) { - (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, - NULL); - self->pt_nwaiters = 0; + /* Zero waiters or one waiter in error case (pthread_exit()). */ + if (self->pt_nwaiters == 0) { + if (self->pt_unpark != 0 && self->pt_willpark == 0) { + rv = (ssize_t)_lwp_unpark(self->pt_unpark, NULL); + self->pt_unpark = 0; + if (rv != 0 && errno != EALREADY && errno != EINTR && + errno != ESRCH) { + pthread__errorfunc(__FILE__, __LINE__, __func__, + "_lwp_unpark failed"); + } + } + return; + } + + /* One waiter or two waiters (the second being a deferred wakeup). */ + if (self->pt_nwaiters == 1) { + if (self->pt_unpark != 0) { + /* Fall through to multiple waiters case. */ + self->pt_waiters[1] = self->pt_unpark; + self->pt_nwaiters = 2; + self->pt_unpark = 0; + } else if (self->pt_willpark) { + /* Defer to _lwp_park(). */ + self->pt_unpark = self->pt_waiters[0]; + self->pt_nwaiters = 0; + return; + } else { + /* Wake one now. */ + rv = (ssize_t)_lwp_unpark(self->pt_waiters[0], NULL); + self->pt_nwaiters = 0; + if (rv != 0 && errno != EALREADY && errno != EINTR && + errno != ESRCH) { + pthread__errorfunc(__FILE__, __LINE__, __func__, + "_lwp_unpark failed"); + } + return; + } + } + + /* Multiple waiters. */ + rv = _lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, NULL); + self->pt_nwaiters = 0; + if (rv != 0 && errno != EINTR) { + pthread__errorfunc(__FILE__, __LINE__, __func__, + "_lwp_unpark_all failed"); } - self->pt_willpark = 0; } void @@ -630,6 +671,7 @@ pthread_exit(void *retval) self = pthread__self(); /* Disable cancellability. */ + self->pt_willpark = 0; pthread_mutex_lock(&self->pt_lock); self->pt_flags |= PT_FLAG_CS_DISABLED; self->pt_cancel = 0; @@ -660,10 +702,12 @@ pthread_exit(void *retval) if (self->pt_flags & PT_FLAG_DETACHED) { /* pthread__reap() will drop the lock. */ pthread__reap(self); + pthread__assert(!self->pt_willpark); pthread__clear_waiters(self); _lwp_exit(); } else { self->pt_state = PT_STATE_ZOMBIE; + pthread__assert(!self->pt_willpark); pthread_mutex_unlock(&self->pt_lock); pthread__clear_waiters(self); /* Note: name will be freed by the joiner. */ @@ -1125,7 +1169,7 @@ pthread__errorfunc(const char *file, int int pthread__park(pthread_t self, pthread_mutex_t *lock, pthread_queue_t *queue, const struct timespec *abstime, - int cancelpt, const void *hint) + int cancelpt) { int rv, error; void *obj; @@ -1170,7 +1214,7 @@ pthread__park(pthread_t self, pthread_mu * have _lwp_park() restart it before blocking. */ error = _lwp_park(CLOCK_REALTIME, TIMER_ABSTIME, - __UNCONST(abstime), self->pt_unpark, hint, hint); + __UNCONST(abstime), self->pt_unpark, NULL, NULL); self->pt_unpark = 0; if (error != 0) { switch (rv = errno) { @@ -1215,22 +1259,14 @@ pthread__unpark(pthread_queue_t *queue, pthread_mutex_t *interlock) { pthread_t target; - u_int max; - size_t nwaiters; - max = pthread__unpark_max; - nwaiters = self->pt_nwaiters; target = PTQ_FIRST(queue); - if (nwaiters == max) { - /* Overflow. */ - (void)_lwp_unpark_all(self->pt_waiters, nwaiters, - __UNVOLATILE(&interlock->ptm_waiters)); - nwaiters = 0; + if (self->pt_nwaiters == pthread__unpark_max) { + pthread__clear_waiters(self); } target->pt_sleepobj = NULL; - self->pt_waiters[nwaiters++] = target->pt_lid; + self->pt_waiters[self->pt_nwaiters++] = target->pt_lid; PTQ_REMOVE(queue, target, pt_sleep); - self->pt_nwaiters = nwaiters; pthread__mutex_deferwake(self, interlock); } @@ -1238,23 +1274,16 @@ void pthread__unpark_all(pthread_queue_t *queue, pthread_t self, pthread_mutex_t *interlock) { + const size_t max = pthread__unpark_max; pthread_t target; - u_int max; - size_t nwaiters; - max = pthread__unpark_max; - nwaiters = self->pt_nwaiters; PTQ_FOREACH(target, queue, pt_sleep) { - if (nwaiters == max) { - /* Overflow. */ - (void)_lwp_unpark_all(self->pt_waiters, nwaiters, - __UNVOLATILE(&interlock->ptm_waiters)); - nwaiters = 0; + if (self->pt_nwaiters == max) { + pthread__clear_waiters(self); } target->pt_sleepobj = NULL; - self->pt_waiters[nwaiters++] = target->pt_lid; + self->pt_waiters[self->pt_nwaiters++] = target->pt_lid; } - self->pt_nwaiters = nwaiters; PTQ_INIT(queue); pthread__mutex_deferwake(self, interlock); } Index: src/lib/libpthread/pthread_barrier.c diff -u src/lib/libpthread/pthread_barrier.c:1.21 src/lib/libpthread/pthread_barrier.c:1.22 --- src/lib/libpthread/pthread_barrier.c:1.21 Wed Jan 29 14:41:57 2020 +++ src/lib/libpthread/pthread_barrier.c Sat May 16 22:53:37 2020 @@ -1,7 +1,7 @@ -/* $NetBSD: pthread_barrier.c,v 1.21 2020/01/29 14:41:57 kamil Exp $ */ +/* $NetBSD: pthread_barrier.c,v 1.22 2020/05/16 22:53:37 ad Exp $ */ /*- - * Copyright (c) 2001, 2003, 2006, 2007, 2009 The NetBSD Foundation, Inc. + * Copyright (c) 2001, 2003, 2006, 2007, 2009, 2020 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -30,7 +30,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_barrier.c,v 1.21 2020/01/29 14:41:57 kamil Exp $"); +__RCSID("$NetBSD: pthread_barrier.c,v 1.22 2020/05/16 22:53:37 ad Exp $"); #include <errno.h> @@ -106,7 +106,7 @@ pthread_barrier_wait(pthread_barrier_t * PTQ_INSERT_TAIL(&barrier->ptb_waiters, self, pt_sleep); self->pt_sleepobj = &barrier->ptb_waiters; (void)pthread__park(self, interlock, &barrier->ptb_waiters, - NULL, 0, __UNVOLATILE(&interlock->ptm_waiters)); + NULL, 0); if (__predict_true(gen != barrier->ptb_generation)) { break; } Index: src/lib/libpthread/pthread_cond.c diff -u src/lib/libpthread/pthread_cond.c:1.68 src/lib/libpthread/pthread_cond.c:1.69 --- src/lib/libpthread/pthread_cond.c:1.68 Tue Apr 14 23:35:07 2020 +++ src/lib/libpthread/pthread_cond.c Sat May 16 22:53:37 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $ */ +/* $NetBSD: pthread_cond.c,v 1.69 2020/05/16 22:53:37 ad Exp $ */ /*- * Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc. @@ -46,7 +46,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_cond.c,v 1.68 2020/04/14 23:35:07 joerg Exp $"); +__RCSID("$NetBSD: pthread_cond.c,v 1.69 2020/05/16 22:53:37 ad Exp $"); #include <stdlib.h> #include <errno.h> @@ -161,9 +161,7 @@ pthread_cond_timedwait(pthread_cond_t *c self->pt_willpark = 0; do { retval = _lwp_park(clkid, TIMER_ABSTIME, - __UNCONST(abstime), self->pt_unpark, - __UNVOLATILE(&mutex->ptm_waiters), - __UNVOLATILE(&mutex->ptm_waiters)); + __UNCONST(abstime), self->pt_unpark, NULL, NULL); self->pt_unpark = 0; } while (retval == -1 && errno == ESRCH); pthread_mutex_lock(mutex); @@ -254,9 +252,7 @@ pthread__cond_wake_one(pthread_cond_t *c * caller (this thread) releases the mutex. */ if (__predict_false(self->pt_nwaiters == (size_t)pthread__unpark_max)) { - (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, - __UNVOLATILE(&mutex->ptm_waiters)); - self->pt_nwaiters = 0; + pthread__clear_waiters(self); } self->pt_waiters[self->pt_nwaiters++] = lid; pthread__mutex_deferwake(self, mutex); @@ -284,7 +280,6 @@ pthread__cond_wake_all(pthread_cond_t *c pthread_t self, signaled; pthread_mutex_t *mutex; u_int max; - size_t nwaiters; /* * Try to defer waking threads (see pthread_cond_signal()). @@ -294,19 +289,14 @@ pthread__cond_wake_all(pthread_cond_t *c pthread__spinlock(self, &cond->ptc_lock); max = pthread__unpark_max; mutex = cond->ptc_mutex; - nwaiters = self->pt_nwaiters; PTQ_FOREACH(signaled, &cond->ptc_waiters, pt_sleep) { - if (__predict_false(nwaiters == max)) { - /* Overflow. */ - (void)_lwp_unpark_all(self->pt_waiters, - nwaiters, __UNVOLATILE(&mutex->ptm_waiters)); - nwaiters = 0; + if (__predict_false(self->pt_nwaiters == max)) { + pthread__clear_waiters(self); } signaled->pt_sleepobj = NULL; - self->pt_waiters[nwaiters++] = signaled->pt_lid; + self->pt_waiters[self->pt_nwaiters++] = signaled->pt_lid; } PTQ_INIT(&cond->ptc_waiters); - self->pt_nwaiters = nwaiters; cond->ptc_mutex = NULL; pthread__spinunlock(self, &cond->ptc_lock); pthread__mutex_deferwake(self, mutex); Index: src/lib/libpthread/pthread_int.h diff -u src/lib/libpthread/pthread_int.h:1.103 src/lib/libpthread/pthread_int.h:1.104 --- src/lib/libpthread/pthread_int.h:1.103 Sun Feb 16 17:45:11 2020 +++ src/lib/libpthread/pthread_int.h Sat May 16 22:53:37 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_int.h,v 1.103 2020/02/16 17:45:11 kamil Exp $ */ +/* $NetBSD: pthread_int.h,v 1.104 2020/05/16 22:53:37 ad Exp $ */ /*- * Copyright (c) 2001, 2002, 2003, 2006, 2007, 2008, 2020 @@ -182,7 +182,7 @@ extern size_t pthread__pagesize; extern int pthread__nspins; extern int pthread__concurrency; extern int pthread__osrev; -extern int pthread__unpark_max; +extern size_t pthread__unpark_max; extern int pthread_keys_max; extern int __uselibcstub; @@ -200,9 +200,9 @@ void pthread__unpark_all(pthread_queue_t void pthread__unpark(pthread_queue_t *, pthread_t, pthread_mutex_t *) PTHREAD_HIDE; int pthread__park(pthread_t, pthread_mutex_t *, pthread_queue_t *, - const struct timespec *, int, const void *) - PTHREAD_HIDE; + const struct timespec *, int) PTHREAD_HIDE; pthread_mutex_t *pthread__hashlock(volatile const void *) PTHREAD_HIDE; +void pthread__clear_waiters(pthread_t) PTHREAD_HIDE; /* Internal locking primitives */ void pthread__lockprim_init(void) PTHREAD_HIDE; Index: src/lib/libpthread/pthread_mutex.c diff -u src/lib/libpthread/pthread_mutex.c:1.76 src/lib/libpthread/pthread_mutex.c:1.77 --- src/lib/libpthread/pthread_mutex.c:1.76 Sun Feb 16 17:45:11 2020 +++ src/lib/libpthread/pthread_mutex.c Sat May 16 22:53:37 2020 @@ -1,7 +1,7 @@ -/* $NetBSD: pthread_mutex.c,v 1.76 2020/02/16 17:45:11 kamil Exp $ */ +/* $NetBSD: pthread_mutex.c,v 1.77 2020/05/16 22:53:37 ad Exp $ */ /*- - * Copyright (c) 2001, 2003, 2006, 2007, 2008 The NetBSD Foundation, Inc. + * Copyright (c) 2001, 2003, 2006, 2007, 2008, 2020 The NetBSD Foundation, Inc. * All rights reserved. * * This code is derived from software contributed to The NetBSD Foundation @@ -47,7 +47,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_mutex.c,v 1.76 2020/02/16 17:45:11 kamil Exp $"); +__RCSID("$NetBSD: pthread_mutex.c,v 1.77 2020/05/16 22:53:37 ad Exp $"); #include <sys/types.h> #include <sys/lwpctl.h> @@ -67,7 +67,6 @@ __RCSID("$NetBSD: pthread_mutex.c,v 1.76 #define MUTEX_WAITERS_BIT ((uintptr_t)0x01) #define MUTEX_RECURSIVE_BIT ((uintptr_t)0x02) -#define MUTEX_DEFERRED_BIT ((uintptr_t)0x04) #define MUTEX_PROTECT_BIT ((uintptr_t)0x08) #define MUTEX_THREAD ((uintptr_t)~0x0f) @@ -98,7 +97,6 @@ __RCSID("$NetBSD: pthread_mutex.c,v 1.76 static void pthread__mutex_wakeup(pthread_t, pthread_mutex_t *); static int pthread__mutex_lock_slow(pthread_mutex_t *, const struct timespec *); -static int pthread__mutex_unlock_slow(pthread_mutex_t *); static void pthread__mutex_pause(void); int _pthread_mutex_held_np(pthread_mutex_t *); @@ -267,48 +265,19 @@ pthread__mutex_spin(pthread_mutex_t *ptm return owner; } -NOINLINE static bool -pthread__mutex_setwaiters(pthread_t self, pthread_mutex_t *ptm) -{ - void *owner, *next; - - /* - * Note that the mutex can become unlocked before we set - * the waiters bit. If that happens it's not safe to sleep - * as we may never be awoken: we must remove the current - * thread from the waiters list and try again. - * - * Because we are doing this atomically, we can't remove - * one waiter: we must remove all waiters and awken them, - * then sleep in _lwp_park() until we have been awoken. - * - * Issue a memory barrier to ensure that we are reading - * the value of ptm_owner/pt_mutexwait after we have entered - * the waiters list (the CAS itself must be atomic). - */ - for (owner = ptm->ptm_owner;; owner = next) { - if (MUTEX_OWNER(owner) == 0) { - pthread__mutex_wakeup(self, ptm); - return true; - } - if (MUTEX_HAS_WAITERS(owner)) { - return false; - } - next = atomic_cas_ptr(&ptm->ptm_owner, owner, - (void *)((uintptr_t)owner | MUTEX_WAITERS_BIT)); - } -} - NOINLINE static int pthread__mutex_lock_slow(pthread_mutex_t *ptm, const struct timespec *ts) { - void *waiters, *new, *owner, *next; + void *waiters, *newval, *owner, *next; pthread_t self; int serrno; int error; owner = ptm->ptm_owner; self = pthread__self(); + serrno = errno; + + pthread__assert(!self->pt_willpark); /* Recursive or errorcheck? */ if (MUTEX_OWNER(owner) == (uintptr_t)self) { @@ -324,37 +293,31 @@ pthread__mutex_lock_slow(pthread_mutex_t /* priority protect */ if (MUTEX_PROTECT(owner) && _sched_protect(ptm->ptm_ceiling) == -1) { - return errno; + error = errno; + errno = serrno; + return error; } - serrno = errno; - for (;; owner = ptm->ptm_owner) { - /* Spin while the owner is running. */ - if (MUTEX_OWNER(owner) != (uintptr_t)self) - owner = pthread__mutex_spin(ptm, owner); + for (;;) { /* If it has become free, try to acquire it again. */ if (MUTEX_OWNER(owner) == 0) { - do { - new = (void *) - ((uintptr_t)self | (uintptr_t)owner); - next = atomic_cas_ptr(&ptm->ptm_owner, owner, - new); - if (next == owner) { - errno = serrno; + newval = (void *)((uintptr_t)self | (uintptr_t)owner); + next = atomic_cas_ptr(&ptm->ptm_owner, owner, newval); + if (__predict_false(next != owner)) { + owner = next; + continue; + } + errno = serrno; #ifndef PTHREAD__ATOMIC_IS_MEMBAR - membar_enter(); + membar_enter(); #endif - return 0; - } - owner = next; - } while (MUTEX_OWNER(owner) == 0); - /* - * We have lost the race to acquire the mutex. - * The new owner could be running on another - * CPU, in which case we should spin and avoid - * the overhead of blocking. - */ - continue; + return 0; + } else if (MUTEX_OWNER(owner) != (uintptr_t)self) { + /* Spin while the owner is running. */ + owner = pthread__mutex_spin(ptm, owner); + if (MUTEX_OWNER(owner) == 0) { + continue; + } } /* @@ -365,32 +328,42 @@ pthread__mutex_lock_slow(pthread_mutex_t self->pt_mutexwait = 1; for (waiters = ptm->ptm_waiters;; waiters = next) { self->pt_mutexnext = waiters; +#ifndef PTHREAD__ATOMIC_IS_MEMBAR membar_producer(); +#endif next = atomic_cas_ptr(&ptm->ptm_waiters, waiters, self); if (next == waiters) break; } - /* Set the waiters bit and block. */ + /* + * Try to set the waiters bit. If the mutex has become free + * since entering self onto the waiters list, need to wake + * everybody up (including self) and retry. It's possible + * to race with the unlocking thread, so self may have + * already been awoken. + */ +#ifndef PTHREAD__ATOMIC_IS_MEMBAR membar_sync(); - if (pthread__mutex_setwaiters(self, ptm)) { - continue; +#endif + next = atomic_cas_ptr(&ptm->ptm_owner, owner, + (void *)((uintptr_t)owner | MUTEX_WAITERS_BIT)); + if (next != owner) { + pthread__mutex_wakeup(self, ptm); } /* - * We may have been awoken by the current thread above, - * or will be awoken by the current holder of the mutex. - * The key requirement is that we must not proceed until - * told that we are no longer waiting (via pt_mutexwait - * being set to zero). Otherwise it is unsafe to re-enter - * the thread onto the waiters list. + * We must not proceed until told that we are no longer + * waiting (via pt_mutexwait being set to zero). Otherwise + * it is unsafe to re-enter the thread onto the waiters + * list. */ +#ifndef PTHREAD__ATOMIC_IS_MEMBAR membar_sync(); +#endif while (self->pt_mutexwait) { error = _lwp_park(CLOCK_REALTIME, TIMER_ABSTIME, - __UNCONST(ts), self->pt_unpark, - __UNVOLATILE(&ptm->ptm_waiters), - __UNVOLATILE(&ptm->ptm_waiters)); + __UNCONST(ts), self->pt_unpark, NULL, NULL); self->pt_unpark = 0; if (__predict_true(error != -1)) { continue; @@ -401,9 +374,11 @@ pthread__mutex_lock_slow(pthread_mutex_t /*priority protect*/ if (MUTEX_PROTECT(owner)) (void)_sched_protect(-1); + errno = serrno; return ETIMEDOUT; } } + owner = ptm->ptm_owner; } } @@ -454,7 +429,8 @@ int pthread_mutex_unlock(pthread_mutex_t *ptm) { pthread_t self; - void *value; + void *val; + int error; if (__predict_false(__uselibcstub)) return __libc_mutex_unlock_stub(ptm); @@ -465,85 +441,63 @@ pthread_mutex_unlock(pthread_mutex_t *pt #ifndef PTHREAD__ATOMIC_IS_MEMBAR membar_exit(); #endif - self = pthread__self(); - value = atomic_cas_ptr(&ptm->ptm_owner, self, NULL); - if (__predict_true(value == self)) { - pthread__smt_wake(); - return 0; - } - return pthread__mutex_unlock_slow(ptm); -} - -NOINLINE static int -pthread__mutex_unlock_slow(pthread_mutex_t *ptm) -{ - pthread_t self, owner, new; - int weown, error; - - self = pthread__self(); - owner = ptm->ptm_owner; - weown = (MUTEX_OWNER(owner) == (uintptr_t)self); error = 0; + self = pthread__self(); - if (__SIMPLELOCK_LOCKED_P(&ptm->ptm_errorcheck)) { - if (!weown) { - error = EPERM; - new = owner; + val = atomic_cas_ptr(&ptm->ptm_owner, self, NULL); + if (__predict_false(val != self)) { + bool weown = (MUTEX_OWNER(val) == (uintptr_t)self); + void *newval = val; + if (__SIMPLELOCK_LOCKED_P(&ptm->ptm_errorcheck)) { + if (!weown) { + error = EPERM; + newval = val; + } else { + newval = NULL; + } + } else if (MUTEX_RECURSIVE(val)) { + if (!weown) { + error = EPERM; + newval = val; + } else if (ptm->ptm_recursed) { + ptm->ptm_recursed--; + newval = val; + } else { + newval = (pthread_t)MUTEX_RECURSIVE_BIT; + } } else { - new = NULL; + pthread__error(EPERM, + "Unlocking unlocked mutex", (val != NULL)); + pthread__error(EPERM, + "Unlocking mutex owned by another thread", weown); + newval = NULL; } - } else if (MUTEX_RECURSIVE(owner)) { - if (!weown) { - error = EPERM; - new = owner; - } else if (ptm->ptm_recursed) { - ptm->ptm_recursed--; - new = owner; - } else { - new = (pthread_t)MUTEX_RECURSIVE_BIT; + + /* + * Release the mutex. If there appear to be waiters, then + * wake them up. + */ + if (newval != val) { + val = atomic_swap_ptr(&ptm->ptm_owner, newval); + if (__predict_false(MUTEX_PROTECT(val))) { + /* restore elevated priority */ + (void)_sched_protect(-1); + } } - } else { - pthread__error(EPERM, - "Unlocking unlocked mutex", (owner != NULL)); - pthread__error(EPERM, - "Unlocking mutex owned by another thread", weown); - new = NULL; } + pthread__smt_wake(); /* - * Release the mutex. If there appear to be waiters, then - * wake them up. + * Finally, wake any waiters and return. */ - if (new != owner) { - owner = atomic_swap_ptr(&ptm->ptm_owner, new); - if (__predict_false(MUTEX_PROTECT(owner))) { - /* restore elevated priority */ - (void)_sched_protect(-1); - } - if (MUTEX_HAS_WAITERS(owner) != 0) { - pthread__mutex_wakeup(self, ptm); - return 0; - } - error = 0; - } - - if (self->pt_nwaiters == 1) { - /* - * If the calling thread is about to block, defer - * unparking the target until _lwp_park() is called. - */ - if (self->pt_willpark && self->pt_unpark == 0) { - self->pt_unpark = self->pt_waiters[0]; - } else { - (void)_lwp_unpark(self->pt_waiters[0], - __UNVOLATILE(&ptm->ptm_waiters)); - } +#ifndef PTHREAD__ATOMIC_IS_MEMBAR + membar_enter(); +#endif + if (MUTEX_HAS_WAITERS(val)) { + pthread__mutex_wakeup(self, ptm); } else if (self->pt_nwaiters > 0) { - (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, - __UNVOLATILE(&ptm->ptm_waiters)); + pthread__clear_waiters(self); } - self->pt_nwaiters = 0; - return error; } @@ -557,62 +511,34 @@ static void pthread__mutex_wakeup(pthread_t self, pthread_mutex_t *ptm) { pthread_t thread, next; - ssize_t n, rv; /* Take ownership of the current set of waiters. */ thread = atomic_swap_ptr(&ptm->ptm_waiters, NULL); membar_datadep_consumer(); /* for alpha */ pthread__smt_wake(); - for (;;) { - /* - * Pull waiters from the queue and add to our list. - * Use a memory barrier to ensure that we safely - * read the value of pt_mutexnext before 'thread' - * sees pt_mutexwait being cleared. - */ - for (n = self->pt_nwaiters, self->pt_nwaiters = 0; - n < pthread__unpark_max && thread != NULL; - thread = next) { - next = thread->pt_mutexnext; - if (thread != self) { - self->pt_waiters[n++] = thread->pt_lid; + /* + * Pull waiters from the queue and add to our list. Use a memory + * barrier to ensure that we safely read the value of pt_mutexnext + * before 'thread' sees pt_mutexwait being cleared. + */ + while (thread != NULL) { + if (self->pt_nwaiters < pthread__unpark_max) { + next = thread->pt_mutexnext; + if (thread != self) { + self->pt_waiters[self->pt_nwaiters++] = + thread->pt_lid; membar_sync(); } thread->pt_mutexwait = 0; /* No longer safe to touch 'thread' */ + thread = next; + continue; } - - switch (n) { - case 0: - return; - case 1: - /* - * If the calling thread is about to block, - * defer unparking the target until _lwp_park() - * is called. - */ - if (self->pt_willpark && self->pt_unpark == 0) { - self->pt_unpark = self->pt_waiters[0]; - return; - } - rv = (ssize_t)_lwp_unpark(self->pt_waiters[0], - __UNVOLATILE(&ptm->ptm_waiters)); - if (rv != 0 && errno != EALREADY && errno != EINTR && - errno != ESRCH) { - pthread__errorfunc(__FILE__, __LINE__, - __func__, "_lwp_unpark failed"); - } - return; - default: - rv = _lwp_unpark_all(self->pt_waiters, (size_t)n, - __UNVOLATILE(&ptm->ptm_waiters)); - if (rv != 0 && errno != EINTR) { - pthread__errorfunc(__FILE__, __LINE__, - __func__, "_lwp_unpark_all failed"); - } - break; - } + pthread__clear_waiters(self); + } + if (self->pt_nwaiters > 0) { + pthread__clear_waiters(self); } } @@ -763,26 +689,17 @@ pthread_mutexattr_setpshared(pthread_mut /* * pthread__mutex_deferwake: try to defer unparking threads in self->pt_waiters * - * In order to avoid unnecessary contention on the interlocking mutex, - * we defer waking up threads until we unlock the mutex. The threads will - * be woken up when the calling thread (self) releases the first mutex with - * MUTEX_DEFERRED_BIT set. It likely be the mutex 'ptm', but no problem - * even if it isn't. + * In order to avoid unnecessary contention on interlocking mutexes, we try + * to defer waking up threads until we unlock the mutex. The threads will + * be woken up when the calling thread (self) releases a mutex. */ - void pthread__mutex_deferwake(pthread_t self, pthread_mutex_t *ptm) { if (__predict_false(ptm == NULL || MUTEX_OWNER(ptm->ptm_owner) != (uintptr_t)self)) { - (void)_lwp_unpark_all(self->pt_waiters, self->pt_nwaiters, - __UNVOLATILE(&ptm->ptm_waiters)); - self->pt_nwaiters = 0; - } else { - atomic_or_ulong((volatile unsigned long *) - (uintptr_t)&ptm->ptm_owner, - (unsigned long)MUTEX_DEFERRED_BIT); + pthread__clear_waiters(self); } } Index: src/lib/libpthread/pthread_rwlock.c diff -u src/lib/libpthread/pthread_rwlock.c:1.39 src/lib/libpthread/pthread_rwlock.c:1.40 --- src/lib/libpthread/pthread_rwlock.c:1.39 Wed Feb 5 11:05:10 2020 +++ src/lib/libpthread/pthread_rwlock.c Sat May 16 22:53:37 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: pthread_rwlock.c,v 1.39 2020/02/05 11:05:10 kamil Exp $ */ +/* $NetBSD: pthread_rwlock.c,v 1.40 2020/05/16 22:53:37 ad Exp $ */ /*- * Copyright (c) 2002, 2006, 2007, 2008 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include <sys/cdefs.h> -__RCSID("$NetBSD: pthread_rwlock.c,v 1.39 2020/02/05 11:05:10 kamil Exp $"); +__RCSID("$NetBSD: pthread_rwlock.c,v 1.40 2020/05/16 22:53:37 ad Exp $"); #include <sys/types.h> #include <sys/lwpctl.h> @@ -220,7 +220,7 @@ pthread__rwlock_rdlock(pthread_rwlock_t self->pt_sleepobj = &ptr->ptr_rblocked; self->pt_early = pthread__rwlock_early; error = pthread__park(self, interlock, &ptr->ptr_rblocked, - ts, 0, &ptr->ptr_rblocked); + ts, 0); /* Did we get the lock? */ if (self->pt_rwlocked == _RW_LOCKED) { @@ -341,7 +341,7 @@ pthread__rwlock_wrlock(pthread_rwlock_t self->pt_sleepobj = &ptr->ptr_wblocked; self->pt_early = pthread__rwlock_early; error = pthread__park(self, interlock, &ptr->ptr_wblocked, - ts, 0, &ptr->ptr_wblocked); + ts, 0); /* Did we get the lock? */ if (self->pt_rwlocked == _RW_LOCKED) {