From b247fd395dc0d2f4f63e7a234c8df0fd28eebe02 Mon Sep 17 00:00:00 2001 From: Mateusz Guzik Date: Sun, 19 Feb 2017 16:28:46 +0000 Subject: [PATCH] locks: make trylock routines check for 'unowned' value Since fcmpset can fail without lock contention e.g. on arm, it was possible to get spurious failures when the caller was expecting the primitive to succeed. Reported by: mmel --- sys/kern/kern_mutex.c | 17 +++++++++++------ sys/kern/kern_rwlock.c | 11 ++++++++--- sys/kern/kern_sx.c | 11 ++++++++--- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c index 18a00532c264..7ad6507bb325 100644 --- a/sys/kern/kern_mutex.c +++ b/sys/kern/kern_mutex.c @@ -402,16 +402,21 @@ _mtx_trylock_flags_(volatile uintptr_t *c, int opts, const char *file, int line) rval = 1; recursed = false; v = MTX_UNOWNED; - if (!_mtx_obtain_lock_fetch(m, &v, tid)) { + for (;;) { + if (_mtx_obtain_lock_fetch(m, &v, tid)) + break; + if (v == MTX_UNOWNED) + continue; if (v == tid && ((m->lock_object.lo_flags & LO_RECURSABLE) != 0 || (opts & MTX_RECURSE) != 0)) { - m->mtx_recurse++; - atomic_set_ptr(&m->mtx_lock, MTX_RECURSED); - recursed = true; - } else { - rval = 0; + m->mtx_recurse++; + atomic_set_ptr(&m->mtx_lock, MTX_RECURSED); + recursed = true; + break; } + rval = 0; + break; } opts &= ~MTX_RECURSE; diff --git a/sys/kern/kern_rwlock.c b/sys/kern/kern_rwlock.c index d2310ed257ac..2e703211e50c 100644 --- a/sys/kern/kern_rwlock.c +++ b/sys/kern/kern_rwlock.c @@ -314,13 +314,18 @@ __rw_try_wlock(volatile uintptr_t *c, const char *file, int line) rval = 1; recursed = false; v = RW_UNLOCKED; - if (!atomic_fcmpset_acq_ptr(&rw->rw_lock, &v, tid)) { + for (;;) { + if (atomic_fcmpset_acq_ptr(&rw->rw_lock, &v, tid)) + break; + if (v == RW_UNLOCKED) + continue; if (v == tid && (rw->lock_object.lo_flags & LO_RECURSABLE)) { rw->rw_recurse++; atomic_set_ptr(&rw->rw_lock, RW_LOCK_WRITER_RECURSED); - } else { - rval = 0; + break; } + rval = 0; + break; } LOCK_LOG_TRY("WLOCK", &rw->lock_object, 0, rval, file, line); diff --git a/sys/kern/kern_sx.c b/sys/kern/kern_sx.c index d7f3f1348230..3236e03885dd 100644 --- a/sys/kern/kern_sx.c +++ b/sys/kern/kern_sx.c @@ -341,13 +341,18 @@ sx_try_xlock_(struct sx *sx, const char *file, int line) rval = 1; recursed = false; x = SX_LOCK_UNLOCKED; - if (!atomic_fcmpset_acq_ptr(&sx->sx_lock, &x, tid)) { + for (;;) { + if (atomic_fcmpset_acq_ptr(&sx->sx_lock, &x, tid)) + break; + if (x == SX_LOCK_UNLOCKED) + continue; if (x == tid && (sx->lock_object.lo_flags & LO_RECURSABLE)) { sx->sx_recurse++; atomic_set_ptr(&sx->sx_lock, SX_LOCK_RECURSED); - } else { - rval = 0; + break; } + rval = 0; + break; } LOCK_LOG_TRY("XLOCK", &sx->lock_object, 0, rval, file, line);