Messages in this thread Patch in this message | | | From | Waiman Long <> | Subject | [PATCH] clocksource: Make clocksource watchdog check with WATCHDOG_INTERVAL period | Date | Fri, 27 May 2022 21:57:14 -0400 |
| |
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@phoronix.com> Signed-off-by: Waiman Long <longman@redhat.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
| |