[patch V3 09/20] cpumask: Cache num_possible_cpus()

Thomas Gleixner posted 20 patches 3 months, 1 week ago
There is a newer version of this series
[patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Thomas Gleixner 3 months, 1 week ago
Reevaluating num_possible_cpus() over and over does not make sense. That
becomes a constant after init as cpu_possible_mask is marked ro_after_init.

Cache the value during initialization and provide that for consumption.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Yury Norov <yury.norov@gmail.com>
---
V2: New patch
---
 include/linux/cpumask.h |   10 ++++++++--
 kernel/cpu.c            |   15 +++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -126,6 +126,7 @@ extern struct cpumask __cpu_dying_mask;
 #define cpu_dying_mask    ((const struct cpumask *)&__cpu_dying_mask)
 
 extern atomic_t __num_online_cpus;
+extern unsigned int __num_possible_cpus;
 
 extern cpumask_t cpus_booted_once_mask;
 
@@ -1152,13 +1153,13 @@ void init_cpu_possible(const struct cpum
 #define __assign_cpu(cpu, mask, val)	\
 	__assign_bit(cpumask_check(cpu), cpumask_bits(mask), (val))
 
-#define set_cpu_possible(cpu, possible)	assign_cpu((cpu), &__cpu_possible_mask, (possible))
 #define set_cpu_enabled(cpu, enabled)	assign_cpu((cpu), &__cpu_enabled_mask, (enabled))
 #define set_cpu_present(cpu, present)	assign_cpu((cpu), &__cpu_present_mask, (present))
 #define set_cpu_active(cpu, active)	assign_cpu((cpu), &__cpu_active_mask, (active))
 #define set_cpu_dying(cpu, dying)	assign_cpu((cpu), &__cpu_dying_mask, (dying))
 
 void set_cpu_online(unsigned int cpu, bool online);
+void set_cpu_possible(unsigned int cpu, bool possible);
 
 /**
  * to_cpumask - convert a NR_CPUS bitmap to a struct cpumask *
@@ -1211,7 +1212,12 @@ static __always_inline unsigned int num_
 {
 	return raw_atomic_read(&__num_online_cpus);
 }
-#define num_possible_cpus()	cpumask_weight(cpu_possible_mask)
+
+static __always_inline unsigned int num_possible_cpus(void)
+{
+	return __num_possible_cpus;
+}
+
 #define num_enabled_cpus()	cpumask_weight(cpu_enabled_mask)
 #define num_present_cpus()	cpumask_weight(cpu_present_mask)
 #define num_active_cpus()	cpumask_weight(cpu_active_mask)
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -3108,6 +3108,9 @@ EXPORT_SYMBOL(__cpu_dying_mask);
 atomic_t __num_online_cpus __read_mostly;
 EXPORT_SYMBOL(__num_online_cpus);
 
+unsigned int __num_possible_cpus __ro_after_init = NR_CPUS;
+EXPORT_SYMBOL(__num_possible_cpus);
+
 void init_cpu_present(const struct cpumask *src)
 {
 	cpumask_copy(&__cpu_present_mask, src);
@@ -3116,6 +3119,7 @@ void init_cpu_present(const struct cpuma
 void init_cpu_possible(const struct cpumask *src)
 {
 	cpumask_copy(&__cpu_possible_mask, src);
+	__num_possible_cpus = cpumask_weight(&__cpu_possible_mask);
 }
 
 void set_cpu_online(unsigned int cpu, bool online)
@@ -3139,6 +3143,17 @@ void set_cpu_online(unsigned int cpu, bo
 	}
 }
 
+void set_cpu_possible(unsigned int cpu, bool possible)
+{
+	if (possible) {
+		if (!cpumask_test_and_set_cpu(cpu, &__cpu_possible_mask))
+			__num_possible_cpus++;
+	} else {
+		if (cpumask_test_and_clear_cpu(cpu, &__cpu_possible_mask))
+			__num_possible_cpus--;
+	}
+}
+
 /*
  * Activate the first processor.
  */
Re: [patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Shrikanth Hegde 3 months, 1 week ago

On 10/29/25 6:39 PM, Thomas Gleixner wrote:
> Reevaluating num_possible_cpus() over and over does not make sense. That
> becomes a constant after init as cpu_possible_mask is marked ro_after_init.
> 
> Cache the value during initialization and provide that for consumption.
> 

Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>


Observation:
I see below usage which could be improved too?
kernel/irq/affinity.c:		set_vecs = cpumask_weight(cpu_possible_mask);
lib/tests/cpumask_kunit.c:	KUNIT_EXPECT_EQ_MSG(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask),

Specially irq_calc_affinity_vectors, it seems to take cpus_read_lock, but I don't think
that lock is protecting possible cpus. possible cpus can't change after boot. No?

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Yury Norov <yury.norov@gmail.com>
> ---
Re: [patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Thomas Gleixner 3 months, 1 week ago
On Mon, Nov 03 2025 at 15:36, Shrikanth Hegde wrote:
> On 10/29/25 6:39 PM, Thomas Gleixner wrote:
>> Reevaluating num_possible_cpus() over and over does not make sense. That
>> becomes a constant after init as cpu_possible_mask is marked ro_after_init.
>> 
>> Cache the value during initialization and provide that for consumption.
>> 
>
> Reviewed-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>
>
> Observation:
> I see below usage which could be improved too?
> kernel/irq/affinity.c:		set_vecs = cpumask_weight(cpu_possible_mask);
> lib/tests/cpumask_kunit.c:	KUNIT_EXPECT_EQ_MSG(test, nr_cpu_ids, cpumask_weight(cpu_possible_mask),
>
> Specially irq_calc_affinity_vectors, it seems to take cpus_read_lock, but I don't think
> that lock is protecting possible cpus. possible cpus can't change after boot. No?

It can't. So yes the cpus_read_lock() is pointless.
Re: [patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Mathieu Desnoyers 3 months, 1 week ago
On 2025-10-29 09:09, Thomas Gleixner wrote:
[...]
>   
> +void set_cpu_possible(unsigned int cpu, bool possible)

Unless I'm missing something, I suspect that "set_cpu_possible()" should
be marked as __init.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Re: [patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Thomas Gleixner 3 months, 1 week ago
On Wed, Oct 29 2025 at 11:54, Mathieu Desnoyers wrote:
> On 2025-10-29 09:09, Thomas Gleixner wrote:
> [...]
>>   
>> +void set_cpu_possible(unsigned int cpu, bool possible)
>
> Unless I'm missing something, I suspect that "set_cpu_possible()" should
> be marked as __init.

Good point!
Re: [patch V3 09/20] cpumask: Cache num_possible_cpus()
Posted by Thomas Gleixner 3 months, 1 week ago
On Wed, Oct 29 2025 at 22:11, Thomas Gleixner wrote:

> On Wed, Oct 29 2025 at 11:54, Mathieu Desnoyers wrote:
>> On 2025-10-29 09:09, Thomas Gleixner wrote:
>> [...]
>>>   
>>> +void set_cpu_possible(unsigned int cpu, bool possible)
>>
>> Unless I'm missing something, I suspect that "set_cpu_possible()" should
>> be marked as __init.
>
> Good point!

And only wishful thinking as set_cpu_possible() is wrongly used in code
which is not marked __init all over the architecture zoo. Cleaning that
up is yet another massive patch series dealing with mostly unmaintained
code. A nice task for people who want to get started with kernel
development. :)

On anything contemporary invoking set_cpu_possible() after init is going
to crash and burn because the related cpumask and variables are marked
RO at that point.

I've just addded a comment to that effect above the function to prevent
people from trying casually.

Thanks,

        tglx