Re: [PATCH v3 4/5] clocksource: Enable clocksource_cyc2ns() to coverbig cycles

[Thomas Gleixner]

On Wed, 6 Mar 2013, Feng Tang wrote:

> Current clocksource_cyc2ns() has a implicit limit that the (cycles * mult)
> can not exceed 64 bits limit. Jason Gunthorpe proposed a way to
> handle this big cycles case, and this patch put the handling into
> clocksource_cyc2ns() so that it could be used unconditionally.

Could be used if it wouldn't break the world and some more.

> Suggested-by: Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>
> ---
>  include/linux/clocksource.h |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index aa7032c..1ecc872 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -274,7 +274,16 @@ static inline u32 clocksource_hz2mult(u32 hz, u32 shift_constant)
>   */
>  static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
>  {
> -	return ((u64) cycles * mult) >> shift;
> +	u64 max = ULLONG_MAX / mult;

This breaks everything which does not have a 64/32bit divide
instruction. And you can't replace it with do_div() as that would
impose massive overhead on those architectures in the fast path.

The max value can be precalculated and stored in the timekeeper
struct. We really do not want expensive calculations in the fast path.

> +	s64 nsec = 0;
> +
> +	/* The (mult * cycles) may overflow 64 bits, so add a max check */
> +	if (cycles > max) {
> +		nsec = ((max * mult) >> shift) * (cycles / max);

This breaks everything which does not have a 64/64bit divide instruction.
 
> +		cycles %= max;

Ditto.

As this is the slow path on resume you can use the 64bit functions
declared in math64.h. And you want to put the slow path out of line.

There is a world outside of x86!

Thanks,

	tglx
--
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/