Re: [PATCH 1/2] Dynamic Tick: Prevent clocksource wrapping during idle

[john stultz <johnstul@us.ibm.com>]

On Wed, 2009-05-27 at 18:01 +0200, Thomas Gleixner wrote:
> On Wed, 27 May 2009, Jon Hunter wrote:
> > + */
> > +s64 timekeeping_max_deferment(void)
> > +{
> > +	s64 max_nsecs;
> > +	u64 max_cycles;
> > +
> > +	/*
> > +	 * Calculate the maximum number of cycles that we can pass to the
> > +	 * cyc2ns function without overflowing a 64-bit signed result. The
> > +	 * maximum number of cycles is equal to ULLONG_MAX/clock->mult which
> > +	 * is equivalent to the below.
> > +	 * max_cycles < (2^63)/clock->mult
> > +	 * max_cycles < 2^(log2((2^63)/clock->mult))
> > +	 * max_cycles < 2^(log2(2^63) - log2(clock->mult))
> > +	 * max_cycles < 2^(63 - log2(clock->mult))
> > +	 * max_cycles < 1 << (63 - log2(clock->mult))
> > +	 * Please note that we add 1 to the result of the log2 to account for
> > +	 * any rounding errors, ensure the above inequality is satisfied and
> > +	 * no overflow will occur.
> > +	 */
> > +	max_cycles = 1ULL << (63 - (ilog2(clock->mult) + 1));
> > +
> > +	/*
> > +	 * The actual maximum number of cycles we can defer the clocksource is
> > +	 * determined by the minimum of max_cycles and clock->mask.
> > +	 */
> > +	max_cycles = min(max_cycles, clock->mask);
> > +	max_nsecs = cyc2ns(clock, max_cycles);
> 
>   Why do you want to recalculate the whole stuff over and over ?
> 
>   That computation can be done when the clock source is initialized or
>   any fundamental change of the clock parameters happens.
> 
>   Stick that value into the clocksource struct and just read it out.

Sigh. 

I was hoping to avoid hanging another bit of junk off of the clocksource
struct. 

But I guess we could compute that value on registration and keep it
around. Changes to mult could effect things, but should be well within
the 6% safety net we give ourselves.


> > +	/*
> > +	 * To ensure that the clocksource does not wrap whilst we are idle,
> > +	 * limit the time the clocksource can be deferred by 6.25%. Please
> > +	 * note a margin of 6.25% is used because this can be computed with
> > +	 * a shift, versus say 5% which would require division.
> > +	 */
> > +	max_nsecs = max_nsecs - (max_nsecs >> 4);
> > +
> > +	if (max_nsecs < 0)
> > +		max_nsecs = 0;
> 
>   How does "max_nsecs = max_nsecs - (max_nsecs >> 4)" ever become
>   negative ?

Fair point. Now we've limited the overflow case, we shouldn't trip
negative values. 

thanks
-john