On Tue, Mar 04, 2014 at 06:20:09AM +0100, Mike Galbraith wrote:
> Greetings,
>
> While rummaging around looking for HTH a gaggle of weird a$$ machines
> can manage to timewarp back and forth by exactly 208 days, I stumbled
> across $subject which looks like it may want to borrow Salman's fix.
>
> clocksource: avoid unnecessary overflow in cyclecounter_cyc2ns()
>
> As per 4cecf6d401a "sched, x86: Avoid unnecessary overflow in sched_clock",
> cycles * mult >> shift is overflow prone. so give it the same treatment.
>
> Cc: Salman Qazi <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Mike Galbraith <[email protected]>
> ---
> include/linux/clocksource.h | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -77,13 +77,18 @@ struct timecounter {
> *
> * XXX - This could use some mult_lxl_ll() asm optimization. Same code
> * as in cyc2ns, but with unsigned result.
> + *
> + * Because it is the same as x86 __cycles_2_ns, give it the same treatment as
> + * commit 4cecf6d401a "sched, x86: Avoid unnecessary overflow in sched_clock"
> + * to avoid a potential cycles * mult overflow.
Do we normally reference a particular commit in a comment? Why not just
grab the same comment and add a "this is grabbed from arch/x86/... ?
> */
> static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
> cycle_t cycles)
> {
> - u64 ret = (u64)cycles;
> - ret = (ret * cc->mult) >> cc->shift;
> - return ret;
> + u64 quot = (u64)cycles >> cc->shift;
> + u64 rem = (u64)cycles & ((1ULL << cc->shift) - 1);
> +
> + return quot * cc->mult + ((rem * cc->mult) >> cc->shift);
> }
Makes sense to me, for whatever that's worth :)
Also, tile could probably do with a similar approach for ns2cycles (not
that I have observed any problems, but in the sense of being consistent and
all). I'll send a patch in a separate email as not to clutter this thread
too much :)
--
Henrik Austad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
