umtx operation UMTX_OP_MUTEX_WAKE has a side-effect that it accesses

a mutex after a thread has unlocked it, it event writes data to the mutex
memory to clear contention bit, there is a race that other threads
can lock it and unlock it, then destroy it, so it should not write
data to the mutex memory if there isn't any waiter.
The new operation UMTX_OP_MUTEX_WAKE2 try to fix the problem. It
requires thread library to clear the lock word entirely, then
call the WAKE2 operation to check if there is any waiter in kernel,
and try to wake up a thread, if necessary, the contention bit is set again
by the operation. This also mitgates the chance that other threads find
the contention bit and try to enter kernel to compete with each other
to wake up sleeping thread, this is unnecessary. With this change, the
mutex owner is no longer holding the mutex until it reaches a point
where kernel umtx queue is locked, it releases the mutex as soon as
possible.
Performance is improved when the mutex is contensted heavily.  On Intel
i3-2310M, the runtime of a benchmark program is reduced from 26.87 seconds
to 2.39 seconds, it even is better than UMTX_OP_MUTEX_WAKE which is
deprecated now. http://people.freebsd.org/~davidxu/bench/mutex_perf.c

4 files changed, 103 insertions, 9 deletions

diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h
index 4b9219d..87af318 100644
--- a/lib/libthr/thread/thr_private.h
+++ b/lib/libthr/thread/thr_private.h
@@ -834,8 +834,6 @@ ssize_t __sys_write(int, const void *, size_t);
 void	__sys_exit(int);
 #endif
 
