Skip to content

Commit 76031d9

Browse files
committed
clocksource: Make negative motion detection more robust
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: c163e40 ("timekeeping: Always check for negative motion") Reported-by: Guenter Roeck <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Guenter Roeck <[email protected]> Link: https://lore.kernel.org/all/8734j5ul4x.ffs@tglx Closes: https://lore.kernel.org/all/[email protected]
1 parent e70140b commit 76031d9

File tree

4 files changed

+20
-7
lines changed

4 files changed

+20
-7
lines changed

include/linux/clocksource.h

+2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ struct module;
4949
* @archdata: Optional arch-specific data
5050
* @max_cycles: Maximum safe cycle value which won't overflow on
5151
* multiplication
52+
* @max_raw_delta: Maximum safe delta value for negative motion detection
5253
* @name: Pointer to clocksource name
5354
* @list: List head for registration (internal)
5455
* @freq_khz: Clocksource frequency in khz.
@@ -109,6 +110,7 @@ struct clocksource {
109110
struct arch_clocksource_data archdata;
110111
#endif
111112
u64 max_cycles;
113+
u64 max_raw_delta;
112114
const char *name;
113115
struct list_head list;
114116
u32 freq_khz;

kernel/time/clocksource.c

+10-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ static void clocksource_enqueue(struct clocksource *cs);
2424

2525
static noinline u64 cycles_to_nsec_safe(struct clocksource *cs, u64 start, u64 end)
2626
{
27-
u64 delta = clocksource_delta(end, start, cs->mask);
27+
u64 delta = clocksource_delta(end, start, cs->mask, cs->max_raw_delta);
2828

2929
if (likely(delta < cs->max_cycles))
3030
return clocksource_cyc2ns(delta, cs->mult, cs->shift);
@@ -993,6 +993,15 @@ static inline void clocksource_update_max_deferment(struct clocksource *cs)
993993
cs->max_idle_ns = clocks_calc_max_nsecs(cs->mult, cs->shift,
994994
cs->maxadj, cs->mask,
995995
&cs->max_cycles);
996+
997+
/*
998+
* Threshold for detecting negative motion in clocksource_delta().
999+
*
1000+
* Allow for 0.875 of the counter width so that overly long idle
1001+
* sleeps, which go slightly over mask/2, do not trigger the
1002+
* negative motion detection.
1003+
*/
1004+
cs->max_raw_delta = (cs->mask >> 1) + (cs->mask >> 2) + (cs->mask >> 3);
9961005
}
9971006

9981007
static struct clocksource *clocksource_find_best(bool oneshot, bool skipcur)

kernel/time/timekeeping.c

+4-2
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,8 @@ static void timekeeping_forward_now(struct timekeeper *tk)
755755
u64 cycle_now, delta;
756756

757757
cycle_now = tk_clock_read(&tk->tkr_mono);
758-
delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
758+
delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask,
759+
tk->tkr_mono.clock->max_raw_delta);
759760
tk->tkr_mono.cycle_last = cycle_now;
760761
tk->tkr_raw.cycle_last = cycle_now;
761762

@@ -2230,7 +2231,8 @@ static bool timekeeping_advance(enum timekeeping_adv_mode mode)
22302231
return false;
22312232

22322233
offset = clocksource_delta(tk_clock_read(&tk->tkr_mono),
2233-
tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
2234+
tk->tkr_mono.cycle_last, tk->tkr_mono.mask,
2235+
tk->tkr_mono.clock->max_raw_delta);
22342236

22352237
/* Check if there's really nothing to do */
22362238
if (offset < real_tk->cycle_interval && mode == TK_ADV_TICK)

kernel/time/timekeeping_internal.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ static inline void timekeeping_inc_mg_floor_swaps(void)
3030

3131
#endif
3232

33-
static inline u64 clocksource_delta(u64 now, u64 last, u64 mask)
33+
static inline u64 clocksource_delta(u64 now, u64 last, u64 mask, u64 max_delta)
3434
{
3535
u64 ret = (now - last) & mask;
3636

3737
/*
38-
* Prevent time going backwards by checking the MSB of mask in
39-
* the result. If set, return 0.
38+
* Prevent time going backwards by checking the result against
39+
* @max_delta. If greater, return 0.
4040
*/
41-
return ret & ~(mask >> 1) ? 0 : ret;
41+
return ret > max_delta ? 0 : ret;
4242
}
4343

4444
/* Semi public for serialization of non timekeeper VDSO updates. */

0 commit comments

Comments
 (0)