Messages in this thread | | | From | John Stultz <> | Date | Fri, 7 Jul 2023 16:31:52 -0700 | Subject | Re: [RFC PATCH 3/7] timekeeping: Fix cross-timestamp interpolation for non-x86 |
| |
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?
thanks -john
| |