From owner-svn-src-head@FreeBSD.ORG Thu Feb 3 17:08:29 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 132D0106564A; Thu, 3 Feb 2011 17:08:29 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id EF68D8FC12; Thu, 3 Feb 2011 17:08:27 +0000 (UTC) Received: by wyf19 with SMTP id 19so1373133wyf.13 for ; Thu, 03 Feb 2011 09:08:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=CWubPBgVPqbwer0xUfL8VVSarR17677TeDgcT6yUAyo=; b=vPytzZ7/MK+RNagc9yO8EKtpJfoyHV1K2m9QteCbU1WuJ/fQUOVkCBwK2JajXeFFII V9Zb8MzR7bQf06Xvbk4pHy0UEl5UYyzca3x1YjO4/X31ySirPcYtMEY2uW0U/fspd6MB T9ivXn3lGRKDt3CPQ4pIP6qT77gpk/jWkJeeU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=DSJsLgmG5LQP+sLH3SFiQLYeXQ3+yfHfI+xReclAZFkzDbH/xGgrrRLl7yklXU67u7 O9d8cQarUSy18N8SB4iDp+Q06vhpcSIWRQpkIatMC0Zhi6uQD9NBVE0DNcDfVrPJcTZp pTzXm75vr19Wtjx0IcJKhd8QHR8vhPWIEIR+U= MIME-Version: 1.0 Received: by 10.216.179.140 with SMTP id h12mr10069891wem.40.1296752900170; Thu, 03 Feb 2011 09:08:20 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.216.62.203 with HTTP; Thu, 3 Feb 2011 09:08:20 -0800 (PST) In-Reply-To: <201102030750.07076.jhb@freebsd.org> References: <201102021635.p12GZA94015170@svn.freebsd.org> <201102030750.07076.jhb@freebsd.org> Date: Thu, 3 Feb 2011 09:08:20 -0800 X-Google-Sender-Auth: E-s5PMGM2Wb-wuguUpOQVtKiI1o Message-ID: From: mdf@FreeBSD.org To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Juli Mallett , 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 17:08:29 -0000 On Thu, Feb 3, 2011 at 4:50 AM, John Baldwin wrote: > 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 =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 c= ommon >> > =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. =A0It 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().) =A0The 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. =A0I 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. =A0Matt u= sed > 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. =A0= I'm not > sure 'sched_amcpuhog() or sched_hoggingcpu()' are really better (and thes= e are > not scheduler dependent, so sched_ would probably not be a good prefix). Bruce correctly points out that the code doesn't work like I expect with PREEMPTION, which most people will be running. I'm thinking of adding a new per-thread field to record the last ticks value that a voluntary mi_switch() was done, so that there's a standard way of checking if a thread is being a hog; this will work for both PREEMPTION and !PREEMPTION, and would be appropriate for the places that previously used a counter. (This would require uio_yield() to be SW_VOL, but I can't see why it's not a voluntary context switch anyways). I'm happy to rename the functions (perhaps just yield_foo() rather than foo_yield()?) and stop using uio_yield as the base name since it's not a uio function. I wanted to keep the uio_yield symbol to preserve the KBI/KPI. Any suggestions for names are welcome. Thanks, matthew