From owner-svn-src-head@FreeBSD.ORG Thu Feb 3 07:47:41 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D29271065673; Thu, 3 Feb 2011 07:47:41 +0000 (UTC) (envelope-from juli@clockworksquid.com) Received: from mail-yi0-f54.google.com (mail-yi0-f54.google.com [209.85.218.54]) by mx1.freebsd.org (Postfix) with ESMTP id 5B3218FC15; Thu, 3 Feb 2011 07:47:41 +0000 (UTC) Received: by yie19 with SMTP id 19so378660yie.13 for ; Wed, 02 Feb 2011 23:47:40 -0800 (PST) Received: by 10.150.95.9 with SMTP id s9mr3990933ybb.26.1296719260556; Wed, 02 Feb 2011 23:47:40 -0800 (PST) MIME-Version: 1.0 Sender: juli@clockworksquid.com Received: by 10.150.196.12 with HTTP; Wed, 2 Feb 2011 23:47:20 -0800 (PST) In-Reply-To: <201102021635.p12GZA94015170@svn.freebsd.org> References: <201102021635.p12GZA94015170@svn.freebsd.org> From: Juli Mallett Date: Wed, 2 Feb 2011 23:47:20 -0800 X-Google-Sender-Auth: Lh0Q-c4lBzGJc1Op1CNAgZyCuS8 Message-ID: To: Matthew D Fleming Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, 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-head@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 03 Feb 2011 07:47:41 -0000 On Wed, Feb 2, 2011 at 08:35, Matthew D Fleming wrote: > Author: mdf > Date: Wed Feb =A02 16:35:10 2011 > New Revision: 218195 > URL: http://svn.freebsd.org/changeset/base/218195 > > Log: > =A0Put the general logic for being a CPU hog into a new function > =A0should_yield(). =A0Use this in various places. =A0Encapsulate the comm= on > =A0case of check-and-yield into a new function maybe_yield(). > > =A0Change several checks for a magic number of iterations to use > =A0should_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. Thanks, Juli.