linux-kernel - Re: [PATCH 0/3] hrtimer: Fix timers queued locally from offline CPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4fbc9ab5-43f3-47b7-a8df-27a8e6cf7040@paulmck-laptop>
Date: Fri, 20 Dec 2024 15:19:31 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>, vlad.wing@...il.com,
	rcu@...r.kernel.org, boqun.feng@...il.com, joel@...lfernandes.org,
	neeraj.upadhyay@....com, urezki@...il.com,
	qiang.zhang1211@...il.com, Cheng-Jui.Wang@...iatek.com,
	leitao@...ian.org, kernel-team@...a.com,
	Usama Arif <usamaarif642@...il.com>,
	Anna-Maria Behnsen <anna-maria@...utronix.de>
Subject: Re: [PATCH 0/3] hrtimer: Fix timers queued locally from offline CPUs

On Thu, Dec 19, 2024 at 09:42:48AM -0800, Paul E. McKenney wrote:
> On Wed, Dec 18, 2024 at 05:50:05PM +0100, Frederic Weisbecker wrote:
> > 5c0930ccaad5 ("hrtimers: Push pending hrtimers away from outgoing CPU earlier")
> > was introduced to fix stalls with scheduler bandwidth timers getting
> > migrated while some kthreads handling CPU hotplug rely on bandwidth.
> > 
> > However this has introduced several other issues which used to be
> > confined to RCU. But not anymore as it is spreading to hotplug code
> > itself (https://lore.kernel.org/all/20241213203739.1519801-1-usamaarif642@gmail.com/)
> > 
> > Instead of introducing yet another new hackery, fix the problem in
> > hrtimers for everyone.
> 
> The good news is that this passes 12 hours of 400*TREE03.  (Yay!!!)
> 
> The so-so news is that this gives only about 70% confidence that these
> patches help, but on the other hand, it also gives much higher confidence
> that these patches are not hurting anything.
> 
> At least for TREE03.

Another day, another 12 hours of 400*TREE03 passed.  This gets us up
beyond 90% confidence that these patches help, and even more confidence
that they are not too severely hurting anything.

> The not-so-good news is that this series causes build failures for
> rcutorture scenarios (such as SRCU-T) that build with CONFIG_SMP=n:
> 
> ------------------------------------------------------------------------
> kernel/time/hrtimer.c: In function ‘enqueue_hrtimer_offline’:
> kernel/time/hrtimer.c:1229:42: error: ‘migration_base’ undeclared (first use in this function); did you mean ‘is_migration_base’?
> ------------------------------------------------------------------------
> 
> When built with KCSAN enabled (--kcsan to kvm.sh), there is this
> additional build failure on that same line of code:
> 
> ------------------------------------------------------------------------
> kernel/time/hrtimer.c:1229:3: error: incompatible pointer types assigning to 'volatile typeof (timer->base)' (aka 'struct hrtimer_clock_base *volatile') from 'bool (*)(struct hrtimer_clock_base *)' (aka '_Bool (*)(struct hrtimer_clock_base *)') [-Werror,-Wincompatible-pointer-types]
> 
>  1229 |                 WRITE_ONCE(timer->base, &migration_base);
> ------------------------------------------------------------------------
> 
> Me, I am a bit surprised that enqueue_hrtimer_offline() is even built
> in a CONFIG_SMP=n kernel.
> 
> But there might be some reason why #ifdef-ing out that function's body
> would be a bad idea, so over to you!  ;-)

Curiosity overcame me, and the #ifdef makes things at least appear to
work for CONFIG_SMP=n kernels.  Experimental patch below, which I intend
to use for further testing, but I have no plans to push it upstream.

							Thanx, Paul

------------------------------------------------------------------------

commit 61a0b92b3a3cfef69e3848806e51d1b99a9e9406
Author: Paul E. McKenney <paulmck@...nel.org>
Date:   Fri Dec 20 15:10:26 2024 -0800

    EXP hrtimers: No-op enqueue_hrtimer_offline() if !HOTPLUG_CPU
    
    In kernels built with CONFIG_HOTPLUG_CPU=n, this experimental commit
    uses #ifdef to remove the body of the enqueue_hrtimer_offline() function.
    This might or might not be the correct fix to the build complaint about
    migration_base being undeclared.
    
    Signed-off-by: Paul E. McKenney <paulmck@...nel.org>
    Cc: Vlad Poenaru <vlad.wing@...il.com>
    Cc: Usama Arif <usamaarif642@...il.com>
    Cc: Frederic Weisbecker <frederic@...nel.org>

diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 48c0078d2c4f2..4235b7825b152 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1213,6 +1213,7 @@ static void enqueue_hrtimer_offline(struct hrtimer *timer,
 				    struct hrtimer_clock_base *base,
 				    const enum hrtimer_mode mode)
 {
+#ifdef CONFIG_HOTPLUG_CPU
 	struct hrtimer_cpu_base *new_cpu_base, *old_cpu_base, *this_cpu_base;
 	struct hrtimer_clock_base *new_base;
 	int cpu;
@@ -1238,6 +1239,7 @@ static void enqueue_hrtimer_offline(struct hrtimer *timer,
 
 	if (enqueue_hrtimer(timer, new_base, mode))
 		smp_call_function_single_async(cpu, &new_cpu_base->csd);
+#endif
 }