linux-kernel - Re: [PATCH v15 7/7] 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] [day] [month] [year] [list]

Message-ID: <1976fcd8034c1a1dc06fdd10e09b7df5ed4037ac.camel@redhat.com>
Date: Thu, 20 Nov 2025 11:48:24 +0100
From: Gabriele Monaco <gmonaco@...hat.com>
To: Waiman Long <llong@...hat.com>, linux-kernel@...r.kernel.org, Anna-Maria
 Behnsen <anna-maria@...utronix.de>, Frederic Weisbecker
 <frederic@...nel.org>, Thomas Gleixner	 <tglx@...utronix.de>
Cc: "John B. Wyatt IV" <jwyatt@...hat.com>, "John B. Wyatt IV"
	 <sageofredondo@...il.com>
Subject: Re: [PATCH v15 7/7] timers: Exclude isolated cpus from timer
 migration

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