Messages in this thread | | | Date | Wed, 13 Sep 2023 11:11:33 +0200 | From | Peter Hilber <> | Subject | Re: [RFC PATCH v2 3/6] timekeeping: Fix cross-timestamp interpolation for non-x86 |
| |
On Fri, Aug 25, 2023 6:04 John Stultz <jstultz@google.com> wrote: > On Thu, Aug 17, 2023 at 6:20 PM Peter Hilber > <peter.hilber@opensynergy.com> wrote: >> >> So far, get_device_system_crosststamp() unconditionally passes >> system_counterval.cycles to timekeeping_cycles_to_ns(). But when >> interpolating system time (do_interp == true), system_counterval.cycles is >> before tkr_mono.cycle_last, contrary to the timekeeping_cycles_to_ns() >> expectations. >> >> On x86, CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE will mitigate on >> interpolating, setting delta to 0. With delta == 0, xtstamp->sys_monoraw >> and xtstamp->sys_realtime are then set to the last update time, as >> implicitly expected by adjust_historical_crosststamp(). On other >> architectures, the resulting nonsense xtstamp->sys_monoraw and >> xtstamp->sys_realtime corrupt the xtstamp (ts) adjustment in >> adjust_historical_crosststamp(). >> >> Fix this by deriving xtstamp->sys_monoraw and xtstamp->sys_realtime from >> the last update time when interpolating, by using the local variable >> "cycles". The local variable already has the right value when >> interpolating, unlike system_counterval.cycles. >> >> Fixes: 2c756feb18d9 ("time: Add history to cross timestamp interface supporting slower devices") >> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com> > > Thanks again for iterating on this. This looks much better! > > Now, I've never had an environment that used this logic, so I'm > trusting you've tested it well? > > Assuming so: > Acked-by: John Stultz <jstultz@google.com>
Thanks for re-reviewing!
I did automated tests with various chrony [1] configurations. The tests check that all PTP_SYS_OFFSET_PRECISE2 ioctls issued by chrony are successful for a combined test time of many hours, and that the cross-timestamps look plausible.
I will add a description of the relevant tests when changing the series to non-RFC.
[1] https://chrony-project.org/
| |