Re: [PATCH v15 7/7] timers: Exclude isolated cpus from timer migration

[Waiman Long]

On 11/20/25 5:48 AM, Gabriele Monaco wrote:
> On Wed, 2025-11-19 at 15:43 -0500, Waiman Long wrote:
> > On 11/13/25 3:33 AM, Gabriele Monaco wrote:
> > > +static int __tmigr_isolated_exclude_cpumask(struct cpumask
> > > *exclude_cpumask)
> > > +{
> > > +	struct work_struct __percpu *works __free(free_percpu) =
> > > +		alloc_percpu(struct work_struct);
> > > +	cpumask_var_t cpumask_unisol __free(free_cpumask_var) =
> > > CPUMASK_VAR_NULL;
> > > +	cpumask_var_t cpumask_isol __free(free_cpumask_var) =
> > > CPUMASK_VAR_NULL;
> > > +	int cpu;
> > There are currently only 2 callers for this function - from late_init
> > call and from cpuset. Concurrent call is not possible. Maybe we can just
> > pre-allocate these cpumask_var_t and percpu work structures once and
> > reuse it instead of doing an allocation and free each time it is called.
> > The pre-allocation can be done in tmigr_init_isolation().
> I have no strong opinion on this, but after changes suggested by Thomas it gets
> superfluous to allocate 2 cpumasks (after flushing what is now cpumask_unisol
> it's no longer needed and we can re-use it).
>
> Considering this only runs at boot and every time a cpuset changes isolation, is
> it worth the extra steps to pre-allocate?

It is just a suggestion. We can see how it goes and decide if this 
change is needed or not.

> > >    
> > > +/**
> > > + * tmigr_isolated_exclude_cpumask - Exclude given CPUs from hierarchy
> > > + * @exclude_cpumask: the cpumask to be excluded from timer migration
> > > hierarchy
> > > + *
> > > + * This function can be called from cpuset code to provide the new set of
> > > + * isolated CPUs that should be excluded from the hierarchy.
> > > + * Online CPUs not present in exclude_cpumask but already excluded are
> > > brought
> > > + * back to the hierarchy.
> > > + * Functions to isolate/unisolate need to be called locally and can sleep.
> > > + */
> > > +int tmigr_isolated_exclude_cpumask(struct cpumask *exclude_cpumask)
> > > +{
> > > +	lockdep_assert_cpus_held();
> > > +	return __tmigr_isolated_exclude_cpumask(exclude_cpumask);
> > > +}
> > Should we put all these functions under "#if defined(CONFIG_SMP) &&
> > defined(CONFIG_NO_HZ_COMMON)" like in the timer.h header file?
> I think that's implied in the build condition of timer_migration.o
> https://elixir.bootlin.com/linux/v6.17.8/source/kernel/time/Makefile#L27
>
> At least I got these ifdefs from timer_migration.h and none of those functions
> are ifdeffed in timer_migration.c

You are right. I haven't check the condition for building timer_migration.o.

Cheers,
Longman