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

[Gabriele Monaco <gmonaco@redhat.com>]

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?

> >   
> > +/**
> > + * 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 

Thanks,
Gabriele