lkml.org 
[lkml]   [2022]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] clocksource: Make clocksource watchdog check with WATCHDOG_INTERVAL period
From
On 5/27/22 21:57, Waiman Long wrote:
> 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;
> }
>

Sorry, I accidentally use the old email address for John.

Cheers,
Longman

\
 
 \ /
  Last update: 2022-05-28 04:08    [W:0.065 / U:0.592 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site