Re: [patch 5/6] [RFD] timekeeping: Provide optional 128bit math

[Ingo Molnar]

* Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:

> If the timekeeping CPU is scheduled out long enough by a hypervisor the
> clocksource delta multiplication can overflow and as a result time can go
> backwards. That's insane to begin with, but people already triggered a
> signed multiplication overflow, so a unsigned overflow is not necessarily
> impossible.
> 
> Implement optional 128bit math which can be selected by a config option.

What's the rough VM interruption time that would trigger an overflow? Given that 
the clock shift tk_read_base::mult is often 1, isn't it 32-bit nsecs, i.e. 4 
seconds?

That doesn't sound 'insanely long'.

Or some other value?

> +#if defined(CONFIG_ARCH_SUPPORTS_INT128) && defined(__SIZEOF_INT128__)
> +static inline u64 timekeeping_delta_to_ns(struct tk_read_base *tkr, u64 delta)
> +{
> +	unsigned __int128 nsec;
> +
> +	nsec = ((unsigned __int128)delta * tkr->mult) + tkr->xtime_nsec;
> +	return (u64) (nsec >> tkr->shift);
> +}
> +#else
> +static inline u64 timekeeping_delta_to_ns(struct tk_read_base *tkr, u64 delta)
> +{
> +	u32 dh, dl;
> +	u64 nsec;
> +
> +	dl = delta;
> +	dh = delta >> 32;
> +
> +	nsec = ((u64)dl * tkr->mult) + tkr->xtime_nsec;
> +	nsec >>= tkr->shift;
> +	if (unlikely(dh))
> +		nsec += ((u64)dh * tkr->mult) << (32 - tkr->shift);
> +	return nsec;
> +}
> +#endif

Actually, 128-bit multiplication shouldn't be too horrible - at least on 64-bit 
architectures. (128-bit division is another matter, but there's no division here.)

So we might as well use this by default on 64-bit architectures that have 64-bit 
cycle counters - which the vast majority of hypervisors are. Assuming I'm correct 
that just 4 seconds of VM delay would make the whole logic unrobust.

Thanks,

	Ingo