Re: [MIPS]clocks_calc_mult_shift() may gen a too big mult value

[John Stultz]

On Mon, 2011-10-31 at 11:30 -0700, David Daney wrote:
> On 10/31/2011 11:12 AM, John Stultz wrote:
> > +	/*
> > +	 * Since mult may be adjusted by ntp, add an extra saftey margin
> > +	 * for clocksources that have large mults, to avoid overflow.
> > +	 *
> > +	 * Assume we won't try to correct for more then 5% adjustments
> 
> Can we do any better than making assumptions about this?
> 
> The current assumption appears to be that only very small adjustments 
> will be made, and that didn't workout so well.

s/only very small/no/

The calc_mult_shift function doesn't take any adjustments into account.

> Is it possible to put hard constraints on these things, so that it will 
> always work?

Fair enough. The patch was a bit off the cuff, and you're right that the
assumption is broken: ntp limits the freq adjustment to 500ppm, but the
kernel limits tick adjustments to 10%. Thus 11% adjustments (easier to
remember then 10.05%) would be the hard limit of adjustments from
external interfaces. So I'll need to rework the patch to adapt for that.

The harder part, once we put a hard constraint on the adjustment, is to
enforce that the timekeeping_bigadjust() doesn't push it beyond that,
since its logic uses relative adjustments and doesn't consider the
original mult value. Further, since its fairly opaque code, proving the
constraint itself won't break in edge cases is also needed, and will
take some time.

So yea, you're point is fair. Its just going to take a bit of thoughtful
review before implementing such a hard constraint universally.

So making a good adversary constraint first (11%), and then iteratively
hardening the code it impacts might be a good approach to get there.

thanks
-john


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/