linux-kernel - Re: [PATCH v11 8/8] 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: <aLYMA8niL9Uxhu7G@pavilion.home>
Date: Mon, 1 Sep 2025 23:11:31 +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 v11 8/8] timers: Exclude isolated cpus from timer
 migration

Le Mon, Sep 01, 2025 at 03:48:15PM +0200, Gabriele Monaco a écrit :
> On Mon, 2025-09-01 at 14:41 +0200, Frederic Weisbecker wrote:
> > Le Fri, Aug 08, 2025 at 06:01:42PM +0200, Gabriele Monaco a écrit :
> > >  /*
> > >   * 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_set_cpu_available,
> > > tmigr_clear_cpu_available);
> > > +	int cpu, ret;
> > > +
> > > +	ret = cpuhp_setup_state(CPUHP_AP_TMIGR_ONLINE,
> > > "tmigr:online",
> > > +				tmigr_set_cpu_available,
> > > tmigr_clear_cpu_available);
> > > +	if (ret)
> > > +		return ret;
> > > +	/*
> > > +	 * The tick CPU may not be marked as available in the
> > > above call, this
> > > +	 * can occur only at boot as hotplug handlers are not
> > > called on the
> > > +	 * tick CPU. Force it enabled here.
> > > +	 */
> > > +	for_each_possible_cpu(cpu) {
> > > +		if (!tick_nohz_cpu_hotpluggable(cpu)) {
> > > +			ret = smp_call_function_single(
> > > +				cpu, tmigr_cpu_unisolate_force,
> > > NULL, 1);
> > > +			break;
> > > +		}
> > > +	}
> > 
> > Why not evaluate tick_nohz_cpu_hotpluggable() from
> > tmigr_clear_cpu_available() instead of this force IPI?
> 
> The idea is that this IPI runs once during late boot only for the tick
> CPU, while the call to tick_nohz_cpu_hotpluggable() would be running at
> every hotplug event if I move it to tmigr_clear_cpu_available.
> In that scenario, it's guaranteed to return true (besides the very
> first call).
> 
> I don't have a strong opinion against running that check every time
> although it's needed only at boot time and remove this IPI, but in my
> understanding that's one of the thing Thomas was finding confusing [1].
> 
> Am I missing anything here?

Right, Thomas didn't like it, but the organization of the code has changed
a bit since then with the late initcall. If the best we can do to workaround
the situation is to make the CPU unavailable regardless and then undo that
right after with an IPI, then it's a good sign that we should just simplify
and eventually check tick_nohz_cpu_hotpluggable() from tmigr_is_isolated().

> > But if I understand correctly, this will be handled by cpuset, right?
> 
> Currently tick_nohz_cpu_hotpluggable() is called by
> tmigr_should_isolate_cpu() and that is called by cpuset code, changing
> cpuset would save that call but won't deal with the tick CPU not
> enabled at boot time, unless I'm misunderstanding what Waiman implied.

Good point!

Thanks.

> 
> Thanks,
> Gabriele
> 
> [1] - https://lore.kernel.org/lkml/875xgqqrel.ffs@tglx
> 

-- 
Frederic Weisbecker
SUSE Labs