Re: [RFC PATCH v2 3/6] timekeeping: Fix cross-timestamp interpolation for non-x86

[Peter Hilber <peter.hilber@opensynergy.com>]

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/