-int	_umtx_op_err(void *, int op, u_long, void *, void *) __hidden;
-
 static inline int
 _thr_isthreaded(void)
 {
diff --git a/lib/libthr/thread/thr_umtx.h b/lib/libthr/thread/thr_umtx.h
index 61a74a8..d74cc81 100644
--- a/lib/libthr/thread/thr_umtx.h
+++ b/lib/libthr/thread/thr_umtx.h
@@ -35,6 +35,7 @@
 #define DEFAULT_UMUTEX	{0,0,{0,0},{0,0,0,0}}
 #define DEFAULT_URWLOCK {0,0,0,0,{0,0,0,0}}
 
+int _umtx_op_err(void *, int op, u_long, void *, void *) __hidden;
 int __thr_umutex_lock(struct umutex *mtx, uint32_t id) __hidden;
 int __thr_umutex_lock_spin(struct umutex *mtx, uint32_t id) __hidden;
 int __thr_umutex_timedlock(struct umutex *mtx, uint32_t id,
@@ -121,9 +122,23 @@ _thr_umutex_timedlock(struct umutex *mtx, uint32_t id,
 static inline int
 _thr_umutex_unlock(struct umutex *mtx, uint32_t id)
 {
-    if (atomic_cmpset_rel_32(&mtx->m_owner, id, UMUTEX_UNOWNED))
-	return (0);
-    return (__thr_umutex_unlock(mtx, id));
+	uint32_t flags = mtx->m_flags;
+
+	if ((flags & (UMUTEX_PRIO_PROTECT | UMUTEX_PRIO_INHERIT)) == 0) {
+		uint32_t owner;
+		do {
+			owner = mtx->m_owner;
+			if (__predict_false((owner & ~UMUTEX_CONTESTED) != id))
+				return (EPERM);
+		} while (__predict_false(!atomic_cmpset_rel_32(&mtx->m_owner,
+					 owner, UMUTEX_UNOWNED)));
+		if ((owner & UMUTEX_CONTESTED))
+			(void)_umtx_op_err(mtx, UMTX_OP_MUTEX_WAKE2, flags, 0, 0);
+		return (0);
+	}
+    	if (atomic_cmpset_rel_32(&mtx->m_owner, id, UMUTEX_UNOWNED))
+		return (0);
+	return (__thr_umutex_unlock(mtx, id));
 }
 
 static inline int
diff --git a/sys/kern/kern_umtx.c b/sys/kern/kern_umtx.c
index 5c57166..71b19e7 100644
--- a/sys/kern/kern_umtx.c
+++ b/sys/kern/kern_umtx.c
@@ -1319,6 +1319,78 @@ do_wake_umutex(struct thread *td, struct umutex *m)
 	return (0);
 }
 
+/*
+ * Check if the mutex has waiters and tries to fix contention bit.
+ */
+static int
+do_wake2_umutex(struct thread *td, struct umutex *m, uint32_t flags)
+{
+	struct umtx_key key;
+	uint32_t owner, old;
+	int type;
+	int error;
+	int count;
+
+	switch(flags & (UMUTEX_PRIO_INHERIT | UMUTEX_PRIO_PROTECT)) {
+	case 0:
+		type = TYPE_NORMAL_UMUTEX;
+		break;
+	case UMUTEX_PRIO_INHERIT:
+		type = TYPE_PI_UMUTEX;
+		break;
+	case UMUTEX_PRIO_PROTECT:
+		type = TYPE_PP_UMUTEX;
+		break;
+	default:
+		return (EINVAL);
+	}
+	if ((error = umtx_key_get(m, type, GET_SHARE(flags),
+	    &key)) != 0)
+		return (error);
+
+	owner = 0;
+	umtxq_lock(&key);
+	umtxq_busy(&key);
+	count = umtxq_count(&key);
+	umtxq_unlock(&key);
+	/*
+	 * Only repair contention bit if there is a waiter, this means the mutex
+	 * is still being referenced by userland code, otherwise don't update
+	 * any memory.
+	 */
+	if (count > 1) {
+		owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
+		while ((owner & UMUTEX_CONTESTED) ==0) {
+			old = casuword32(&m->m_owner, owner,
+			    owner|UMUTEX_CONTESTED);
+			if (old == owner)
+				break;
+			owner = old;
+		}
+	} else if (count == 1) {
+		owner = fuword32(__DEVOLATILE(uint32_t *, &m->m_owner));
+		while ((owner & ~UMUTEX_CONTESTED) != 0 &&
+		       (owner & UMUTEX_CONTESTED) == 0) {
+			old = casuword32(&m->m_owner, owner,
+			    owner|UMUTEX_CONTESTED);
+			if (old == owner)
+				break;
+			owner = old;
+		}
+	}
+	umtxq_lock(&key);
+	if (owner == -1) {
+		error = EFAULT;
+		umtxq_signal(&key, INT_MAX);
+	}
+	else if (count != 0 && (owner & ~UMUTEX_CONTESTED) == 0)
+		umtxq_signal(&key, 1);
+	umtxq_unbusy(&key);
+	umtxq_unlock(&key);
+	umtx_key_release(&key);
+	return (error);
+}
+
 static inline struct umtx_pi *
 umtx_pi_alloc(int flags)
 {
@@ -3152,6 +3224,12 @@ __umtx_op_sem_wake(struct thread *td, struct _umtx_op_args *uap)
 	return do_sem_wake(td, uap->obj);
 }
 
+static int
+__umtx_op_wake2_umutex(struct thread *td, struct _umtx_op_args *uap)
+{
+	return do_wake2_umutex(td, uap->obj, uap->val);
+}
+
 typedef int (*_umtx_op_func)(struct thread *td, struct _umtx_op_args *uap);
 
 static _umtx_op_func op_table[] = {
@@ -3176,7 +3254,8 @@ static _umtx_op_func op_table[] = {
 	__umtx_op_wake_umutex,		/* UMTX_OP_UMUTEX_WAKE */
 	__umtx_op_sem_wait,		/* UMTX_OP_SEM_WAIT */
 	__umtx_op_sem_wake,		/* UMTX_OP_SEM_WAKE */
-	__umtx_op_nwake_private		/* UMTX_OP_NWAKE_PRIVATE */
+	__umtx_op_nwake_private,	/* UMTX_OP_NWAKE_PRIVATE */
+	__umtx_op_wake2_umutex		/* UMTX_OP_UMUTEX_WAKE2 */
 };
 
 int
@@ -3478,7 +3557,8 @@ static _umtx_op_func op_table_compat32[] = {
 	__umtx_op_wake_umutex,		/* UMTX_OP_UMUTEX_WAKE */
 	__umtx_op_sem_wait_compat32,	/* UMTX_OP_SEM_WAIT */
 	__umtx_op_sem_wake,		/* UMTX_OP_SEM_WAKE */
-	__umtx_op_nwake_private32	/* UMTX_OP_NWAKE_PRIVATE */
+	__umtx_op_nwake_private32,	/* UMTX_OP_NWAKE_PRIVATE */
+	__umtx_op_wake2_umutex		/* UMTX_OP_UMUTEX_WAKE2 */
 };
 
 int
diff --git a/sys/sys/umtx.h b/sys/sys/umtx.h
index aa90629..8becd20 100644
--- a/sys/sys/umtx.h
+++ b/sys/sys/umtx.h
@@ -76,11 +76,12 @@
 #define	UMTX_OP_WAIT_UINT_PRIVATE	15
 #define	UMTX_OP_WAKE_PRIVATE	16
 #define	UMTX_OP_MUTEX_WAIT	17
-#define	UMTX_OP_MUTEX_WAKE	18
+#define	UMTX_OP_MUTEX_WAKE	18	/* deprecated */
 #define	UMTX_OP_SEM_WAIT	19
 #define	UMTX_OP_SEM_WAKE	20
 #define	UMTX_OP_NWAKE_PRIVATE   21
-#define	UMTX_OP_MAX		22
+#define	UMTX_OP_MUTEX_WAKE2	22
+#define	UMTX_OP_MAX		23
 
 /* Flags for UMTX_OP_CV_WAIT */
 #define	CVWAIT_CHECK_UNPARKING	0x01