From owner-svn-src-head@FreeBSD.ORG Fri May 15 13:50:38 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 99E8350E; Fri, 15 May 2015 13:50:38 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7AA4D1E16; Fri, 15 May 2015 13:50:38 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id t4FDocDU054146; Fri, 15 May 2015 13:50:38 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id t4FDocQT054144; Fri, 15 May 2015 13:50:38 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201505151350.t4FDocQT054144@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Fri, 15 May 2015 13:50:38 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r282971 - in head/sys: kern sys X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 May 2015 13:50:38 -0000 Author: jhb Date: Fri May 15 13:50:37 2015 New Revision: 282971 URL: https://svnweb.freebsd.org/changeset/base/282971 Log: Previously, cv_waiters was only updated by cv_signal or cv_wait. If a 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 Modified: head/sys/kern/kern_condvar.c head/sys/sys/condvar.h Modified: head/sys/kern/kern_condvar.c ============================================================================== --- head/sys/kern/kern_condvar.c Fri May 15 13:36:50 2015 (r282970) +++ head/sys/kern/kern_condvar.c Fri May 15 13:50:37 2015 (r282971) @@ -122,7 +122,7 @@ _cv_wait(struct cv *cvp, struct lock_obj 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_obj 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 l 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 l 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 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 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 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 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, st 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, st 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(); Modified: head/sys/sys/condvar.h ============================================================================== --- head/sys/sys/condvar.h Fri May 15 13:36:50 2015 (r282970) +++ head/sys/sys/condvar.h Fri May 15 13:50:37 2015 (r282971) @@ -45,7 +45,7 @@ TAILQ_HEAD(cv_waitq, thread); */ struct cv { const char *cv_description; - int cv_waiters; + volatile int cv_waiters; }; #ifdef _KERNEL