Skip to content

Commit 35e13e9

Browse files
committed
Merge branch 'clocksource' of git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu into timers/core
Pull clocksource watchdog updates from Paul McKenney: - Avoid accidental unstable marking of clocksources by rejecting clocksource measurements where the source of the skew is the delay reading reference clocksource itself. This change avoids many of the current false positives caused by epic cache-thrashing workloads. - Reduce the default clocksource_watchdog() retries to 2, thus offsetting the increased overhead due to #1 above rereading the reference clocksource. Link: https://lore.kernel.org/lkml/20220105001723.GA536708@paulmck-ThinkPad-P17-Gen-1
2 parents 6629c07 + 1a56206 commit 35e13e9

File tree

7 files changed

+49
-12
lines changed

7 files changed

+49
-12
lines changed

Documentation/admin-guide/kernel-parameters.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,8 +603,8 @@
603603
clocksource.max_cswd_read_retries= [KNL]
604604
Number of clocksource_watchdog() retries due to
605605
external delays before the clock will be marked
606-
unstable. Defaults to three retries, that is,
607-
four attempts to read the clock under test.
606+
unstable. Defaults to two retries, that is,
607+
three attempts to read the clock under test.
608608

609609
clocksource.verify_n_cpus= [KNL]
610610
Limit the number of CPUs checked for clocksources

kernel/time/clocksource.c

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ static u64 suspend_start;
107107
* This delay could be due to SMIs, NMIs, or to VCPU preemptions. Used as
108108
* a lower bound for cs->uncertainty_margin values when registering clocks.
109109
*/
110-
#define WATCHDOG_MAX_SKEW (50 * NSEC_PER_USEC)
110+
#define WATCHDOG_MAX_SKEW (100 * NSEC_PER_USEC)
111111

112112
#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
113113
static void clocksource_watchdog_work(struct work_struct *work);
@@ -199,23 +199,30 @@ void clocksource_mark_unstable(struct clocksource *cs)
199199
spin_unlock_irqrestore(&watchdog_lock, flags);
200200
}
201201

202-
ulong max_cswd_read_retries = 3;
202+
ulong max_cswd_read_retries = 2;
203203
module_param(max_cswd_read_retries, ulong, 0644);
204204
EXPORT_SYMBOL_GPL(max_cswd_read_retries);
205205
static int verify_n_cpus = 8;
206206
module_param(verify_n_cpus, int, 0644);
207207

208-
static bool cs_watchdog_read(struct clocksource *cs, u64 *csnow, u64 *wdnow)
208+
enum wd_read_status {
209+
WD_READ_SUCCESS,
210+
WD_READ_UNSTABLE,
211+
WD_READ_SKIP
212+
};
213+
214+
static enum wd_read_status cs_watchdog_read(struct clocksource *cs, u64 *csnow, u64 *wdnow)
209215
{
210216
unsigned int nretries;
211-
u64 wd_end, wd_delta;
212-
int64_t wd_delay;
217+
u64 wd_end, wd_end2, wd_delta;
218+
int64_t wd_delay, wd_seq_delay;
213219

214220
for (nretries = 0; nretries <= max_cswd_read_retries; nretries++) {
215221
local_irq_disable();
216222
*wdnow = watchdog->read(watchdog);
217223
*csnow = cs->read(cs);
218224
wd_end = watchdog->read(watchdog);
225+
wd_end2 = watchdog->read(watchdog);
219226
local_irq_enable();
220227

221228
wd_delta = clocksource_delta(wd_end, *wdnow, watchdog->mask);
@@ -226,13 +233,34 @@ static bool cs_watchdog_read(struct clocksource *cs, u64 *csnow, u64 *wdnow)
226233
pr_warn("timekeeping watchdog on CPU%d: %s retried %d times before success\n",
227234
smp_processor_id(), watchdog->name, nretries);
228235
}
229-
return true;
236+
return WD_READ_SUCCESS;
230237
}
238+
239+
/*
240+
* Now compute delay in consecutive watchdog read to see if
241+
* there is too much external interferences that cause
242+
* significant delay in reading both clocksource and watchdog.
243+
*
244+
* If consecutive WD read-back delay > WATCHDOG_MAX_SKEW/2,
245+
* report system busy, reinit the watchdog and skip the current
246+
* watchdog test.
247+
*/
248+
wd_delta = clocksource_delta(wd_end2, wd_end, watchdog->mask);
249+
wd_seq_delay = clocksource_cyc2ns(wd_delta, watchdog->mult, watchdog->shift);
250+
if (wd_seq_delay > WATCHDOG_MAX_SKEW/2)
251+
goto skip_test;
231252
}
232253

