1) Check NULL pointer before calling umtx_pi_adjust_locked(), this avoids

a PANIC.
2) Rework locking for POSIX priority-mutex, this fixes a
   race where a thread may wait there forever even if the mutex is unlocked.
This commit is contained in:
David Xu 2009-03-13 06:06:20 +00:00
parent e6d3b1bd2a
commit 326bf9493d
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=189756

View File

@ -166,6 +166,7 @@ struct umtxq_chain {
}; };
#define UMTXQ_LOCKED_ASSERT(uc) mtx_assert(&(uc)->uc_lock, MA_OWNED) #define UMTXQ_LOCKED_ASSERT(uc) mtx_assert(&(uc)->uc_lock, MA_OWNED)
#define UMTXQ_BUSY_ASSERT(uc) KASSERT(&(uc)->uc_busy, ("umtx chain is not busy"))
/* /*
* Don't propagate time-sharing priority, there is a security reason, * Don't propagate time-sharing priority, there is a security reason,
@ -1392,6 +1393,7 @@ umtx_unpropagate_priority(struct umtx_pi *pi)
oldpri = pi->pi_owner->td_user_pri; oldpri = pi->pi_owner->td_user_pri;
sched_unlend_user_prio(pi->pi_owner, pri); sched_unlend_user_prio(pi->pi_owner, pri);
thread_unlock(pi->pi_owner); thread_unlock(pi->pi_owner);
if (uq_owner->uq_pi_blocked != NULL)
umtx_pi_adjust_locked(pi->pi_owner, oldpri); umtx_pi_adjust_locked(pi->pi_owner, oldpri);
pi = uq_owner->uq_pi_blocked; pi = uq_owner->uq_pi_blocked;
} }
@ -1513,7 +1515,9 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi,
KASSERT(td == curthread, ("inconsistent uq_thread")); KASSERT(td == curthread, ("inconsistent uq_thread"));
uc = umtxq_getchain(&uq->uq_key); uc = umtxq_getchain(&uq->uq_key);
UMTXQ_LOCKED_ASSERT(uc); UMTXQ_LOCKED_ASSERT(uc);
UMTXQ_BUSY_ASSERT(uc);
umtxq_insert(uq); umtxq_insert(uq);
mtx_lock_spin(&umtx_lock);
if (pi->pi_owner == NULL) { if (pi->pi_owner == NULL) {
/* XXX /* XXX
* Current, We only support process private PI-mutex, * Current, We only support process private PI-mutex,
@ -1524,6 +1528,7 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi,
* For process private PI-mutex, we can find owner * For process private PI-mutex, we can find owner
* thread and boost its priority safely. * thread and boost its priority safely.
*/ */
mtx_unlock_spin(&umtx_lock);
PROC_LOCK(curproc); PROC_LOCK(curproc);
td1 = thread_find(curproc, owner); td1 = thread_find(curproc, owner);
mtx_lock_spin(&umtx_lock); mtx_lock_spin(&umtx_lock);
@ -1532,8 +1537,6 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi,
umtx_pi_setowner(pi, td1); umtx_pi_setowner(pi, td1);
} }
PROC_UNLOCK(curproc); PROC_UNLOCK(curproc);
} else {
mtx_lock_spin(&umtx_lock);
} }
TAILQ_FOREACH(uq1, &pi->pi_blocked, uq_lockq) { TAILQ_FOREACH(uq1, &pi->pi_blocked, uq_lockq) {
@ -1551,26 +1554,18 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi,
thread_lock(td); thread_lock(td);
td->td_flags |= TDF_UPIBLOCKED; td->td_flags |= TDF_UPIBLOCKED;
thread_unlock(td); thread_unlock(td);
mtx_unlock_spin(&umtx_lock);
umtxq_unlock(&uq->uq_key);
mtx_lock_spin(&umtx_lock);
umtx_propagate_priority(td); umtx_propagate_priority(td);
mtx_unlock_spin(&umtx_lock); mtx_unlock_spin(&umtx_lock);
umtxq_unbusy(&uq->uq_key);
umtxq_lock(&uq->uq_key);
if (uq->uq_flags & UQF_UMTXQ) { if (uq->uq_flags & UQF_UMTXQ) {
error = msleep(uq, &uc->uc_lock, PCATCH, wmesg, timo); error = msleep(uq, &uc->uc_lock, PCATCH, wmesg, timo);
if (error == EWOULDBLOCK) if (error == EWOULDBLOCK)
error = ETIMEDOUT; error = ETIMEDOUT;
if (uq->uq_flags & UQF_UMTXQ) { if (uq->uq_flags & UQF_UMTXQ) {
umtxq_busy(&uq->uq_key);
umtxq_remove(uq); umtxq_remove(uq);
umtxq_unbusy(&uq->uq_key);
} }
} }
umtxq_unlock(&uq->uq_key);
mtx_lock_spin(&umtx_lock); mtx_lock_spin(&umtx_lock);
uq->uq_pi_blocked = NULL; uq->uq_pi_blocked = NULL;
thread_lock(td); thread_lock(td);
@ -1579,8 +1574,7 @@ umtxq_sleep_pi(struct umtx_q *uq, struct umtx_pi *pi,
TAILQ_REMOVE(&pi->pi_blocked, uq, uq_lockq); TAILQ_REMOVE(&pi->pi_blocked, uq, uq_lockq);
umtx_unpropagate_priority(pi); umtx_unpropagate_priority(pi);
mtx_unlock_spin(&umtx_lock); mtx_unlock_spin(&umtx_lock);
umtxq_unlock(&uq->uq_key);
umtxq_lock(&uq->uq_key);
return (error); return (error);
} }
@ -1606,7 +1600,6 @@ static void
umtx_pi_unref(struct umtx_pi *pi) umtx_pi_unref(struct umtx_pi *pi)
{ {
struct umtxq_chain *uc; struct umtxq_chain *uc;
int free = 0;
uc = umtxq_getchain(&pi->pi_key); uc = umtxq_getchain(&pi->pi_key);
UMTXQ_LOCKED_ASSERT(uc); UMTXQ_LOCKED_ASSERT(uc);
@ -1622,11 +1615,9 @@ umtx_pi_unref(struct umtx_pi *pi)
("blocked queue not empty")); ("blocked queue not empty"));
mtx_unlock_spin(&umtx_lock); mtx_unlock_spin(&umtx_lock);
TAILQ_REMOVE(&uc->uc_pi_list, pi, pi_hashlink); TAILQ_REMOVE(&uc->uc_pi_list, pi, pi_hashlink);
free = 1;
}
if (free)
umtx_pi_free(pi); umtx_pi_free(pi);
} }
}
/* /*
* Find a PI mutex in hash table. * Find a PI mutex in hash table.
@ -1686,7 +1677,6 @@ _do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags, int timo,
if (new_pi == NULL) { if (new_pi == NULL) {
umtxq_unlock(&uq->uq_key); umtxq_unlock(&uq->uq_key);
new_pi = umtx_pi_alloc(M_WAITOK); new_pi = umtx_pi_alloc(M_WAITOK);
new_pi->pi_key = uq->uq_key;
umtxq_lock(&uq->uq_key); umtxq_lock(&uq->uq_key);
pi = umtx_pi_lookup(&uq->uq_key); pi = umtx_pi_lookup(&uq->uq_key);
if (pi != NULL) { if (pi != NULL) {
@ -1732,7 +1722,9 @@ _do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags, int timo,
if (owner == UMUTEX_CONTESTED) { if (owner == UMUTEX_CONTESTED) {
umtxq_lock(&uq->uq_key); umtxq_lock(&uq->uq_key);
umtxq_busy(&uq->uq_key);
error = umtx_pi_claim(pi, td); error = umtx_pi_claim(pi, td);
umtxq_unbusy(&uq->uq_key);
umtxq_unlock(&uq->uq_key); umtxq_unlock(&uq->uq_key);
break; break;
} }
@ -1787,7 +1779,6 @@ _do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags, int timo,
} }
umtxq_lock(&uq->uq_key); umtxq_lock(&uq->uq_key);
umtxq_unbusy(&uq->uq_key);
/* /*
* We set the contested bit, sleep. Otherwise the lock changed * We set the contested bit, sleep. Otherwise the lock changed
* and we need to retry or we lost a race to the thread * and we need to retry or we lost a race to the thread
@ -1796,8 +1787,11 @@ _do_lock_pi(struct thread *td, struct umutex *m, uint32_t flags, int timo,
if (old == owner) if (old == owner)
error = umtxq_sleep_pi(uq, pi, owner & ~UMUTEX_CONTESTED, error = umtxq_sleep_pi(uq, pi, owner & ~UMUTEX_CONTESTED,
"umtxpi", timo); "umtxpi", timo);
else {
umtxq_unbusy(&uq->uq_key);
umtxq_unlock(&uq->uq_key); umtxq_unlock(&uq->uq_key);
} }
}
umtxq_lock(&uq->uq_key); umtxq_lock(&uq->uq_key);
umtx_pi_unref(pi); umtx_pi_unref(pi);
@ -1851,18 +1845,26 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
umtxq_busy(&key); umtxq_busy(&key);
count = umtxq_count_pi(&key, &uq_first); count = umtxq_count_pi(&key, &uq_first);
if (uq_first != NULL) { if (uq_first != NULL) {
mtx_lock_spin(&umtx_lock);
pi = uq_first->uq_pi_blocked; pi = uq_first->uq_pi_blocked;
KASSERT(pi != NULL, ("pi == NULL?"));
if (pi->pi_owner != curthread) { if (pi->pi_owner != curthread) {
mtx_unlock_spin(&umtx_lock);
umtxq_unbusy(&key); umtxq_unbusy(&key);
umtxq_unlock(&key); umtxq_unlock(&key);
umtx_key_release(&key);
/* userland messed the mutex */ /* userland messed the mutex */
return (EPERM); return (EPERM);
} }
uq_me = curthread->td_umtxq; uq_me = curthread->td_umtxq;
mtx_lock_spin(&umtx_lock);
pi->pi_owner = NULL; pi->pi_owner = NULL;
TAILQ_REMOVE(&uq_me->uq_pi_contested, pi, pi_link); TAILQ_REMOVE(&uq_me->uq_pi_contested, pi, pi_link);
/* get highest priority thread which is still sleeping. */
uq_first = TAILQ_FIRST(&pi->pi_blocked); uq_first = TAILQ_FIRST(&pi->pi_blocked);
while (uq_first != NULL &&
(uq_first->uq_flags & UQF_UMTXQ) == 0) {
uq_first = TAILQ_NEXT(uq_first, uq_lockq);
}
pri = PRI_MAX; pri = PRI_MAX;
TAILQ_FOREACH(pi2, &uq_me->uq_pi_contested, pi_link) { TAILQ_FOREACH(pi2, &uq_me->uq_pi_contested, pi_link) {
uq_first2 = TAILQ_FIRST(&pi2->pi_blocked); uq_first2 = TAILQ_FIRST(&pi2->pi_blocked);
@ -1875,6 +1877,8 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
sched_unlend_user_prio(curthread, pri); sched_unlend_user_prio(curthread, pri);
thread_unlock(curthread); thread_unlock(curthread);
mtx_unlock_spin(&umtx_lock); mtx_unlock_spin(&umtx_lock);
if (uq_first)
umtxq_signal_thread(uq_first);
} }
umtxq_unlock(&key); umtxq_unlock(&key);
@ -1887,8 +1891,6 @@ do_unlock_pi(struct thread *td, struct umutex *m, uint32_t flags)
count <= 1 ? UMUTEX_UNOWNED : UMUTEX_CONTESTED); count <= 1 ? UMUTEX_UNOWNED : UMUTEX_CONTESTED);
umtxq_lock(&key); umtxq_lock(&key);
if (uq_first != NULL)
umtxq_signal_thread(uq_first);
umtxq_unbusy(&key); umtxq_unbusy(&key);
umtxq_unlock(&key); umtxq_unlock(&key);
umtx_key_release(&key); umtx_key_release(&key);