From 8c31e244f2179705d23a9bc680f47ba978ff9bcb Mon Sep 17 00:00:00 2001 From: davidxu Date: Thu, 5 Apr 2012 02:24:08 +0000 Subject: 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 --- lib/libthr/thread/thr_private.h | 2 -- lib/libthr/thread/thr_umtx.h | 21 ++++++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) (limited to 'lib/libthr') 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 -- cgit v1.1