From owner-svn-src-head@FreeBSD.ORG Fri May 15 15:34:02 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 237AEC3D; Fri, 15 May 2015 15:34:02 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id F26C41B8B; Fri, 15 May 2015 15:34:01 +0000 (UTC) Received: from ralph.baldwin.cx (pool-173-54-116-245.nwrknj.fios.verizon.net [173.54.116.245]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 641CDB95E; Fri, 15 May 2015 11:34:00 -0400 (EDT) From: John Baldwin To: src-committers@freebsd.org Cc: svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r282971 - in head/sys: kern sys Date: Fri, 15 May 2015 09:55:07 -0400 Message-ID: <6021980.e9743X4Cvi@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.1-STABLE; KDE/4.14.3; amd64; ; ) In-Reply-To: <201505151350.t4FDocQT054144@svn.freebsd.org> References: <201505151350.t4FDocQT054144@svn.freebsd.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 15 May 2015 11:34:00 -0400 (EDT) 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 15:34:02 -0000 On Friday, May 15, 2015 01:50:38 PM John Baldwin wrote: > 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 With the additional overhead of the atomic ops it might be worth running some benchmarks to compare this with removing cv_waiters entirely. The theoretical gain from cv_waiters is avoiding looking in the hash table for a matching sleepqueue when there are no waiters. When cv_waiters was a simple integer the cost of having it around was very small, so even a tiny gain was worth having. (It is worth noting that in pre-SMPng code it was fairly common practice to keep a "WANTED" flag around that was set by waiters and would result in wakeup() being skipped if it wasn't set. These flags were maintained by each caller, not centrally. cv_waiters makes this sort of thing centrally maintained rather than something that each caller has to do.) Now that cv_waiters is updated with atomics, the cost is not quite as small. -- John Baldwin