[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <187a05e7-fcf7-4ed4-99a6-fbece3add5c3@redhat.com>
Date: Thu, 20 Nov 2025 16:04:04 -0500
From: Waiman Long <llong@...hat.com>
To: Gabriele Monaco <gmonaco@...hat.com>, 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 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
Powered by blists - more mailing lists