Skip to content

Commit f87cbcb

Browse files
KAGA-KOKOIngo Molnar
authored andcommitted
timekeeping: Use READ/WRITE_ONCE() for tick_do_timer_cpu
tick_do_timer_cpu is used lockless to check which CPU needs to take care of the per tick timekeeping duty. This is done to avoid a thundering herd problem on jiffies_lock. The read and writes are not annotated so KCSAN complains about data races: BUG: KCSAN: data-race in tick_nohz_idle_stop_tick / tick_nohz_next_event write to 0xffffffff8a2bda30 of 4 bytes by task 0 on cpu 26: tick_nohz_idle_stop_tick+0x3b1/0x4a0 do_idle+0x1e3/0x250 read to 0xffffffff8a2bda30 of 4 bytes by task 0 on cpu 16: tick_nohz_next_event+0xe7/0x1e0 tick_nohz_get_sleep_length+0xa7/0xe0 menu_select+0x82/0xb90 cpuidle_select+0x44/0x60 do_idle+0x1c2/0x250 value changed: 0x0000001a -> 0xffffffff Annotate them with READ/WRITE_ONCE() to document the intentional data race. Reported-by: Mirsad Todorovac <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Tested-by: Sean Anderson <[email protected]> Link: https://lore.kernel.org/r/87cyqy7rt3.ffs@tglx
1 parent 6d029c2 commit f87cbcb

File tree

2 files changed

+31
-22
lines changed

2 files changed

+31
-22
lines changed

