Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 02 Jun 2012 17:56:08 +0300
From:      Alexander Motin <mav@FreeBSD.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Davide Italiano <davide@FreeBSD.org>, svn-src-projects@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r236449 - projects/calloutng/sys/kern
Message-ID:  <4FCA2988.8090106@FreeBSD.org>
In-Reply-To: <20120602233307.S1957@besplex.bde.org>
References:  <201206021304.q52D4p2X090537@svn.freebsd.org> <20120602233307.S1957@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 06/02/12 17:16, Bruce Evans wrote:
> On Sat, 2 Jun 2012, Davide Italiano wrote:
>
>> Log:
>> Replace binuptime() with getbinuptime() because it's suitable for the
>> purpose and it's cheaper. Update the relative comment on precision error
>> during callwheel scan as well.
>
> This makes even less sense than converting to bintimes at all.
> getbinuptime() can give an even lower precision than the current timer
> ticks, since it is only updated every tc_tick/hz seconds while ticks
> are updated every 1/hz seconds. Also, you have to keep timeouts firing
> for getbintime() to work. OTOH, bintime() is unusable in timeout code,
> since it may be too inefficient, depending on the timecounter hardware.
> cpu_ticks() might be usable. It doesn't use bintimes and isn't very
> accurate, but these are other bugs which become features for use here.

You are right. It was misunderstanding and we have already revealed it.

>> Modified: projects/calloutng/sys/kern/kern_timeout.c
>> ==============================================================================
>>
>> --- projects/calloutng/sys/kern/kern_timeout.c Sat Jun 2 12:26:14 2012
>> (r236448)
>> +++ projects/calloutng/sys/kern/kern_timeout.c Sat Jun 2 13:04:50 2012
>> (r236449)
>> @@ -373,9 +373,9 @@ callout_tick(void)
>> need_softclock = 0;
>> cc = CC_SELF();
>> mtx_lock_spin_flags(&cc->cc_lock, MTX_QUIET);
>> - binuptime(&now);
>> + getbinuptime(&now);
>> /*
>> - * Get binuptime() may be inaccurate and return time up to 1/HZ in
>> the past.
>> + * getbinuptime() may be inaccurate and return time up to 1/HZ in the
>> past.
>> * In order to avoid the possible loss of one or more events look back
>> 1/HZ
>> * in the past from the time we last checked.
>> */
>
> Up to tc_tick/hz, not up to 1/HZ. tc_tick is the read-only sysctl
> variable kern.timecounter.tick that is set to make tc_tick/hz as close
> to 1 msec as possible. If someone asks for busy-waiting by setting
> HZ to much larger than 1000 and uses this to generate lots of timeouts,
> they probably get this now, but get*time() cannot be used even to
> distingish times differing by the timeout granularity. It is hard to
> see how it could ever work for the above use (timout scheduling with
> a granularity of ~1/hz when you can only see differences of ~tc_tick/hz,
> with tc_tick quite often 4-10, or 100-1000 to use or test corner
> cases??). With a tickless kernel, timeouts wouldn't have a fixed
> granularity, but you need accurate measurements of times even more.
> One slow way to get them is to call binuptime() again in the above.
> Another, even worse way to update timecounters after every timeout
> expires (the update has a much higher overhead than binuptime(), so
> this will be very slow iff timeouts that expire are actually used).

I agree with the first part, but could you tell more about tc_windup() 
complexity? There are a lot of time passed since that code was written, 
CPUs got faster and I have feeling that cost of that math could reduce 
and may be not so significant now.

May be at least tc_windup() could be refactored to separate time 
updating (making it's cost closet to single binuptime() call) and all 
other fancy (complicated) things? New eventtimers(4) subsystem uses 
binuptime() a lot. As soon as we already reading relatively slow 
timecounter hardware, it would be nice to get some benefit from it.

-- 
Alexander Motin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FCA2988.8090106>