linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "tip-bot2 for Peter Hilber" <tip-bot2@linutronix.de>
To: linux-tip-commits@vger.kernel.org
Cc: Peter Hilber <peter.hilber@opensynergy.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: [tip: timers/core] timekeeping: Fix cross-timestamp interpolation corner case decision
Date: Mon, 19 Feb 2024 16:35:16 -0000	[thread overview]
Message-ID: <170836051645.398.6755085884635470259.tip-bot2@tip-bot2> (raw)
In-Reply-To: <20231218073849.35294-3-peter.hilber@opensynergy.com>

The following commit has been merged into the timers/core branch of tip:

Commit-ID:     87a41130881995f82f7adbafbfeddaebfb35f0ef
Gitweb:        https://git.kernel.org/tip/87a41130881995f82f7adbafbfeddaebfb35f0ef
Author:        Peter Hilber <peter.hilber@opensynergy.com>
AuthorDate:    Mon, 18 Dec 2023 08:38:40 +01:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Mon, 19 Feb 2024 12:18:51 +01:00

timekeeping: Fix cross-timestamp interpolation corner case decision

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>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/r/20231218073849.35294-3-peter.hilber@opensynergy.com

---
 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 8f35455..4e9f2f8 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;
 }
@@ -1246,7 +1248,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;
@@ -1277,13 +1279,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;

  reply	other threads:[~2024-02-19 16:35 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-18  7:38 [RFC PATCH v3 0/7] Add virtio_rtc module and related changes Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 1/7] timekeeping: Fix cross-timestamp interpolation on counter wrap Peter Hilber
2024-02-19 16:35   ` [tip: timers/core] " tip-bot2 for Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 2/7] timekeeping: Fix cross-timestamp interpolation corner case decision Peter Hilber
2024-02-19 16:35   ` tip-bot2 for Peter Hilber [this message]
2023-12-18  7:38 ` [RFC PATCH v3 3/7] timekeeping: Fix cross-timestamp interpolation for non-x86 Peter Hilber
2024-02-19 16:35   ` [tip: timers/core] " tip-bot2 for Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 4/7] virtio_rtc: Add module and driver core Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 5/7] virtio_rtc: Add PTP clocks Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 6/7] virtio_rtc: Add Arm Generic Timer cross-timestamping Peter Hilber
2023-12-18  7:38 ` [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver Peter Hilber
2024-03-08 17:03   ` Alexandre Belloni
2024-03-11 18:28     ` Peter Hilber
2024-03-11 19:46       ` Alexandre Belloni
2024-03-13  9:13         ` Peter Hilber
2024-03-07 14:02 ` [RFC PATCH v3 0/7] Add virtio_rtc module and related changes David Woodhouse
2024-03-08 10:32   ` Peter Hilber
2024-03-08 12:33     ` David Woodhouse
2024-03-11 18:24       ` Peter Hilber
2024-03-12 17:15         ` David Woodhouse
2024-03-13  9:45           ` Peter Hilber
2024-03-13 11:18             ` Alexandre Belloni
2024-03-13 12:29               ` David Woodhouse
2024-03-13 12:58                 ` Alexandre Belloni
2024-03-13 14:06                   ` David Woodhouse
2024-03-13 14:50                     ` Alexandre Belloni
2024-03-13 20:12                       ` Andrew Lunn
2024-03-14  9:13                         ` Peter Hilber
2024-03-13 17:50                     ` Peter Hilber
2024-03-13 14:15               ` Peter Hilber
2024-03-13 12:45             ` David Woodhouse
2024-03-13 17:50               ` Peter Hilber
2024-03-13 18:18                 ` David Woodhouse
2024-03-14 10:13                   ` Peter Hilber
2024-03-14 14:19                     ` David Woodhouse
2024-03-19 13:47                       ` Peter Hilber
2024-03-20 17:22                         ` David Woodhouse

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=170836051645.398.6755085884635470259.tip-bot2@tip-bot2 \
    --to=tip-bot2@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=peter.hilber@opensynergy.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).