233254
pr_warn("timekeeping watchdog on CPU%d: %s read-back delay of %lldns, attempt %d, marking unstable\n",
234255
smp_processor_id(), watchdog->name, wd_delay, nretries);
235-
return false;
256+
return WD_READ_UNSTABLE;
257+
258+
skip_test:
259+
pr_info("timekeeping watchdog on CPU%d: %s wd-wd read-back delay of %lldns\n",
260+
smp_processor_id(), watchdog->name, wd_seq_delay);
261+
pr_info("wd-%s-wd read-back delay of %lldns, clock-skew test skipped!\n",
262+
cs->name, wd_delay);
263+
return WD_READ_SKIP;
236264
}
237265

238266
static u64 csnow_mid;
@@ -356,6 +384,7 @@ static void clocksource_watchdog(struct timer_list *unused)
356384
int next_cpu, reset_pending;
357385
int64_t wd_nsec, cs_nsec;
358386
struct clocksource *cs;
387+
enum wd_read_status read_ret;
359388
u32 md;
360389

361390
spin_lock(&watchdog_lock);
@@ -373,9 +402,12 @@ static void clocksource_watchdog(struct timer_list *unused)
373402
continue;
374403
}
375404

376-
if (!cs_watchdog_read(cs, &csnow, &wdnow)) {
377-
/* Clock readout unreliable, so give it up. */
378-
__clocksource_unstable(cs);
405+
read_ret = cs_watchdog_read(cs, &csnow, &wdnow);
406+
407+
if (read_ret != WD_READ_SUCCESS) {
408+
if (read_ret == WD_READ_UNSTABLE)
409+
/* Clock readout unreliable, so give it up. */
410+
__clocksource_unstable(cs);
379411
continue;
380412
}
381413

tools/testing/selftests/rcutorture/configs/rcu/SRCU-T

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ CONFIG_SMP=n
22
CONFIG_PREEMPT_NONE=y
33
CONFIG_PREEMPT_VOLUNTARY=n
44
CONFIG_PREEMPT=n
5+
CONFIG_PREEMPT_DYNAMIC=n
56
#CHECK#CONFIG_TINY_SRCU=y
67
CONFIG_RCU_TRACE=n
78
CONFIG_DEBUG_LOCK_ALLOC=y

tools/testing/selftests/rcutorture/configs/rcu/SRCU-U

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ CONFIG_SMP=n
22
CONFIG_PREEMPT_NONE=y
33
CONFIG_PREEMPT_VOLUNTARY=n
44
CONFIG_PREEMPT=n
5+
CONFIG_PREEMPT_DYNAMIC=n
56
#CHECK#CONFIG_TINY_SRCU=y
67
CONFIG_RCU_TRACE=n
78
CONFIG_DEBUG_LOCK_ALLOC=n

tools/testing/selftests/rcutorture/configs/rcu/TINY01

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ CONFIG_SMP=n
22
CONFIG_PREEMPT_NONE=y
33
CONFIG_PREEMPT_VOLUNTARY=n
44
CONFIG_PREEMPT=n
5+
CONFIG_PREEMPT_DYNAMIC=n
56
#CHECK#CONFIG_TINY_RCU=y
67
CONFIG_HZ_PERIODIC=n
78
CONFIG_NO_HZ_IDLE=y

tools/testing/selftests/rcutorture/configs/rcu/TINY02

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ CONFIG_SMP=n
22
CONFIG_PREEMPT_NONE=y
33
CONFIG_PREEMPT_VOLUNTARY=n
44
CONFIG_PREEMPT=n
5+
CONFIG_PREEMPT_DYNAMIC=n
56
#CHECK#CONFIG_TINY_RCU=y
67
CONFIG_HZ_PERIODIC=y
78
CONFIG_NO_HZ_IDLE=n

tools/testing/selftests/rcutorture/configs/rcuscale/TINY

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ CONFIG_SMP=n
22
CONFIG_PREEMPT_NONE=y
33
CONFIG_PREEMPT_VOLUNTARY=n
44
CONFIG_PREEMPT=n
5+
CONFIG_PREEMPT_DYNAMIC=n
56
#CHECK#CONFIG_TINY_RCU=y
67
CONFIG_HZ_PERIODIC=n
78
CONFIG_NO_HZ_IDLE=y

0 commit comments

Comments
 (0)