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

Previously, cv_waiters was only updated by cv_signal or cv_wait. If a

Browse Source 
thread awakened due to a time out, then cv_waiters was not decremented.
If INT_MAX threads timed out on a cv without an intervening cv_broadcast,
then cv_waiters could overflow. To fix this, have each sleeping thread
decrement cv_waiters when it resumes.

Note that previously cv_waiters was protected by the sleepq chain lock.
However, that lock is not held when threads resume from sleep. In
addition, the interlock is also not always reacquired after resuming
(cv_wait_unlock), nor is it always held by callers of cv_signal() or
cv_broadcast(). Instead, use atomic ops to update cv_waiters. Since
the sleepq chain lock is still held on every increment, it should
still be safe to compare cv_waiters against zero while holding the
lock in the wakeup routines as the only way the race should be lost
would result in extra calls to sleepq_signal() or sleepq_broadcast().

Differential Revision:	https://reviews.freebsd.org/D2427
Reviewed by:	benno
Reported by:	benno (wrap of cv_waiters in the field)
MFC after:	2 weeks
This commit is contained in:
John Baldwin 2015-05-15 13:50:37 +00:00
parent 5b157f2144
commit 5c894ee2ef
2 changed files with 13 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
sys/kern/kern_condvar.c
View File

@ -122,7 +122,7 @@ _cv_wait(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
atomic_add_int(&cvp->cv_waiters, 1);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -137,6 +137,7 @@ _cv_wait(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
}
sleepq_wait(cvp, 0);
atomic_subtract_int(&cvp->cv_waiters, 1);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -184,7 +185,7 @@ _cv_wait_unlock(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
atomic_add_int(&cvp->cv_waiters, 1);
DROP_GIANT();
sleepq_add(cvp, lock, cvp->cv_description, SLEEPQ_CONDVAR, 0);
@ -194,6 +195,7 @@ _cv_wait_unlock(struct cv *cvp, struct lock_object *lock)
if (class->lc_flags & LC_SLEEPABLE)
sleepq_lock(cvp);
sleepq_wait(cvp, 0);
atomic_subtract_int(&cvp->cv_waiters, 1);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -240,7 +242,7 @@ _cv_wait_sig(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
cvp->cv_waiters++;
atomic_add_int(&cvp->cv_waiters, 1);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -256,6 +258,7 @@ _cv_wait_sig(struct cv *cvp, struct lock_object *lock)
sleepq_lock(cvp);
}
rval = sleepq_wait_sig(cvp, 0);
atomic_subtract_int(&cvp->cv_waiters, 1);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -307,7 +310,7 @@ _cv_timedwait_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt,
sleepq_lock(cvp);
cvp->cv_waiters++;
atomic_add_int(&cvp->cv_waiters, 1);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -323,6 +326,7 @@ _cv_timedwait_sbt(struct cv *cvp, struct lock_object *lock, sbintime_t sbt,
sleepq_lock(cvp);
}
rval = sleepq_timedwait(cvp, 0);
atomic_subtract_int(&cvp->cv_waiters, 1);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -376,7 +380,7 @@ _cv_timedwait_sig_sbt(struct cv *cvp, struct lock_object *lock,
sleepq_lock(cvp);
cvp->cv_waiters++;
atomic_add_int(&cvp->cv_waiters, 1);
if (lock == &Giant.lock_object)
mtx_assert(&Giant, MA_OWNED);
DROP_GIANT();
@ -393,6 +397,7 @@ _cv_timedwait_sig_sbt(struct cv *cvp, struct lock_object *lock,
sleepq_lock(cvp);
}
rval = sleepq_timedwait_sig(cvp, 0);
atomic_subtract_int(&cvp->cv_waiters, 1);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -421,10 +426,8 @@ cv_signal(struct cv *cvp)
wakeup_swapper = 0;
sleepq_lock(cvp);
if (cvp->cv_waiters > 0) {
cvp->cv_waiters--;
if (cvp->cv_waiters > 0)
wakeup_swapper = sleepq_signal(cvp, SLEEPQ_CONDVAR, 0, 0);
}
sleepq_release(cvp);
if (wakeup_swapper)
kick_proc0();
@ -447,10 +450,8 @@ cv_broadcastpri(struct cv *cvp, int pri)
if (pri == -1)
pri = 0;
sleepq_lock(cvp);
if (cvp->cv_waiters > 0) {
cvp->cv_waiters = 0;
if (cvp->cv_waiters > 0)
wakeup_swapper = sleepq_broadcast(cvp, SLEEPQ_CONDVAR, pri, 0);
}
sleepq_release(cvp);
if (wakeup_swapper)
kick_proc0();

2
sys/sys/condvar.h
View File

@ -45,7 +45,7 @@ TAILQ_HEAD(cv_waitq, thread);
*/
struct cv {
const char *cv_description;
int cv_waiters;
volatile int cv_waiters;
};
#ifdef _KERNEL