linux-kernel - Re: [PATCH v8 7/7] timers: Exclude isolated cpus from timer migration

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: <aIK8UFz3A7hNc1sq@pavilion.home>
Date: Fri, 25 Jul 2025 01:05:52 +0200
From: Frederic Weisbecker <frederic@...nel.org>
To: Gabriele Monaco <gmonaco@...hat.com>
Cc: linux-kernel@...r.kernel.org,
	Anna-Maria Behnsen <anna-maria@...utronix.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v8 7/7] timers: Exclude isolated cpus from timer migration

Le Mon, Jul 14, 2025 at 03:30:58PM +0200, Gabriele Monaco a écrit :
> diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
> index 878fd3af40ecb..c07cc9a2b209d 100644
> --- a/kernel/time/timer_migration.c
> +++ b/kernel/time/timer_migration.c
> @@ -10,6 +10,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/timerqueue.h>
>  #include <trace/events/ipi.h>
> +#include <linux/sched/isolation.h>
>  
>  #include "timer_migration.h"
>  #include "tick-internal.h"
> @@ -428,6 +429,9 @@ static DEFINE_PER_CPU(struct tmigr_cpu, tmigr_cpu);
>   */
>  static cpumask_var_t tmigr_available_cpumask;
>  
> +/* Enabled during late initcall */
> +static bool tmigr_exclude_isolated __read_mostly;

This variable is still annoying.

> +
>  #define TMIGR_NONE	0xFF
>  #define BIT_CNT		8
[...]  
> +int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
> +{
> +	cpumask_var_t cpumask;
> +
> +	lockdep_assert_cpus_held();
> +
> +	if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
> +		return -ENOMEM;
> +
> +	cpumask_and(cpumask, exclude_cpumask, tmigr_available_cpumask);
> +	cpumask_and(cpumask, cpumask, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE));
> +	on_each_cpu_cond_mask(tmigr_should_isolate_cpu, tmigr_cpu_isolate, NULL,
> +			      1, cpumask);
> +
> +	cpumask_andnot(cpumask, cpu_online_mask, exclude_cpumask);
> +	cpumask_andnot(cpumask, cpumask, tmigr_available_cpumask);
> +	on_each_cpu_mask(cpumask, tmigr_cpu_unisolate, NULL, 1);
> +
> +	free_cpumask_var(cpumask);
> +	return 0;
> +}
> +
> +static int __init tmigr_init_isolation(void)
> +{
> +	cpumask_var_t cpumask;
> +
> +	tmigr_exclude_isolated = true;
> +	if (!housekeeping_enabled(HK_TYPE_DOMAIN))
> +		return 0;
> +	if (!alloc_cpumask_var(&cpumask, GFP_KERNEL))
> +		return -ENOMEM;
> +	cpumask_andnot(cpumask, tmigr_available_cpumask,
> +		       housekeeping_cpumask(HK_TYPE_DOMAIN));
> +	cpumask_and(cpumask, cpumask, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE));
> +	on_each_cpu_cond_mask(tmigr_should_isolate_cpu, tmigr_cpu_isolate, NULL,
> +			      1, cpumask);

And this is basically repeating the same logic as before but in reverse.

Here is a proposal: register the online/offline callbacks later, on
late_initcall(). This solves two problems:

1) The online/offline callbacks are called for the first time in the right
   place. You don't need that tmigr_exclude_isolated anymore.

2) You don't need to make the on_each_cpu_cond_mask() call anymore in
   tmigr_init_isolation(). In fact you don't need that function. The
   online/offline callbacks already take care of everything.

Here is a patch you can use (only built tested):

commit ad21e35e05865e2d37a60bf5d77b0d6fa22a54ee
Author: Frederic Weisbecker <frederic@...nel.org>
Date:   Fri Jul 25 00:06:20 2025 +0200

    timers/migration: Postpone online/offline callbacks registration to late initcall
    
    During the early boot process, the default clocksource used for
    timekeeping is the jiffies. Better clocksources can only be selected
    once clocksource_done_booting() is called as an fs initcall.
    
    NOHZ can only be enabled after that stage, making global timer migration
    irrelevant up to that point.
    
    Therefore, don't bother with trashing the cache within that tree from
    the SMP bootup until NOHZ even matters.
    
    Make the CPUs available to the tree on late initcall, after the right
    clocksource had a chance to be selected. This will also simplify the
    handling of domain isolated CPUs on further patches.
    
    Signed-off-by: Frederic Weisbecker <frederic@...nel.org>

diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 2f6330831f08..f730107d948d 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -1484,6 +1484,17 @@ static int tmigr_cpu_online(unsigned int cpu)
 	return 0;
 }
 
+/*
+ * NOHZ can only be enabled after clocksource_done_booting(). Don't
+ * bother trashing the cache in the tree before.
+ */
+static int __init tmigr_late_init(void)
+{
+	return cpuhp_setup_state(CPUHP_AP_TMIGR_ONLINE, "tmigr:online",
+				 tmigr_cpu_online, tmigr_cpu_offline);
+}
+late_initcall(tmigr_late_init);
+
 static void tmigr_init_group(struct tmigr_group *group, unsigned int lvl,
 			     int node)
 {
@@ -1846,18 +1857,9 @@ static int __init tmigr_init(void)
 
 	ret = cpuhp_setup_state(CPUHP_TMIGR_PREPARE, "tmigr:prepare",
 				tmigr_cpu_prepare, NULL);
-	if (ret)
-		goto err;
-
-	ret = cpuhp_setup_state(CPUHP_AP_TMIGR_ONLINE, "tmigr:online",
-				tmigr_cpu_online, tmigr_cpu_offline);
-	if (ret)
-		goto err;
-
-	return 0;
-
 err:
-	pr_err("Timer migration setup failed\n");
+	if (ret)
+		pr_err("Timer migration setup failed\n");
 	return ret;
 }
 early_initcall(tmigr_init);