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) {

Reply via email to