kernel/time/tick-common.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* Copyright(C) 2005-2007, Red Hat, Inc., Ingo Molnar
88
* Copyright(C) 2006-2007, Timesys Corp., Thomas Gleixner
99
*/
10+
#include <linux/compiler.h>
1011
#include <linux/cpu.h>
1112
#include <linux/err.h>
1213
#include <linux/hrtimer.h>
@@ -84,7 +85,7 @@ int tick_is_oneshot_available(void)
8485
*/
8586
static void tick_periodic(int cpu)
8687
{
87-
if (tick_do_timer_cpu == cpu) {
88+
if (READ_ONCE(tick_do_timer_cpu) == cpu) {
8889
raw_spin_lock(&jiffies_lock);
8990
write_seqcount_begin(&jiffies_seq);
9091

@@ -215,8 +216,8 @@ static void tick_setup_device(struct tick_device *td,
215216
* If no cpu took the do_timer update, assign it to
216217
* this cpu:
217218
*/
218-
if (tick_do_timer_cpu == TICK_DO_TIMER_BOOT) {
219-
tick_do_timer_cpu = cpu;
219+
if (READ_ONCE(tick_do_timer_cpu) == TICK_DO_TIMER_BOOT) {
220+
WRITE_ONCE(tick_do_timer_cpu, cpu);
220221
tick_next_period = ktime_get();
221222
#ifdef CONFIG_NO_HZ_FULL
222223
/*
@@ -232,7 +233,7 @@ static void tick_setup_device(struct tick_device *td,
232233
!tick_nohz_full_cpu(cpu)) {
233234
tick_take_do_timer_from_boot();
234235
tick_do_timer_boot_cpu = -1;
235-
WARN_ON(tick_do_timer_cpu != cpu);
236+
WARN_ON(READ_ONCE(tick_do_timer_cpu) != cpu);
236237
#endif
237238
}
238239

@@ -406,10 +407,10 @@ void tick_assert_timekeeping_handover(void)
406407
int tick_cpu_dying(unsigned int dying_cpu)
407408
{
408409
/*
409-
* If the current CPU is the timekeeper, it's the only one that
410-
* can safely hand over its duty. Also all online CPUs are in
411-
* stop machine, guaranteed not to be idle, therefore it's safe
412-
* to pick any online successor.
410+
* If the current CPU is the timekeeper, it's the only one that can
411+
* safely hand over its duty. Also all online CPUs are in stop
412+
* machine, guaranteed not to be idle, therefore there is no
413+
* concurrency and it's safe to pick any online successor.
413414
*/
414415
if (tick_do_timer_cpu == dying_cpu)
415416
tick_do_timer_cpu = cpumask_first(cpu_online_mask);

kernel/time/tick-sched.c

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*
99
* Started by: Thomas Gleixner and Ingo Molnar
1010
*/
11+
#include <linux/compiler.h>
1112
#include <linux/cpu.h>
1213
#include <linux/err.h>
1314
#include <linux/hrtimer.h>
@@ -204,7 +205,7 @@ static inline void tick_sched_flag_clear(struct tick_sched *ts,
204205

205206
static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
206207
{
207-
int cpu = smp_processor_id();
208+
int tick_cpu, cpu = smp_processor_id();
208209

209210
/*
210211
* Check if the do_timer duty was dropped. We don't care about
@@ -216,16 +217,18 @@ static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
216217
* If nohz_full is enabled, this should not happen because the
217218
* 'tick_do_timer_cpu' CPU never relinquishes.
218219
*/
219-
if (IS_ENABLED(CONFIG_NO_HZ_COMMON) &&
220-
unlikely(tick_do_timer_cpu == TICK_DO_TIMER_NONE)) {
220+
tick_cpu = READ_ONCE(tick_do_timer_cpu);
221+
222+
if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && unlikely(tick_cpu == TICK_DO_TIMER_NONE)) {
221223
#ifdef CONFIG_NO_HZ_FULL
222224
WARN_ON_ONCE(tick_nohz_full_running);
223225
#endif
224-
tick_do_timer_cpu = cpu;
226+
WRITE_ONCE(tick_do_timer_cpu, cpu);
227+
tick_cpu = cpu;
225228
}
226229

227230
/* Check if jiffies need an update */
228-
if (tick_do_timer_cpu == cpu)
231+
if (tick_cpu == cpu)
229232
tick_do_update_jiffies64(now);
230233

231234
/*
@@ -610,7 +613,7 @@ bool tick_nohz_cpu_hotpluggable(unsigned int cpu)
610613
* timers, workqueues, timekeeping, ...) on behalf of full dynticks
611614
* CPUs. It must remain online when nohz full is enabled.
612615
*/
613-
if (tick_nohz_full_running && tick_do_timer_cpu == cpu)
616+
if (tick_nohz_full_running && READ_ONCE(tick_do_timer_cpu) == cpu)
614617
return false;
615618
return true;
616619
}
@@ -891,6 +894,7 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
891894
{
892895
u64 basemono, next_tick, delta, expires;
893896
unsigned long basejiff;
897+
int tick_cpu;
894898

895899
basemono = get_jiffies_update(&basejiff);
896900
ts->last_jiffies = basejiff;
@@ -947,9 +951,9 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
947951
* Otherwise we can sleep as long as we want.
948952
*/
949953
delta = timekeeping_max_deferment();
950-
if (cpu != tick_do_timer_cpu &&
951-
(tick_do_timer_cpu != TICK_DO_TIMER_NONE ||
952-
!tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST)))
954+
tick_cpu = READ_ONCE(tick_do_timer_cpu);
955+
if (tick_cpu != cpu &&
956+
(tick_cpu != TICK_DO_TIMER_NONE || !tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST)))
953957
delta = KTIME_MAX;
954958

955959
/* Calculate the next expiry time */
@@ -970,6 +974,7 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
970974
unsigned long basejiff = ts->last_jiffies;
971975
u64 basemono = ts->timer_expires_base;
972976
bool timer_idle = tick_sched_flag_test(ts, TS_FLAG_STOPPED);
977+
int tick_cpu;
973978
u64 expires;
974979

975980
/* Make sure we won't be trying to stop it twice in a row. */
@@ -1007,10 +1012,11 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu)
10071012
* do_timer() never gets invoked. Keep track of the fact that it
10081013
* was the one which had the do_timer() duty last.
10091014
*/
1010-
if (cpu == tick_do_timer_cpu) {
1011-
tick_do_timer_cpu = TICK_DO_TIMER_NONE;
1015+
tick_cpu = READ_ONCE(tick_do_timer_cpu);
1016+
if (tick_cpu == cpu) {
1017+
WRITE_ONCE(tick_do_timer_cpu, TICK_DO_TIMER_NONE);
10121018
tick_sched_flag_set(ts, TS_FLAG_DO_TIMER_LAST);
1013-
} else if (tick_do_timer_cpu != TICK_DO_TIMER_NONE) {
1019+
} else if (tick_cpu != TICK_DO_TIMER_NONE) {
10141020
tick_sched_flag_clear(ts, TS_FLAG_DO_TIMER_LAST);
10151021
}
10161022

@@ -1173,15 +1179,17 @@ static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
11731179
return false;
11741180

11751181
if (tick_nohz_full_enabled()) {
1182+
int tick_cpu = READ_ONCE(tick_do_timer_cpu);
1183+
11761184
/*
11771185
* Keep the tick alive to guarantee timekeeping progression
11781186
* if there are full dynticks CPUs around
11791187
*/
1180-
if (tick_do_timer_cpu == cpu)
1188+
if (tick_cpu == cpu)
11811189
return false;
11821190

11831191
/* Should not happen for nohz-full */
1184-
if (WARN_ON_ONCE(tick_do_timer_cpu == TICK_DO_TIMER_NONE))
1192+
if (WARN_ON_ONCE(tick_cpu == TICK_DO_TIMER_NONE))
11851193
return false;
11861194
}
11871195

0 commit comments

Comments
 (0)