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

[Juli Mallett]

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.

Thanks,
Juli.