Commit 2e27e793 authored by Paul E. McKenney's avatar Paul E. McKenney Committed by Thomas Gleixner
Browse files

clocksource: Reduce clocksource-skew threshold



Currently, WATCHDOG_THRESHOLD is set to detect a 62.5-millisecond skew in
a 500-millisecond WATCHDOG_INTERVAL.  This requires that clocks be skewed
by more than 12.5% in order to be marked unstable.  Except that a clock
that is skewed by that much is probably destroying unsuspecting software
right and left.  And given that there are now checks for false-positive
skews due to delays between reading the two clocks, it should be possible
to greatly decrease WATCHDOG_THRESHOLD, at least for fine-grained clocks
such as TSC.

Therefore, add a new uncertainty_margin field to the clocksource structure
that contains the maximum uncertainty in nanoseconds for the corresponding
clock.  This field may be initialized manually, as it is for
clocksource_tsc_early and clocksource_jiffies, which is copied to
refined_jiffies.  If the field is not initialized manually, it will be
computed at clock-registry time as the period of the clock in question
based on the scale and freq parameters to __clocksource_update_freq_scale()
function.  If either of those two parameters are zero, the
tens-of-milliseconds WATCHDOG_THRESHOLD is used as a cowardly alternative
to dividing by zero.  No matter how the uncertainty_margin field is
calculated, it is bounded below by twice WATCHDOG_MAX_SKEW, that is, by 100
microseconds.

Note that manually initialized uncertainty_margin fields are not adjusted,
but there is a WARN_ON_ONCE() that triggers if any such field is less than
twice WATCHDOG_MAX_SKEW.  This WARN_ON_ONCE() is intended to discourage
production use of the one-nanosecond uncertainty_margin values that are
used to test the clock-skew code itself.

The actual clock-skew check uses the sum of the uncertainty_margin fields
of the two clocksource structures being compared.  Integer overflow is
avoided because the largest computed value of the uncertainty_margin
fields is one billion (10^9), and double that value fits into an
unsigned int.  However, if someone manually specifies (say) UINT_MAX,
they will get what they deserve.

Note that the refined_jiffies uncertainty_margin field is initialized to
TICK_NSEC, which means that skew checks involving this clocksource will
be sufficently forgiving.  In a similar vein, the clocksource_tsc_early
uncertainty_margin field is initialized to 32*NSEC_PER_MSEC, which
replicates the current behavior and allows custom setting if needed
in order to address the rare skews detected for this clocksource in
current mainline.

