Fix the turnstile_lock() KPI.

turnstile_{lock,unlock}() were added for use in epoch.  turnstile_lock()
returned NULL to indicate that the calling thread had lost a race and
the turnstile was no longer associated with the given lock, or the lock
owner.  However, reader-writer locks may not have a designated owner,
in which case turnstile_lock() would return NULL and
epoch_block_handler_preempt() would leak spinlocks as a result.

Apply a minimal fix: return the lock owner as a separate return value.

Reviewed by:	kib
MFC after:	3 days
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D21048
This commit is contained in:
Mark Johnston 2019-07-24 23:04:59 +00:00
parent ae5f2ca7b9
commit 2fb62b1a46
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=350310
3 changed files with 22 additions and 25 deletions

View File

@ -438,26 +438,20 @@ epoch_block_handler_preempt(struct ck_epoch *global __unused,
*/ */
critical_enter(); critical_enter();
thread_unlock(td); thread_unlock(td);
owner = turnstile_lock(ts, &lock);
/* if (turnstile_lock(ts, &lock, &owner)) {
* The owner pointer indicates that the lock succeeded. if (ts == curwaittd->td_blocked) {
* Only in case we hold the lock and the turnstile we MPASS(TD_IS_INHIBITED(curwaittd) &&
* locked is still the one that curwaittd is blocked on TD_ON_LOCK(curwaittd));
* can we continue. Otherwise the turnstile pointer has critical_exit();
* been changed out from underneath us, as in the case turnstile_wait(ts, owner,
* where the lock holder has signalled curwaittd, curwaittd->td_tsqueue);
* and we need to continue. counter_u64_add(turnstile_count, 1);
*/ thread_lock(td);
if (owner != NULL && ts == curwaittd->td_blocked) { return;
MPASS(TD_IS_INHIBITED(curwaittd) && }
TD_ON_LOCK(curwaittd));
critical_exit();
turnstile_wait(ts, owner, curwaittd->td_tsqueue);
counter_u64_add(turnstile_count, 1);
thread_lock(td);
return;
} else if (owner != NULL)
turnstile_unlock(ts, lock); turnstile_unlock(ts, lock);
}
thread_lock(td); thread_lock(td);
critical_exit(); critical_exit();
KASSERT(td->td_locks == locksheld, KASSERT(td->td_locks == locksheld,

View File

@ -566,24 +566,26 @@ turnstile_trywait(struct lock_object *lock)
return (ts); return (ts);
} }
struct thread * bool
turnstile_lock(struct turnstile *ts, struct lock_object **lockp) turnstile_lock(struct turnstile *ts, struct lock_object **lockp,
struct thread **tdp)
{ {
struct turnstile_chain *tc; struct turnstile_chain *tc;
struct lock_object *lock; struct lock_object *lock;
if ((lock = ts->ts_lockobj) == NULL) if ((lock = ts->ts_lockobj) == NULL)
return (NULL); return (false);
tc = TC_LOOKUP(lock); tc = TC_LOOKUP(lock);
mtx_lock_spin(&tc->tc_lock); mtx_lock_spin(&tc->tc_lock);
mtx_lock_spin(&ts->ts_lock); mtx_lock_spin(&ts->ts_lock);
if (__predict_false(lock != ts->ts_lockobj)) { if (__predict_false(lock != ts->ts_lockobj)) {
mtx_unlock_spin(&tc->tc_lock); mtx_unlock_spin(&tc->tc_lock);
mtx_unlock_spin(&ts->ts_lock); mtx_unlock_spin(&ts->ts_lock);
return (NULL); return (false);
} }
*lockp = lock; *lockp = lock;
return (ts->ts_owner); *tdp = ts->ts_owner;
return (true);
} }
void void

View File

@ -99,7 +99,8 @@ int turnstile_signal(struct turnstile *, int);
struct turnstile *turnstile_trywait(struct lock_object *); struct turnstile *turnstile_trywait(struct lock_object *);
void turnstile_unpend(struct turnstile *); void turnstile_unpend(struct turnstile *);
void turnstile_wait(struct turnstile *, struct thread *, int); void turnstile_wait(struct turnstile *, struct thread *, int);
struct thread *turnstile_lock(struct turnstile *, struct lock_object **); bool turnstile_lock(struct turnstile *, struct lock_object **,
struct thread **);
void turnstile_unlock(struct turnstile *, struct lock_object *); void turnstile_unlock(struct turnstile *, struct lock_object *);
void turnstile_assert(struct turnstile *); void turnstile_assert(struct turnstile *);
#endif /* _KERNEL */ #endif /* _KERNEL */