Re: [PATCH v11 8/8] timers: Exclude isolated cpus from timer migration

[Gabriele Monaco <gmonaco@redhat.com>]

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?

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

Thanks,
Gabriele

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