Date: Thu, 3 May 2012 03:05:19 +0000 (UTC) From: David Xu <davidxu@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r234937 - in stable/8: lib/libthr/thread sys/kern sys/sys Message-ID: <201205030305.q4335Js5015973@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: davidxu Date: Thu May 3 03:05:18 2012 New Revision: 234937 URL: http://svn.freebsd.org/changeset/base/234937 Log: Merge 233103, 233912 from head: 233103: Some software think a mutex can be destroyed after it owned it, for example, it uses a serialization point like following: pthread_mutex_lock(&mutex); pthread_mutex_unlock(&mutex); pthread_mutex_destroy(&muetx); They think a previous lock holder should have already left the mutex and is no longer referencing it, so they destroy it. To be maximum compatible with such code, we use IA64 version to unlock the mutex in kernel, remove the two steps unlocking code. 233912: 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 Special code for stable/8: And add code to detect if the UMTX_OP_MUTEX_WAKE2 is available. PR: threads/167308 Modified: stable/8/lib/libthr/thread/thr_private.h stable/8/lib/libthr/thread/thr_umtx.c stable/8/lib/libthr/thread/thr_umtx.h stable/8/sys/kern/kern_umtx.c stable/8/sys/sys/umtx.h Directory Properties: stable/8/lib/libthr/ (props changed) Modified: stable/8/lib/libthr/thread/thr_private.h ============================================================================== --- stable/8/lib/libthr/thread/thr_private.h Thu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_private.h Thu May 3 03:05:18 2012 (r234937) @@ -706,8 +706,6 @@ ssize_t __sys_write(int, const void *, s void __sys_exit(int); #endif -int _umtx_op_err(void *, int op, u_long, void *, void *) __hidden; - static inline int _thr_isthreaded(void) { Modified: stable/8/lib/libthr/thread/thr_umtx.c ============================================================================== --- stable/8/lib/libthr/thread/thr_umtx.c Thu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_umtx.c Thu May 3 03:05:18 2012 (r234937) @@ -112,6 +112,35 @@ __thr_umutex_timedlock(struct umutex *mt int __thr_umutex_unlock(struct umutex *mtx, uint32_t id) { + static int wake2_avail = 0; + + if (__predict_false(wake2_avail == 0)) { + struct umutex test = DEFAULT_UMUTEX; + + if (_umtx_op(&test, UMTX_OP_MUTEX_WAKE2, test.m_flags, 0, 0) == -1) + wake2_avail = -1; + else + wake2_avail = 1; + } + + if (wake2_avail != 1) + goto unlock; + + 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); + } +unlock: return _umtx_op_err(mtx, UMTX_OP_MUTEX_UNLOCK, 0, 0, 0); } Modified: stable/8/lib/libthr/thread/thr_umtx.h ============================================================================== --- stable/8/lib/libthr/thread/thr_umtx.h Thu May 3 01:41:12 2012 (r234936) +++ stable/8/lib/libthr/thread/thr_umtx.h Thu May 3 03:05:18 2012 (r234937) @@ -34,6 +34,7 @@ #define DEFAULT_UMUTEX {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_timedlock(struct umutex *mtx, uint32_t id, const struct timespec *timeout) __hidden; @@ -100,9 +101,9 @@ _thr_umutex_timedlock(struct umutex *mtx 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)); + if (atomic_cmpset_rel_32(&mtx->m_owner, id, UMUTEX_UNOWNED)) + return (0); + return (__thr_umutex_unlock(mtx, id)); } static inline int Modified: stable/8/sys/kern/kern_umtx.c ============================================================================== --- stable/8/sys/kern/kern_umtx.c Thu May 3 01:41:12 2012 (r234936) +++ stable/8/sys/kern/kern_umtx.c Thu May 3 03:05:18 2012 (r234937) @@ -1261,6 +1261,78 @@ do_wake_umutex(struct thread *td, struct 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) { @@ -3026,6 +3098,18 @@ __umtx_op_rw_unlock(struct thread *td, s return do_rw_unlock(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); +} + +static int +__umtx_op_not_sup(struct thread *td __unused, struct _umtx_op_args *uap __unused) +{ + return ENOTSUP; +} + typedef int (*_umtx_op_func)(struct thread *td, struct _umtx_op_args *uap); static _umtx_op_func op_table[] = { @@ -3047,7 +3131,11 @@ static _umtx_op_func op_table[] = { __umtx_op_wait_uint_private, /* UMTX_OP_WAIT_UINT_PRIVATE */ __umtx_op_wake_private, /* UMTX_OP_WAKE_PRIVATE */ __umtx_op_wait_umutex, /* UMTX_OP_UMUTEX_WAIT */ - __umtx_op_wake_umutex /* UMTX_OP_UMUTEX_WAKE */ + __umtx_op_wake_umutex, /* UMTX_OP_UMUTEX_WAKE */ + __umtx_op_not_sup, /* UMTX_OP_SEM_WAIT */ + __umtx_op_not_sup, /* UMTX_OP_SEM_WAKE */ + __umtx_op_not_sup, /* UMTX_OP_NWAKE_PRIVATE */ + __umtx_op_wake2_umutex /* UMTX_OP_UMUTEX_WAKE2 */ }; int @@ -3266,7 +3354,11 @@ static _umtx_op_func op_table_compat32[] __umtx_op_wait_uint_private_compat32, /* UMTX_OP_WAIT_UINT_PRIVATE */ __umtx_op_wake_private, /* UMTX_OP_WAKE_PRIVATE */ __umtx_op_wait_umutex_compat32, /* UMTX_OP_UMUTEX_WAIT */ - __umtx_op_wake_umutex /* UMTX_OP_UMUTEX_WAKE */ + __umtx_op_wake_umutex, /* UMTX_OP_UMUTEX_WAKE */ + __umtx_op_not_sup, /* UMTX_OP_SEM_WAIT */ + __umtx_op_not_sup, /* UMTX_OP_SEM_WAKE */ + __umtx_op_not_sup, /* UMTX_OP_NWAKE_PRIVATE */ + __umtx_op_wake2_umutex /* UMTX_OP_UMUTEX_WAKE2 */ }; int Modified: stable/8/sys/sys/umtx.h ============================================================================== --- stable/8/sys/sys/umtx.h Thu May 3 01:41:12 2012 (r234936) +++ stable/8/sys/sys/umtx.h Thu May 3 03:05:18 2012 (r234937) @@ -100,10 +100,11 @@ struct urwlock { #define UMTX_OP_RW_WRLOCK 13 #define UMTX_OP_RW_UNLOCK 14 #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_MAX 19 +#define UMTX_OP_WAKE_PRIVATE 16 +#define UMTX_OP_MUTEX_WAIT 17 +#define UMTX_OP_MUTEX_WAKE 18 +#define UMTX_OP_MUTEX_WAKE2 22 +#define UMTX_OP_MAX 23 /* flags for UMTX_OP_CV_WAIT */ #define UMTX_CHECK_UNPARKING 0x01
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205030305.q4335Js5015973>