svn commit: r218195 - in head/sys: amd64/amd64 arm/arm i386/i386
ia64/ia64 kern mips/mips powerpc/powerpc sparc64/sparc64 sun4v/sun4v sys
mdf at FreeBSD.org
mdf at FreeBSD.org
Thu Feb 3 17:08:29 UTC 2011
On Thu, Feb 3, 2011 at 4:50 AM, John Baldwin <jhb at freebsd.org> wrote:
> 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).
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.
More information about the svn-src-all