svn commit: r218195 - in head/sys: amd64/amd64 arm/arm i386/i386
ia64/ia64 kern mips/mips powerpc/powerpc sparc64/sparc64 sun4v/sun4v sys
brde at optusnet.com.au
Thu Feb 3 13:11:09 UTC 2011
On Wed, 2 Feb 2011, 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
>> 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.
Grr, hard \xa0.
> 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 sent a long private mail saying how it is quite broken. The hogticks
hasn't worked for 10.4 years, since switchtime is reset on every context
switch and there are lots of context switches for ithreads). Thus
should_yield() almost always returns false, and the old code that used
its logic was mostly dead. The old code that didn't use its logic
worked, but now mostly doesn't. Also, preemption makes the non-preemptive
yielding mostly unnecessary. When preemption occurs, it doesn't do
the right thing in cases where it is necessary to drop all locks, while
manual yielding does. But preemption certainly resets switchtime, so
if is happening then sched_yield() returns false always instead of
only almost always. (Almost always is when the context switch rate
is less than 5 Hz. Non-fast interrups at more than 5 Hz normally
> 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
I noted many namespace problems too, starting with my static uio_yield()
already being exported for abuse by non-uio things.
> 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.
But this has nothing to do with uio. The uio case has been most
perfectly broken for 10.4 years, since the uio_yield() calls for actual
uio things are the main places that use the should_yield() condition.
Non-uio places mostly did an unconditional uio_yield(), and this worked.
I think uio also doesn't hold any locks, for the same reason that
copyin()/out() don't (it might fault and have to sleep for a long time).
Thus preemption works right for the uio case, and its perfect brokenness
10.4 years ago turned into mostly just dead code when the kernel became
The breakage is perfect in bdeBSD, since it uses ithreads for clock
interrupts. Thus switchticks is guaranteed to be reset by the same
interrupts that advance `ticks', and so (ticks - switchticks) is
either 0 or 1 (mostly 1?).
More information about the svn-src-all