Re: [PATCH v4 1/2] sched/topology: improve topology_span_sane speed

[K Prateek Nayak <kprateek.nayak@amd.com>]

On 6/17/2025 2:52 PM, K Prateek Nayak wrote:
>>   [    0.435961] smp: Bringing up secondary CPUs ...
>>   [    0.437573] smpboot: x86: Booting SMP configuration:
>>   [    0.438611] .... node  #0, CPUs:        #1
>>   [    0.440449] .... node  #1, CPUs:    #2  #3
>>   [    0.442906] .... node  #2, CPUs:    #4  #5
>>   [    0.445298] .... node  #3, CPUs:    #6  #7
>>   [    0.447715] .... node  #4, CPUs:    #8  #9
>>   [    0.481482] smp: Brought up 5 nodes, 10 CPUs
>>   [    0.483160] smpboot: Total of 10 processors activated (45892.16 BogoMIPS)
>>   [    0.486872] tl(SMT) CPU(0) ID(0) CPU_TL_SPAN(0) ID_TL_SPAN(0)
>>   [    0.488029] tl(SMT) CPU(1) ID(1) CPU_TL_SPAN(1) ID_TL_SPAN(1)
>>   [    0.489151] tl(SMT) CPU(2) ID(2) CPU_TL_SPAN(2) ID_TL_SPAN(2)
>>   [    0.489761] tl(SMT) CPU(3) ID(3) CPU_TL_SPAN(3) ID_TL_SPAN(3)
>>   [    0.490876] tl(SMT) CPU(4) ID(4) CPU_TL_SPAN(4) ID_TL_SPAN(4)
>>   [    0.491996] tl(SMT) CPU(5) ID(5) CPU_TL_SPAN(5) ID_TL_SPAN(5)
>>   [    0.493115] tl(SMT) CPU(6) ID(6) CPU_TL_SPAN(6) ID_TL_SPAN(6)
>>   [    0.493754] tl(SMT) CPU(7) ID(7) CPU_TL_SPAN(7) ID_TL_SPAN(7)
>>   [    0.494875] tl(SMT) CPU(8) ID(8) CPU_TL_SPAN(8) ID_TL_SPAN(8)
>>   [    0.496008] tl(SMT) CPU(9) ID(9) CPU_TL_SPAN(9) ID_TL_SPAN(9)
>>   [    0.497129] tl(PKG) CPU(0) ID(0) CPU_TL_SPAN(0-1) ID_TL_SPAN(0-1)
>>   [    0.497763] tl(PKG) CPU(1) ID(0) CPU_TL_SPAN(0-1) ID_TL_SPAN(0-1)
>>   [    0.498954] tl(PKG) CPU(2) ID(2) CPU_TL_SPAN(2-3) ID_TL_SPAN(2-3)
>>   [    0.500167] tl(PKG) CPU(3) ID(2) CPU_TL_SPAN(2-3) ID_TL_SPAN(2-3)
>>   [    0.501371] tl(PKG) CPU(4) ID(4) CPU_TL_SPAN(4-5) ID_TL_SPAN(4-5)
>>   [    0.501792] tl(PKG) CPU(5) ID(4) CPU_TL_SPAN(4-5) ID_TL_SPAN(4-5)
>>   [    0.503001] tl(PKG) CPU(6) ID(6) CPU_TL_SPAN(6-7) ID_TL_SPAN(6-7)
>>   [    0.504202] tl(PKG) CPU(7) ID(6) CPU_TL_SPAN(6-7) ID_TL_SPAN(6-7)
>>   [    0.505419] tl(PKG) CPU(8) ID(8) CPU_TL_SPAN(8-9) ID_TL_SPAN(8-9)
>>   [    0.506637] tl(PKG) CPU(9) ID(8) CPU_TL_SPAN(8-9) ID_TL_SPAN(8-9)
>>   [    0.507843] tl(NODE) CPU(0) ID(0) CPU_TL_SPAN(0-1,8-9) ID_TL_SPAN(0-1,8-9)
>>   [    0.509199] tl(NODE) CPU(1) ID(0) CPU_TL_SPAN(0-1,8-9) ID_TL_SPAN(0-1,8-9)
>>   [    0.509792] tl(NODE) CPU(2) ID(2) CPU_TL_SPAN(2-3,8-9) ID_TL_SPAN(2-3,8-9)
> 
> Looking at this, NODE should be a SD_OVERLAP domain here since the spans
> across the nodes overlap. The following solves the warning for me:

So turns out the mask resolved for NODE is all wrong!

> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 8e06b1d22e91..759f7b8e24e6 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -2010,6 +2010,7 @@ void sched_init_numa(int offline_node)
>        */
>       tl[i++] = (struct sched_domain_topology_level){
>           .mask = sd_numa_mask,
> +        .flags = SDTL_OVERLAP,
>           .numa_level = 0,
>           SD_INIT_NAME(NODE)
>       };
> -- 

And this solution is wrong too! Leon, could you please try the below diff
and let me know if it solves the issue in your case:

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index a2a38e1b6f18..e106035d78d8 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -2426,6 +2426,14 @@ static bool topology_span_sane(const struct cpumask *cpu_map)
  		cpumask_clear(covered);
  		cpumask_clear(id_seen);
  
+#ifdef CONFIG_NUMA
+		/*
+		 * Reset sched_domains_curr_level since tl->mask(cpu)
+		 * below can resolve to sd_numa_mask() for NODE.
+		 */
+		sched_domains_curr_level = tl->numa_level;
+#endif
+
  		/*
  		 * Non-NUMA levels cannot partially overlap - they must be either
  		 * completely equal or completely disjoint. Otherwise we can end up
---

We can reset "sched_domains_curr_level" to 0 before the loop and that
should work too since all numa levels >= 1 have SDTL_OVERLAP set but
this is just to err on the side of caution.

Previously, topology_span_sane() used the sched_domain_span() which
didn't depend on "sched_domains_curr_level" to resolve the tl->mask()
but since the rework uses tl directly now, this is needed.

-- 
Thanks and Regards,
Prateek