On Sun, Nov 29, 2015 at 5:30 PM, David Gibson <[email protected]> wrote: > 1e75fa8 "time: Condense timekeeper.xtime into xtime_sec" replaced a call to > clocksource_cyc2ns() from timekeeping_get_ns() with an open-coded version > of the same logic to avoid keeping a semi-redundant struct timespec > in struct timekeeper. > > However, the commit also introduced a subtle semantic change - where > clocksource_cyc2ns() uses purely unsigned math, the new version introduces > a signed temporary, meaning that if (delta * tk->mult) has a 63-bit > overflow the following shift will still give a negative result. The > choice of 'maxsec' in __clocksource_updatefreq_scale() means this will > generally happen if there's a ~10 minute pause in examining the > clocksource. > > This can be triggered on a powerpc KVM guest by stopping it from qemu for > a bit over 10 minutes. After resuming time has jumped backwards several > minutes causing numerous problems (jiffies does not advance, msleep()s can > be extended by minutes..). It doesn't happen on x86 KVM guests, because > the guest TSC is effectively frozen while the guest is stopped, which is > not the case for the powerpc timebase. > > Obviously an unsigned (64 bit) overflow will only take twice as long as a > signed, 63-bit overflow. I don't know the time code well enough to know > if that will still cause incorrect calculations, or if a 64-bit overflow > is avoided elsewhere. > > Still, an incorrect forwards clock adjustment will cause less trouble than > time going backwards. So, this patch removes the potential for > intermediate signed overflow. > > Suggested-by: Laurent Vivier <[email protected]> > Signed-off-by: David Gibson <[email protected]>
Thanks for sending this in. I've queued it for 4.5 thanks -john -- 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/
