lkml.org 
[lkml]   [2023]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC PATCH v3 2/7] timekeeping: Fix cross-timestamp interpolation corner case decision
    Date
    The cycle_between() helper checks if parameter test is in the open interval
    (before, after). Colloquially speaking, this also applies to the counter
    wrap-around special case before > after. get_device_system_crosststamp()
    currently uses cycle_between() at the first call site to decide whether to
    interpolate for older counter readings.

    get_device_system_crosststamp() has the following problem with
    cycle_between() testing against an open interval: Assume that, by chance,
    cycles == tk->tkr_mono.cycle_last (in the following, "cycle_last" for
    brevity). Then, cycle_between() at the first call site, with effective
    argument values cycle_between(cycle_last, cycles, now), returns false,
    enabling interpolation. During interpolation,
    get_device_system_crosststamp() will then call cycle_between() at the
    second call site (if a history_begin was supplied). The effective argument
    values are cycle_between(history_begin->cycles, cycles, cycles), since
    system_counterval.cycles == interval_start == cycles, per the assumption.
    Due to the test against the open interval, cycle_between() returns false
    again. This causes get_device_system_crosststamp() to return -EINVAL.

    This failure should be avoided, since get_device_system_crosststamp() works
    both when cycles follows cycle_last (no interpolation), and when cycles
    precedes cycle_last (interpolation). For the case cycles == cycle_last,
    interpolation is actually unneeded.

    Fix this by changing cycle_between() into timestamp_in_interval(), which
    now checks against the closed interval, rather than the open interval.

    This changes the get_device_system_crosststamp() behavior for three corner
    cases:

    1. Bypass interpolation in the case cycles == tk->tkr_mono.cycle_last,
    fixing the problem described above.

    2. At the first timestamp_in_interval() call site, cycles == now no longer
    causes failure.

    3. At the second timestamp_in_interval() call site, history_begin->cycles
    == system_counterval.cycles no longer causes failure.
    adjust_historical_crosststamp() also works for this corner case,
    where partial_history_cycles == total_history_cycles.

    These behavioral changes should not cause any problems.

    Fixes: 2c756feb18d9 ("time: Add history to cross timestamp interface supporting slower devices")
    Signed-off-by: Peter Hilber <peter.hilber@opensynergy.com>
    ---

    Notes:
    v3:

    - switch back to v1 style closed interval test (Thomas Gleixner)
    - document effect of closed interval test on corner cases
    - do not carry "Acked-by: John Stultz <jstultz@google.com>" due to above
    changes

    v2:

    - try to clarify problem description (John Stultz)
    - simplify fix

    kernel/time/timekeeping.c | 18 ++++++++++--------
    1 file changed, 10 insertions(+), 8 deletions(-)

    diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
    index 08a3d0052baa..24ffd681aa23 100644
    --- a/kernel/time/timekeeping.c
    +++ b/kernel/time/timekeeping.c
    @@ -1180,13 +1180,15 @@ static int adjust_historical_crosststamp(struct system_time_snapshot *history,
    }

    /*
    - * cycle_between - true if test occurs chronologically between before and after
    + * timestamp_in_interval - true if ts is chronologically in [start, end]
    + *
    + * True if ts occurs chronologically at or after start, and before or at end.
    */
    -static bool cycle_between(u64 before, u64 test, u64 after)
    +static bool timestamp_in_interval(u64 start, u64 end, u64 ts)
    {
    - if (test > before && test < after)
    + if (ts >= start && ts <= end)
    return true;
    - if (before > after && (test > before || test < after))
    + if (start > end && (ts >= start || ts <= end))
    return true;
    return false;
    }
    @@ -1247,7 +1249,7 @@ 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 (!timestamp_in_interval(interval_start, now, cycles)) {
    clock_was_set_seq = tk->clock_was_set_seq;
    cs_was_changed_seq = tk->cs_was_changed_seq;
    cycles = interval_start;
    @@ -1278,13 +1280,13 @@ int get_device_system_crosststamp(int (*get_time_fn)
    bool discontinuity;

    /*
    - * Check that the counter value occurs after the provided
    + * Check that the counter value is not before the provided
    * history reference and that the history doesn't cross a
    * clocksource change
    */
    if (!history_begin ||
    - !cycle_between(history_begin->cycles,
    - system_counterval.cycles, cycles) ||
    + !timestamp_in_interval(history_begin->cycles,
    + cycles, system_counterval.cycles) ||
    history_begin->cs_was_changed_seq != cs_was_changed_seq)
    return -EINVAL;
    partial_history_cycles = cycles - system_counterval.cycles;
    --
    2.40.1

    \
     
     \ /
      Last update: 2023-12-18 09:21    [W:8.872 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site