]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
clocksource: Make negative motion detection more robust
authorThomas Gleixner <tglx@linutronix.de>
Tue, 3 Dec 2024 10:16:30 +0000 (11:16 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 14 Dec 2024 19:04:16 +0000 (20:04 +0100)
commit 76031d9536a076bf023bedbdb1b4317fc801dd67 upstream.

Guenter reported boot stalls on a emulated ARM 32-bit platform, which has a
24-bit wide clocksource.

It turns out that the calculated maximal idle time, which limits idle
sleeps to prevent clocksource wrap arounds, is close to the point where the
negative motion detection triggers.

  max_idle_ns:                    597268854 ns
  negative motion tripping point: 671088640 ns

If the idle wakeup is delayed beyond that point, the clocksource
advances far enough to trigger the negative motion detection. This
prevents the clock to advance and in the worst case the system stalls
completely if the consecutive sleeps based on the stale clock are
delayed as well.

Cure this by calculating a more robust cut-off value for negative motion,
which covers 87.5% of the actual clocksource counter width. Compare the
delta against this value to catch negative motion. This is specifically for
clock sources with a small counter width as their wrap around time is close
to the half counter width. For clock sources with wide counters this is not
a problem because the maximum idle time is far from the half counter width
due to the math overflow protection constraints.

For the case at hand this results in a tripping point of 1174405120ns.

Note, that this cannot prevent issues when the delay exceeds the 87.5%
margin, but that's not different from the previous unchecked version which
allowed arbitrary time jumps.

Systems with small counter width are prone to invalid results, but this
problem is unlikely to be seen on real hardware. If such a system
completely stalls for more than half a second, then there are other more
urgent problems than the counter wrapping around.

Fixes: c163e40af9b2 ("timekeeping: Always check for negative motion")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/all/8734j5ul4x.ffs@tglx
Closes: https://lore.kernel.org/all/387b120b-d68a-45e8-b6ab-768cd95d11c2@roeck-us.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
include/linux/clocksource.h
kernel/time/clocksource.c
kernel/time/timekeeping.c
kernel/time/timekeeping_internal.h

index d35b677b08fe1365145160fab0f9e24e6720aff7..c846436b64593e913d452d2ba07870e13729ea21 100644 (file)
@@ -49,6 +49,7 @@ struct module;
  * @archdata:          Optional arch-specific data
  * @max_cycles:                Maximum safe cycle value which won't overflow on
  *                     multiplication
+ * @max_raw_delta:     Maximum safe delta value for negative motion detection
  * @name:              Pointer to clocksource name
  * @list:              List head for registration (internal)
  * @freq_khz:          Clocksource frequency in khz.
@@ -109,6 +110,7 @@ struct clocksource {
        struct arch_clocksource_data archdata;
 #endif
        u64                     max_cycles;
+       u64                     max_raw_delta;
        const char              *name;
        struct list_head        list;
        u32                     freq_khz;
index 23336eecb4f43b4fa23ded7f0bdc9ce81b1799e3..8a40a616288b81974ad5c814fa1f7aa71877241e 100644 (file)
@@ -22,7 +22,7 @@
 
 static noinline u64 cycles_to_nsec_safe(struct clocksource *cs, u64 start, u64 end)
 {
-       u64 delta = clocksource_delta(end, start, cs->mask);
+       u64 delta = clocksource_delta(end, start, cs->mask, cs->max_raw_delta);
 
        if (likely(delta < cs->max_cycles))
                return clocksource_cyc2ns(delta, cs->mult, cs->shift);
@@ -985,6 +985,15 @@ static inline void clocksource_update_max_deferment(struct clocksource *cs)
        cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
                                                cs->maxadj, cs->mask,
                                                &cs->max_cycles);
+
+       /*
+        * Threshold for detecting negative motion in clocksource_delta().
+        *
+        * Allow for 0.875 of the counter width so that overly long idle
+        * sleeps, which go slightly over mask/2, do not trigger the
+        * negative motion detection.
+        */
+       cs->max_raw_delta = (cs->mask >> 1) + (cs->mask >> 2) + (cs->mask >> 3);
 }
 
 static struct clocksource *clocksource_find_best(bool oneshot, bool skipcur)
index 171a00f9184bad788c3ddaf7f6c6d801115b034c..96933082431fe0c99a2f0704d9c5c64dd57de3cf 100644 (file)
@@ -694,7 +694,8 @@ static void timekeeping_forward_now(struct timekeeper *tk)
        u64 cycle_now, delta;
 
        cycle_now = tk_clock_read(&tk->tkr_mono);
-       delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
+       delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask,
+                                 tk->tkr_mono.clock->max_raw_delta);
        tk->tkr_mono.cycle_last = cycle_now;
        tk->tkr_raw.cycle_last  = cycle_now;
 
@@ -2193,7 +2194,8 @@ static bool timekeeping_advance(enum timekeeping_adv_mode mode)
                goto out;
 
        offset = clocksource_delta(tk_clock_read(&tk->tkr_mono),
-                                  tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
+                                  tk->tkr_mono.cycle_last, tk->tkr_mono.mask,
+                                  tk->tkr_mono.clock->max_raw_delta);
 
        /* Check if there's really nothing to do */
        if (offset < real_tk->cycle_interval && mode == TK_ADV_TICK)
index 1d4854d5c386e20e141c4cf4d5ca3872a36b4633..feb366b01428874553f17007c0170cf7d31d3b89 100644 (file)
@@ -15,15 +15,15 @@ extern void tk_debug_account_sleep_time(const struct timespec64 *t);
 #define tk_debug_account_sleep_time(x)
 #endif
 
-static inline u64 clocksource_delta(u64 now, u64 last, u64 mask)
+static inline u64 clocksource_delta(u64 now, u64 last, u64 mask, u64 max_delta)
 {
        u64 ret = (now - last) & mask;
 
        /*
-        * Prevent time going backwards by checking the MSB of mask in
-        * the result. If set, return 0.
+        * Prevent time going backwards by checking the result against
+        * @max_delta. If greater, return 0.
         */
-       return ret & ~(mask >> 1) ? 0 : ret;
+       return ret > max_delta ? 0 : ret;
 }
 
 /* Semi public for serialization of non timekeeper VDSO updates. */