From nobody Mon Feb 9 04:30:20 2026 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 871F180629 for ; Tue, 9 Apr 2024 10:29:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712658556; cv=none; b=M5YBwqpv/SVf31v+VU6khHTF00eggkbZ9HWPqQO1OwQXfoSjrPjCdz8UEkstrg8ylt9xPL0uyY2sjQCEXsKsKk1qcwV3zKWP/egRVNgHNCzBgVMAqGNr8GV1A3VElFbE7DciF0VevR0wursIlcn+i+NeyZUNsoX5v4j72/WA/8E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712658556; c=relaxed/simple; bh=XNnyP8qVi1HgN7u6NP7Q7yseLfpjzGBeNAOYFNuri88=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=RZzhbBisiyO+IiR8jQc2nFJN22vw4mn/jnQki47I4ILeY0W5njXmUusjaw+o+kCU9AVAip+RMoOissGq83EJGBsgvph4DLy/PQye80AvV7OHSOhqdkm4VAk71mmQ64E7t3yo+I7aUEq79VSsh3dONLeAqnhryuPecbUOQHA3Q+U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=lYZJJGS1; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=0pcujErO; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="lYZJJGS1"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="0pcujErO" From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1712658552; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EF6ovFLdWNLhvSGuwEQg4yxIeTGiTX0nsnyNSwoTsr0=; b=lYZJJGS1T08kmtgYpWSzPd3Ox4L8SjKZOeTLqC4dEC0cQJRdfztnNIOhWhxc4wW7X+g4H7 +iPdkT5l3MdTExFEjZREIrunsdd0R9L135tv4I+eZ79Wxs7yXHNwTLegUDWefjN49wctZI wtfz12cfp71zTWVC7tMsokckBTfNGn42SJp2Q2S6mzNTu94rTfO5P+oEBLyJJ5CpvEAnfy aiudqhaOglPtSmPR+dHj4++m5/aHeMK7H+l9V3KCrPEhgicbkOvqKH5Fd6zsjXt5DqkKvj rZ9sobL+RXRwaglGNRrv71dWI0Ttu4bLKWcaCfwMLD9g0CCpkpFSnCXXNwGtMg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1712658552; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=EF6ovFLdWNLhvSGuwEQg4yxIeTGiTX0nsnyNSwoTsr0=; b=0pcujErOj0KtWGpK/4UyeHbcdqpP0+9z5skGuaH7NrgnggkFNteBp3Z2wrETC/NgE+lN3+ l2cL83BopLDVphCQ== To: Sean Anderson , Mirsad Todorovac , linux-kernel@vger.kernel.org Cc: Frederic Weisbecker , Ingo Molnar Subject: timekeeping: Use READ/WRITE_ONCE() for tick_do_timer_cpu In-Reply-To: References: <40ac9903-469a-b1ea-4339-8b06f855450e@alu.unizg.hr> <3993c468-fdac-2ff2-c3ee-9784c098694c@alu.unizg.hr> <83b3c900-d4eb-457f-99e6-cb21e0eae4cb@seco.com> <87a5n4rfhu.ffs@tglx> Date: Tue, 09 Apr 2024 12:29:12 +0200 Message-ID: <87cyqy7rt3.ffs@tglx> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 = = =20 = = =20 write to 0xffffffff8a2bda30 of 4 bytes by task 0 on cpu 26: = = =20 tick_nohz_idle_stop_tick+0x3b1/0x4a0 = = =20 do_idle+0x1e3/0x250 = = =20 = = =20 read to 0xffffffff8a2bda30 of 4 bytes by task 0 on cpu 16: = = =20 tick_nohz_next_event+0xe7/0x1e0 = = =20 tick_nohz_get_sleep_length+0xa7/0xe0 = = =20 menu_select+0x82/0xb90 = = =20 cpuidle_select+0x44/0x60 = = =20 do_idle+0x1c2/0x250 = = =20 = = =20 value changed: 0x0000001a -> 0xffffffff =20 Annotate them with READ/WRITE_ONCE() to document the intentional data race. Reported-by: Mirsad Todorovac Signed-off-by: Thomas Gleixner Tested-by: Sean Anderson --- kernel/time/tick-common.c | 17 +++++++++-------- kernel/time/tick-sched.c | 36 ++++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 22 deletions(-) --- a/kernel/time/tick-common.c +++ b/kernel/time/tick-common.c @@ -7,6 +7,7 @@ * Copyright(C) 2005-2007, Red Hat, Inc., Ingo Molnar * Copyright(C) 2006-2007, Timesys Corp., Thomas Gleixner */ +#include #include #include #include @@ -84,7 +85,7 @@ int tick_is_oneshot_available(void) */ static void tick_periodic(int cpu) { - if (tick_do_timer_cpu =3D=3D cpu) { + if (READ_ONCE(tick_do_timer_cpu) =3D=3D cpu) { raw_spin_lock(&jiffies_lock); write_seqcount_begin(&jiffies_seq); =20 @@ -215,8 +216,8 @@ static void tick_setup_device(struct tic * If no cpu took the do_timer update, assign it to * this cpu: */ - if (tick_do_timer_cpu =3D=3D TICK_DO_TIMER_BOOT) { - tick_do_timer_cpu =3D cpu; + if (READ_ONCE(tick_do_timer_cpu) =3D=3D TICK_DO_TIMER_BOOT) { + WRITE_ONCE(tick_do_timer_cpu, cpu); tick_next_period =3D ktime_get(); #ifdef CONFIG_NO_HZ_FULL /* @@ -232,7 +233,7 @@ static void tick_setup_device(struct tic !tick_nohz_full_cpu(cpu)) { tick_take_do_timer_from_boot(); tick_do_timer_boot_cpu =3D -1; - WARN_ON(tick_do_timer_cpu !=3D cpu); + WARN_ON(READ_ON_ONCE(tick_do_timer_cpu) !=3D cpu); #endif } =20 @@ -406,10 +407,10 @@ void tick_assert_timekeeping_handover(vo int tick_cpu_dying(unsigned int dying_cpu) { /* - * If the current CPU is the timekeeper, it's the only one that - * can safely hand over its duty. Also all online CPUs are in - * stop machine, guaranteed not to be idle, therefore it's safe - * to pick any online successor. + * If the current CPU is the timekeeper, it's the only one that can + * safely hand over its duty. Also all online CPUs are in stop + * machine, guaranteed not to be idle, therefore there is no + * concurrency and it's safe to pick any online successor. */ if (tick_do_timer_cpu =3D=3D dying_cpu) tick_do_timer_cpu =3D cpumask_first(cpu_online_mask); --- a/kernel/time/tick-sched.c +++ b/kernel/time/tick-sched.c @@ -8,6 +8,7 @@ * * Started by: Thomas Gleixner and Ingo Molnar */ +#include #include #include #include @@ -204,7 +205,7 @@ static inline void tick_sched_flag_clear =20 static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now) { - int cpu =3D smp_processor_id(); + int tick_cpu, cpu =3D smp_processor_id(); =20 /* * Check if the do_timer duty was dropped. We don't care about @@ -216,16 +217,18 @@ static void tick_sched_do_timer(struct t * If nohz_full is enabled, this should not happen because the * 'tick_do_timer_cpu' CPU never relinquishes. */ - if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && - unlikely(tick_do_timer_cpu =3D=3D TICK_DO_TIMER_NONE)) { + tick_cpu =3D READ_ONCE(tick_do_timer_cpu); + + if (IS_ENABLED(CONFIG_NO_HZ_COMMON) && unlikely(tick_cpu =3D=3D TICK_DO_T= IMER_NONE)) { #ifdef CONFIG_NO_HZ_FULL WARN_ON_ONCE(tick_nohz_full_running); #endif - tick_do_timer_cpu =3D cpu; + WRITE_ONCE(tick_do_timer_cpu, cpu); + tick_cpu =3D cpu; } =20 /* Check if jiffies need an update */ - if (tick_do_timer_cpu =3D=3D cpu) + if (tick_cpu =3D=3D cpu) tick_do_update_jiffies64(now); =20 /* @@ -610,7 +613,7 @@ bool tick_nohz_cpu_hotpluggable(unsigned * timers, workqueues, timekeeping, ...) on behalf of full dynticks * CPUs. It must remain online when nohz full is enabled. */ - if (tick_nohz_full_running && tick_do_timer_cpu =3D=3D cpu) + if (tick_nohz_full_running && READ_ONCE(tick_do_timer_cpu) =3D=3D cpu) return false; return true; } @@ -891,6 +894,7 @@ static ktime_t tick_nohz_next_event(stru { u64 basemono, next_tick, delta, expires; unsigned long basejiff; + int tick_cpu; =20 basemono =3D get_jiffies_update(&basejiff); ts->last_jiffies =3D basejiff; @@ -947,9 +951,9 @@ static ktime_t tick_nohz_next_event(stru * Otherwise we can sleep as long as we want. */ delta =3D timekeeping_max_deferment(); - if (cpu !=3D tick_do_timer_cpu && - (tick_do_timer_cpu !=3D TICK_DO_TIMER_NONE || - !tick_sched_flag_test(ts, TS_FLAG_DO_TIMER_LAST))) + tick_cpu =3D READ_ONCE(tick_do_timer_cpu); + if (tick_cpu !=3D cpu && + (tick_cpu !=3D TICK_DO_TIMER_NONE || !tick_sched_flag_test(ts, TS_FLA= G_DO_TIMER_LAST))) delta =3D KTIME_MAX; =20 /* Calculate the next expiry time */ @@ -970,6 +974,7 @@ static void tick_nohz_stop_tick(struct t unsigned long basejiff =3D ts->last_jiffies; u64 basemono =3D ts->timer_expires_base; bool timer_idle =3D tick_sched_flag_test(ts, TS_FLAG_STOPPED); + int tick_cpu; u64 expires; =20 /* 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 t * do_timer() never gets invoked. Keep track of the fact that it * was the one which had the do_timer() duty last. */ - if (cpu =3D=3D tick_do_timer_cpu) { - tick_do_timer_cpu =3D TICK_DO_TIMER_NONE; + tick_cpu =3D READ_ONCE(tick_do_timer_cpu); + if (tick_cpu =3D=3D cpu) { + WRITE_ONCE(tick_do_timer_cpu, TICK_DO_TIMER_NONE); tick_sched_flag_set(ts, TS_FLAG_DO_TIMER_LAST); - } else if (tick_do_timer_cpu !=3D TICK_DO_TIMER_NONE) { + } else if (tick_cpu !=3D TICK_DO_TIMER_NONE) { tick_sched_flag_clear(ts, TS_FLAG_DO_TIMER_LAST); } =20 @@ -1173,15 +1179,17 @@ static bool can_stop_idle_tick(int cpu, return false; =20 if (tick_nohz_full_enabled()) { + int tick_cpu =3D READ_ONCE(tick_do_timer_cpu); + /* * Keep the tick alive to guarantee timekeeping progression * if there are full dynticks CPUs around */ - if (tick_do_timer_cpu =3D=3D cpu) + if (tick_cpu =3D=3D cpu) return false; =20 /* Should not happen for nohz-full */ - if (WARN_ON_ONCE(tick_do_timer_cpu =3D=3D TICK_DO_TIMER_NONE)) + if (WARN_ON_ONCE(tick_cpu =3D=3D TICK_DO_TIMER_NONE)) return false; }