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: <381cb8d6b28648dbd5772c6bc0b2f1f10c29b53d.camel@redhat.com>
Date: Fri, 25 Jul 2025 08:42:19 +0200
From: Gabriele Monaco <gmonaco@...hat.com>
To: Frederic Weisbecker <frederic@...nel.org>
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

On Fri, 2025-07-25 at 01:05 +0200, Frederic Weisbecker wrote:
> 
> 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.
> 

Yeah, that's much neater thanks!
I'm going to try it and update the patch.

> 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>
> 

I assume it's cleaner if I squash it in 7/7 and add a
Co-developed-by: Frederic Weisbecker <frederic@...nel.org>
and/or
Signed-off-by: Frederic Weisbecker <frederic@...nel.org>

Do you agree?

Thanks for the review and help,
Gabriele

> 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);