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
jhb at freebsd.org
Thu Feb 3 13:57:03 UTC 2011
On Thursday, February 03, 2011 2:47:20 am Juli Mallett wrote:
> On Wed, Feb 2, 2011 at 08:35, Matthew D Fleming <mdf at freebsd.org> 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).
More information about the svn-src-all