Messages in this thread | | | Date | Fri, 15 Sep 2023 19:30:16 +0200 | From | Peter Hilber <> | Subject | Re: [RFC PATCH v2 2/6] timekeeping: Fix cross-timestamp interpolation corner case decision |
| |
On 15.09.23 18:10, Thomas Gleixner wrote: > On Fri, Aug 18 2023 at 03:20, Peter Hilber wrote: >> --- a/kernel/time/timekeeping.c >> +++ b/kernel/time/timekeeping.c >> @@ -1247,7 +1247,8 @@ int get_device_system_crosststamp(int (*get_time_fn) >> */ >> now = tk_clock_read(&tk->tkr_mono); >> interval_start = tk->tkr_mono.cycle_last; >> - if (!cycle_between(interval_start, cycles, now)) { >> + if (!cycle_between(interval_start, cycles, now) && >> + cycles != interval_start) { >> clock_was_set_seq = tk->clock_was_set_seq; >> cs_was_changed_seq = tk->cs_was_changed_seq; >> cycles = interval_start; > > So the explanation in the changelog makes some sense, but this code > without any further explanation just makes my brain explode. > > This whole thing screams for a change to cycle_between() so it becomes: > > timestamp_in_interval(start, end, ts) > > and make start inclusive and not exclusive, no?
I tried like this in v1 (having 'end' inclusive as well), but didn't like the effect at the second usage site.
> > That's actually correct for both usage sites because for interpolation > the logic is the same. history_begin->cycles is a valid timestamp, no?
AFAIU, with the timestamp_in_interval() change, history_begin->cycles would become a valid timestamp. To me it looks like adjust_historical_crosststamp() should then work unmodified for now. But one would have to be careful with the additional corner case in the future.
So, document the current one-line change, or switch to timestamp_in_interval()?
Thanks for the review!
Peter
| |