svn commit: r236449 - projects/calloutng/sys/kern

[Alexander Motin]

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