[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220528015714.109442-1-longman@redhat.com>
Date: Fri, 27 May 2022 21:57:14 -0400
From: Waiman Long <longman@...hat.com>
To: John Stultz <john.stultz@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...nel.org>,
Feng Tang <feng.tang@...el.com>,
"Paul E. McKenney" <paulmck@...nel.org>
Cc: linux-kernel@...r.kernel.org,
Michael Larabel <Michael@...ronix.com>,
Joe Mario <jmario@...hat.com>,
Michey Mehta <mimehta@...hat.com>,
Waiman Long <longman@...hat.com>
Subject: [PATCH] clocksource: Make clocksource watchdog check with WATCHDOG_INTERVAL period
Since commit c86ff8c55b8a ("clocksource: Avoid accidental unstable
marking of clocksource"), a new WD_READ_SKIP value was introduced
as a possible return value of cs_watchdog_read() to skip the current
check. However, this has an undesriable side effect of extending the
time gap between csnow and cs_last to more than one WATCHDOG_INTERVAL
(0.5s) in case of intermittent WD_READ_SKIP's.
There was an instance of reported clocksource watchdog failure with
the time skew of 485us where the uncertainly threshold is 400us. In
that particular case, the (now - last) gap was about 2s. Looking at
the dmesg log, it was clear there was a successful cs_watchdog_read()
followed by 3 skips and then another successful cs_watchdog_read().
If there is an existing skew between the hpet (watchdog) and tsc
clocksource, enlarging the period by 4x will certainly increase the
measured skew causing it to exceed the threshold in this case. Fix
this variable period problem by resetting the CLOCK_SOURCE_WATCHDOG bit
after each WD_READ_SKIP to force the reloading of wd_last and cs_last
in the next round. This ensures that we have two consecutive successful
cs_watchdog_read()'s before checking the clock skew.
Fixes: c86ff8c55b8a ("clocksource: Avoid accidental unstable marking of clocksource")
Reported-by: Michael Larabel <Michael@...ronix.com>
Signed-off-by: Waiman Long <longman@...hat.com>
---
kernel/time/clocksource.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index cee5da1e54c4..173e052c12b6 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -411,9 +411,18 @@ static void clocksource_watchdog(struct timer_list *unused)
read_ret = cs_watchdog_read(cs, &csnow, &wdnow);
if (read_ret != WD_READ_SUCCESS) {
- if (read_ret == WD_READ_UNSTABLE)
+ if (read_ret == WD_READ_UNSTABLE) {
/* Clock readout unreliable, so give it up. */
__clocksource_unstable(cs);
+ } else { /* WD_READ_SKIP */
+ /*
+ * Watchdog clock unstable at the moment,
+ * discard the stored wd_last and cs_last to
+ * make sure the gap between now and last
+ * is always one WATCHDOG_INTERVAL.
+ */
+ cs->flags &= ~CLOCK_SOURCE_WATCHDOG;
+ }
continue;
}
--
2.31.1
Powered by blists - more mailing lists