From owner-svn-src-all@FreeBSD.ORG Thu Feb 3 13:57:03 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B14C106564A; Thu, 3 Feb 2011 13:57:03 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 56E758FC12; Thu, 3 Feb 2011 13:57:03 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 0132946B53; Thu, 3 Feb 2011 08:57:03 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 124928A027; Thu, 3 Feb 2011 08:57:02 -0500 (EST) From: John Baldwin To: Juli Mallett Date: Thu, 3 Feb 2011 07:50:06 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201102021635.p12GZA94015170@svn.freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201102030750.07076.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Thu, 03 Feb 2011 08:57:02 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=0.5 required=4.2 tests=BAYES_00,MAY_BE_FORGED, RDNS_DYNAMIC autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: svn-src-head@freebsd.org, Matthew D Fleming , svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r218195 - in head/sys: amd64/amd64 arm/arm i386/i386 ia64/ia64 kern mips/mips powerpc/powerpc sparc64/sparc64 sun4v/sun4v sys ufs/ffs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Feb 2011 13:57:03 -0000 On Thursday, February 03, 2011 2:47:20 am Juli Mallett wrote: > On Wed, Feb 2, 2011 at 08:35, Matthew D Fleming wrote: > > Author: mdf > > Date: Wed Feb 2 16:35:10 2011 > > New Revision: 218195 > > URL: http://svn.freebsd.org/changeset/base/218195 > > > > Log: > > Put the general logic for being a CPU hog into a new function > > should_yield(). Use this in various places. Encapsulate the common > > case of check-and-yield into a new function maybe_yield(). > > > > Change several checks for a magic number of iterations to use > > should_yield() instead. > > First off, I admittedly don't know or care very much about this area, > but this commit stood out to me and I had a few minor concerns. > > I'm slightly uncomfortable with the flat namespace here. It isn't > obvious from the names that maybe_yield() and should_yield() relate > only to uio_yield() and not other types of yielding (from DELAY() to > cpu_idle() to sched_yield().) The other problematic element here is > that "maybe_yield" and "should_yield" could quite reasonably be > variables or functions in existing code in the kernel, and although we > don't try to protect against changes that could cause such collisions, > we shouldn't do them gratuitously, and there's even something that > seems aesthetically off about these; they seem...informal, even > Linuxy. I think names like uio_should_yield() and uio_maybe_yield() > wouldn't have nearly as much of a problem, since the context of the > question of "should" is isolated to uio operations rather than, say, > whether the scheduler would *like* for us, as the running thread, to > yield, or other considerations that may be more general. I mostly agree, but these checks are no longer specific to uio. Matt used them to replace many ad-hoc checks using counters with hardcoded maximums in places like softupdates, etc. I don't have any good suggestions for what else you would call these. I'm not sure 'sched_amcpuhog() or sched_hoggingcpu()' are really better (and these are not scheduler dependent, so sched_ would probably not be a good prefix). -- John Baldwin