Messages in this thread | | | Date | Thu, 27 Jul 2023 12:21:56 +0200 | From | Peter Hilber <> | Subject | Re: [RFC PATCH 3/7] timekeeping: Fix cross-timestamp interpolation for non-x86 |
| |
On 08.07.23 01:31, John Stultz wrote: > On Fri, Jun 30, 2023 at 10:12 AM 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 always setting the delta to 0 when interpolating. >> >> Fixes: 2c756feb18d9 ("time: Add history to cross timestamp interface supporting slower devices") >> Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com> >> --- >> kernel/time/timekeeping.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c >> index 7e86d5cd784d..7ccc2377c319 100644 >> --- a/kernel/time/timekeeping.c >> +++ b/kernel/time/timekeeping.c >> @@ -1259,10 +1259,15 @@ int get_device_system_crosststamp(int (*get_time_fn) >> tk_core.timekeeper.offs_real); >> base_raw = tk->tkr_raw.base; >> >> - nsec_real = timekeeping_cycles_to_ns(&tk->tkr_mono, >> - system_counterval.cycles); >> - nsec_raw = timekeeping_cycles_to_ns(&tk->tkr_raw, >> - system_counterval.cycles); >> + if (do_interp) { >> + nsec_real = timekeeping_delta_to_ns(&tk->tkr_mono, 0); >> + nsec_raw = timekeeping_delta_to_ns(&tk->tkr_raw, 0); >> + } else { >> + nsec_real = timekeeping_cycles_to_ns( >> + &tk->tkr_mono, system_counterval.cycles); >> + nsec_raw = timekeeping_cycles_to_ns( >> + &tk->tkr_raw, system_counterval.cycles); >> + } > > Rather than adding another conditional branch here to go through, why > not just use "cycles" instead of system_counterval.cycles as it seems > to be set properly already?
OK. Thanks for the review and suggestion!
| |