Suggested-by: default avatarThomas Gleixner <tglx@linutronix.de>
Signed-off-by: default avatarPaul E. McKenney <paulmck@kernel.org>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Acked-by: default avatarFeng Tang <feng.tang@intel.com>
Link: https://lore.kernel.org/r/20210527190124.440372-4-paulmck@kernel.org
parent fa218f1c
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -1128,6 +1128,7 @@ static int tsc_cs_enable(struct clocksource *cs)
static struct clocksource clocksource_tsc_early = {
	.name			= "tsc-early",
	.rating			= 299,
	.uncertainty_margin	= 32 * NSEC_PER_MSEC,
	.read			= read_tsc,
	.mask			= CLOCKSOURCE_MASK(64),
	.flags			= CLOCK_SOURCE_IS_CONTINUOUS |
+3 −0
Original line number Diff line number Diff line
@@ -43,6 +43,8 @@ struct module;
 * @shift:		Cycle to nanosecond divisor (power of two)
 * @max_idle_ns:	Maximum idle time permitted by the clocksource (nsecs)
 * @maxadj:		Maximum adjustment value to mult (~11%)
 * @uncertainty_margin:	Maximum uncertainty in nanoseconds per half second.
 *			Zero says to use default WATCHDOG_THRESHOLD.
 * @archdata:		Optional arch-specific data
 * @max_cycles:		Maximum safe cycle value which won't overflow on
 *			multiplication
@@ -98,6 +100,7 @@ struct clocksource {
	u32			shift;
	u64			max_idle_ns;
	u32			maxadj;
	u32			uncertainty_margin;
#ifdef CONFIG_ARCH_CLOCKSOURCE_DATA
	struct arch_clocksource_data archdata;
#endif
+38 −10
Original line number Diff line number Diff line
@@ -95,6 +95,20 @@ static char override_name[CS_NAME_LEN];
static int finished_booting;
static u64 suspend_start;

/*
 * Threshold: 0.0312s, when doubled: 0.0625s.
 * Also a default for cs->uncertainty_margin when registering clocks.
 */
#define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 5)

/*
 * Maximum permissible delay between two readouts of the watchdog
 * clocksource surrounding a read of the clocksource being validated.
 * This delay could be due to SMIs, NMIs, or to VCPU preemptions.  Used as
 * a lower bound for cs->uncertainty_margin values when registering clocks.
 */
#define WATCHDOG_MAX_SKEW (50 * NSEC_PER_USEC)

#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
static void clocksource_watchdog_work(struct work_struct *work);
static void clocksource_select(void);
@@ -121,17 +135,9 @@ static int clocksource_watchdog_kthread(void *data);
static void __clocksource_change_rating(struct clocksource *cs, int rating);

/*
 * Interval: 0.5sec Threshold: 0.0625s
 * Interval: 0.5sec.
 */
#define WATCHDOG_INTERVAL (HZ >> 1)
#define WATCHDOG_THRESHOLD (NSEC_PER_SEC >> 4)

/*
 * Maximum permissible delay between two readouts of the watchdog
 * clocksource surrounding a read of the clocksource being validated.
 * This delay could be due to SMIs, NMIs, or to VCPU preemptions.
 */
#define WATCHDOG_MAX_SKEW (100 * NSEC_PER_USEC)

static void clocksource_watchdog_work(struct work_struct *work)
{
@@ -348,6 +354,7 @@ static void clocksource_watchdog(struct timer_list *unused)
	int next_cpu, reset_pending;
	int64_t wd_nsec, cs_nsec;
	struct clocksource *cs;
	u32 md;

	spin_lock(&watchdog_lock);
	if (!watchdog_running)
@@ -394,7 +401,8 @@ static void clocksource_watchdog(struct timer_list *unused)
			continue;

		/* Check the deviation from the watchdog clocksource. */
		if (abs(cs_nsec - wd_nsec) > WATCHDOG_THRESHOLD) {
		md = cs->uncertainty_margin + watchdog->uncertainty_margin;
		if (abs(cs_nsec - wd_nsec) > md) {
			pr_warn("timekeeping watchdog on CPU%d: Marking clocksource '%s' as unstable because the skew is too large:\n",
				smp_processor_id(), cs->name);
			pr_warn("                      '%s' wd_now: %llx wd_last: %llx mask: %llx\n",
@@ -1047,6 +1055,26 @@ void __clocksource_update_freq_scale(struct clocksource *cs, u32 scale, u32 freq
		clocks_calc_mult_shift(&cs->mult, &cs->shift, freq,
				       NSEC_PER_SEC / scale, sec * scale);
	}

	/*
	 * If the uncertainty margin is not specified, calculate it.
	 * If both scale and freq are non-zero, calculate the clock
	 * period, but bound below at 2*WATCHDOG_MAX_SKEW.  However,
	 * if either of scale or freq is zero, be very conservative and
	 * take the tens-of-milliseconds WATCHDOG_THRESHOLD value for the
	 * uncertainty margin.  Allow stupidly small uncertainty margins
	 * to be specified by the caller for testing purposes, but warn
	 * to discourage production use of this capability.
	 */
	if (scale && freq && !cs->uncertainty_margin) {
		cs->uncertainty_margin = NSEC_PER_SEC / (scale * freq);
		if (cs->uncertainty_margin < 2 * WATCHDOG_MAX_SKEW)
			cs->uncertainty_margin = 2 * WATCHDOG_MAX_SKEW;
	} else if (!cs->uncertainty_margin) {
		cs->uncertainty_margin = WATCHDOG_THRESHOLD;
	}
	WARN_ON_ONCE(cs->uncertainty_margin < 2 * WATCHDOG_MAX_SKEW);

	/*
	 * Ensure clocksources that have large 'mult' values don't overflow
	 * when adjusted.
+8 −7
Original line number Diff line number Diff line
@@ -51,6 +51,7 @@ static u64 jiffies_read(struct clocksource *cs)
static struct clocksource clocksource_jiffies = {
	.name			= "jiffies",
	.rating			= 1, /* lowest valid rating*/
	.uncertainty_margin	= 32 * NSEC_PER_MSEC,
	.read			= jiffies_read,
	.mask			= CLOCKSOURCE_MASK(32),
	.mult			= TICK_NSEC << JIFFIES_SHIFT, /* details above */