Re: [PATCH v6 6/6] timers: Exclude isolated cpus from timer migation

Gabriele Monaco
2025-06-25T13:46:08Z

On Wed, 2025-06-25 at 15:16 +0200, Frederic Weisbecker wrote:
> Le Wed, Jun 25, 2025 at 12:45:32PM +0200, Thomas Gleixner a écrit :
> > On Tue, Jun 24 2025 at 16:52, Frederic Weisbecker wrote:
> > > Right I think you need to keep those checks because if CPU 0 is
> > > isolcpus
> > > and CPU 5 is nohz_full, CPU 0 will become later the timekeeper
> > > and must stay
> > > in the tmigr hierarchy.
> > >
> > > OTOH if CPU 0 is isolcpus and there is no nohz_full CPUs, then
> > > CPU 0 doesn't
> > > want to go to the hierarchy.
> > >
> > > cpuset isolated partitions are different because they issue SMP
> > > calls whereas
> > > isolcpus is defined on boot.
> > >
> > > An alternative for isolcpus could be to make a late initcall and
> > > do the smp
> > > calls from there just like is done for cpusets.
> >
> > There is zero reason for isolcpus and nohz full muck to be
> > active/evaluated during early boot. That's all irrelevant and just
> > complicates things further.
> >
> > Can we please clean this up and make it a sensible design instead
> > of
> > duct taping new functionality into it in completely
> > incomprehensible
> > ways?
> >
> > Especially under the aspect that all this should become run-time
> > modifyable. That requires a run-time switch mechanism anyway, so
> > the
> > obvious design choice is to utilize that run-time switch late in
> > the
> > boot sequence to set this stuff up before user space starts and
> > leave
> > the boot process alone and simple.
> >
> > The KISS principle applies here fully.
>
> Ok so the late initcall should work.
>

Thanks both for the reviews.
I'm a bit puzzled by what is expected now, though.

The late initcall would work just fine to replace the call to
tick_nohz_cpu_hotpluggable(), indeed superfluous for hotplug calls,
however the checks for housekeeping CPUs is required to prevent
isolated CPUs getting online from becoming available and so will run on
early boot too (without any practical reason, only because the hotplug
handlers run there).

I might avoid it by playing with cpuhp_setup_state_nocalls perhaps, but
that feels even more hacky.

Otherwise, I can refactor the code to maintain a separate field
(isolated), restore the 'online' field and keep the functions for
online/offline and isolation as separate as possible, while considering
available = !isolated && online

This would make reading housekeeping masks superfluous on hotplug (and
boot) code, but again, it doesn't look simpler to me.

Am I missing some obviously elegant solution here?

Thanks,
Gabriele