d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

mtx: reduce lock accesses

Browse Source 
Instead of spuriously re-reading the lock value, read it once.

This change also has a side effect of fixing a performance bug:
on failed _mtx_obtain_lock, it was possible that re-read would find
the lock is unowned, but in this case the primitive would make a trip
through turnstile code.

This is diff reduction to a variant which uses atomic_fcmpset.

Discussed with:	jhb (previous version)
Tested by:	pho (previous version)
This commit is contained in:
Mateusz Guzik 2017-01-03 21:36:15 +00:00
parent 68278ec60f
commit 2604eb9e17
2 changed files with 57 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

89
sys/kern/kern_mutex.c
View File

@ -94,8 +94,6 @@ PMC_SOFT_DEFINE( , , lock, failed);
#define mtx_destroyed(m) ((m)->mtx_lock == MTX_DESTROYED)
#define mtx_owner(m) ((struct thread *)((m)->mtx_lock & ~MTX_FLAGMASK))
static void assert_mtx(const struct lock_object *lock, int what);
#ifdef DDB
static void db_show_mtx(const struct lock_object *lock);
@ -491,8 +489,9 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
lock_delay_arg_init(&lda, NULL);
#endif
m = mtxlock2mtx(c);
v = MTX_READ_VALUE(m);
if (mtx_owned(m)) {
if (__predict_false(lv_mtx_owner(v) == (struct thread *)tid)) {
KASSERT((m->lock_object.lo_flags & LO_RECURSABLE) != 0 ||
(opts & MTX_RECURSE) != 0,
("_mtx_lock_sleep: recursed on non-recursive mutex %s @ %s:%d\n",
@ -520,8 +519,12 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
#endif
for (;;) {
if (m->mtx_lock == MTX_UNOWNED && _mtx_obtain_lock(m, tid))
break;
if (v == MTX_UNOWNED) {
if (_mtx_obtain_lock(m, tid))
break;
v = MTX_READ_VALUE(m);
continue;
}
#ifdef KDTRACE_HOOKS
lda.spin_cnt++;
#endif
@ -530,31 +533,30 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
* If the owner is running on another CPU, spin until the
* owner stops running or the state of the lock changes.
*/
v = m->mtx_lock;
if (v != MTX_UNOWNED) {
owner = (struct thread *)(v & ~MTX_FLAGMASK);
if (TD_IS_RUNNING(owner)) {
if (LOCK_LOG_TEST(&m->lock_object, 0))
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, m, owner);
KTR_STATE1(KTR_SCHED, "thread",
sched_tdname((struct thread *)tid),
"spinning", "lockname:\"%s\"",
m->lock_object.lo_name);
while (mtx_owner(m) == owner &&
TD_IS_RUNNING(owner))
lock_delay(&lda);
KTR_STATE0(KTR_SCHED, "thread",
sched_tdname((struct thread *)tid),
"running");
continue;
}
owner = lv_mtx_owner(v);
if (TD_IS_RUNNING(owner)) {
if (LOCK_LOG_TEST(&m->lock_object, 0))
CTR3(KTR_LOCK,
"%s: spinning on %p held by %p",
__func__, m, owner);
KTR_STATE1(KTR_SCHED, "thread",
sched_tdname((struct thread *)tid),
"spinning", "lockname:\"%s\"",
m->lock_object.lo_name);
do {
lock_delay(&lda);
v = m->mtx_lock;
owner = lv_mtx_owner(v);
} while (v != MTX_UNOWNED && TD_IS_RUNNING(owner));
KTR_STATE0(KTR_SCHED, "thread",
sched_tdname((struct thread *)tid),
"running");
continue;
}
#endif
ts = turnstile_trywait(&m->lock_object);
v = m->mtx_lock;
v = MTX_READ_VALUE(m);
/*
* Check if the lock has been released while spinning for
@ -573,7 +575,7 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
* chain lock. If so, drop the turnstile lock and try
* again.
*/
owner = (struct thread *)(v & ~MTX_FLAGMASK);
owner = lv_mtx_owner(v);
if (TD_IS_RUNNING(owner)) {
turnstile_cancel(ts);
continue;
@ -588,6 +590,7 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
if ((v & MTX_CONTESTED) == 0 &&
!atomic_cmpset_ptr(&m->mtx_lock, v, v | MTX_CONTESTED)) {
turnstile_cancel(ts);
v = MTX_READ_VALUE(m);
continue;
}
@ -618,6 +621,7 @@ __mtx_lock_sleep(volatile uintptr_t *c, uintptr_t tid, int opts,
sleep_time += lockstat_nsecs(&m->lock_object);
sleep_cnt++;
#endif
v = MTX_READ_VALUE(m);
}
#ifdef KDTRACE_HOOKS
all_time += lockstat_nsecs(&m->lock_object);
@ -675,6 +679,7 @@ _mtx_lock_spin_cookie(volatile uintptr_t *c, uintptr_t tid, int opts,
{
struct mtx *m;
struct lock_delay_arg lda;
uintptr_t v;
#ifdef LOCK_PROFILING
int contested = 0;
uint64_t waittime = 0;
@ -701,24 +706,30 @@ _mtx_lock_spin_cookie(volatile uintptr_t *c, uintptr_t tid, int opts,
#ifdef KDTRACE_HOOKS
spin_time -= lockstat_nsecs(&m->lock_object);
#endif
v = MTX_READ_VALUE(m);
for (;;) {
if (m->mtx_lock == MTX_UNOWNED && _mtx_obtain_lock(m, tid))
break;
if (v == MTX_UNOWNED) {
if (_mtx_obtain_lock(m, tid))
break;
v = MTX_READ_VALUE(m);
continue;
}
/* Give interrupts a chance while we spin. */
spinlock_exit();
while (m->mtx_lock != MTX_UNOWNED) {
do {
if (lda.spin_cnt < 10000000) {
lock_delay(&lda);
continue;
} else {
lda.spin_cnt++;
if (lda.spin_cnt < 60000000 || kdb_active ||
panicstr != NULL)
DELAY(1);
else
_mtx_lock_spin_failed(m);
cpu_spinwait();
}
lda.spin_cnt++;
if (lda.spin_cnt < 60000000 || kdb_active ||
panicstr != NULL)
DELAY(1);
else
_mtx_lock_spin_failed(m);
cpu_spinwait();
}
v = MTX_READ_VALUE(m);
} while (v != MTX_UNOWNED);
spinlock_enter();
}
#ifdef KDTRACE_HOOKS

8
sys/sys/mutex.h
View File

@ -420,9 +420,15 @@ extern struct mtx_pool *mtxpool_sleep;
_sleep((chan), &(mtx)->lock_object, (pri), (wmesg), \
tick_sbt * (timo), 0, C_HARDCLOCK)
#define MTX_READ_VALUE(m) ((m)->mtx_lock)
#define mtx_initialized(m) lock_initialized(&(m)->lock_object)
#define mtx_owned(m) (((m)->mtx_lock & ~MTX_FLAGMASK) == (uintptr_t)curthread)
#define lv_mtx_owner(v) ((struct thread *)((v) & ~MTX_FLAGMASK))
#define mtx_owner(m) lv_mtx_owner(MTX_READ_VALUE(m))
#define mtx_owned(m) (mtx_owner(m) == curthread)
#define mtx_recursed(m) ((m)->mtx_recurse != 0)