Re: [PATCH] clockevents: Sanitize ticks to nsec conversion

From: Thomas Gleixner
Date: Wed Sep 18 2013 - 05:38:17 EST


On Wed, 18 Sep 2013, Uwe Kleine-König wrote:
> > Now we can easily verify whether the whole equation fits into the
> > 64bit boundary. Shifting the "clc" result back by evt->shift MUST
> > result in "latch". If that's not the case, we have a clear indicator
> But this is only the case if evt->mult is <= (1 << evt->shift). Is this
> always given?

Crap, no. It's only true for device frequency <= 1GHz. Good catch!

> Is it more sensible to adjust dev->max_delta_ns once at register time
> and so save the often recurrent overflow check in
> clockevents_program_event?

Which overflow check are you talking about?

There is only the boundary check:

delta = min(delta, (int64_t) dev->max_delta_ns);
delta = max(delta, (int64_t) dev->min_delta_ns);

Which sensible adjustment at register time is going to remove that?

> Another doubt I have is: You changed clockevent_delta2ns to round up now
> unconditionally. For the numbers on at91 that doesn't matter, but I
> wonder if there are situations that make the timer core violate the
> max_delta_ticks condition now.

And how so? The + (mult - 1) ensures, that the conversion back to
ticks results in the same value as latch. So how should it violate
the max boundary?

Math is hard, right?

> > for boundary violation and can limit "clc" to (1 << 63) - 1 before the
> Where does this magic constant come from?

Rolling my magic hex dice gave me that.

Thanks,

tglx