Re: [BUG] sched/cache: "Make LLC id continuous" causes NULL cpumask dereference in build_sched_domains on POWER9

["Chen, Yu C" <yu.c.chen@intel.com>]

On 5/26/2026 12:16 AM, K Prateek Nayak wrote:
> Hello Chenyu, Venkat,
> 
> On 5/25/2026 9:05 PM, Chen, Yu C wrote:
>> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
>> index 3467f86fd78f..cf6c2e4190ab 100644
>> --- a/arch/powerpc/kernel/smp.c
>> +++ b/arch/powerpc/kernel/smp.c
>> @@ -1042,11 +1042,6 @@ static const struct cpumask *tl_smallcore_smt_mask(struct sched_domain_topology_
>>   }
>>   #endif
>>
>> -struct cpumask *cpu_coregroup_mask(int cpu)
>> -{
>> -       return per_cpu(cpu_coregroup_map, cpu);
>> -}
>> -
>>   static bool has_coregroup_support(void)
>>   {
>>          /* Coregroup identification not available on shared systems */
>> @@ -1056,6 +1051,14 @@ static bool has_coregroup_support(void)
>>          return coregroup_enabled;
>>   }
>>
>> +struct cpumask *cpu_coregroup_mask(int cpu)
>> +{
>> +       if (!has_coregroup_support())
>> +               return cpu_l2_cache_mask(cpu);
>> +> +       return per_cpu(cpu_coregroup_map, cpu);
> 
> Interestingly, on powerpc, the MC domain doesn't have SD_SHARE_LLC flag
> set but I believe there is still some benefit of keeping the tasks on
> the same hemisphere?
> 
You are right. I guess power9 reported here does not have hemisphere and
power10 has, according to commit fb2ff9fa72e2:
"From Power10 processors onwards, each chip has 2 hemispheres"
but yes on both power9 and power10, MC domain doesn't have SD_SHARE_LLC
thus aggregating threads to 1 L2 domain might bring benefit. A side note is
that, cache aware scheduling is disabled on power for now because
power does not use the generic cacheinfo framework, thus its cache size
is returned as 0 so get_effective_llc_bytes() returns 0(for now, unless
we get the help from IBM friends to support it)
commit 7030513a0877 ("7030513a0877")

> If we are actually aiming for LLC, I think cpu_l2_cache_mask() is the
> right cpumask for all cases since tl_cache_mask() also returns that
> and the l2_cache_mask is set in all cases covered by update_mask_by_l2()
> in the same file.
> 
> If consolidation on hemisphere is beneficial, then the above diff
> looks correct.
> 
> Note: For has_coregroup_support(), with the above fix, the scheduler
> side llc_id will now be based on MC domain's span which seems wrong
> since it doesn't have SD_SHARE_LLC flag and might lead to other
> behavioral changes now.
> 

You are right, it seems to be a bug when has_coregroup_support() is enabled.
Maybe we can always return l2 id for power?

How about this(revert previous diff):
diff --git a/arch/powerpc/include/asm/topology.h 
b/arch/powerpc/include/asm/topology.h
index 66ed5fe1b718..3b3b4156f418 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -130,13 +130,15 @@ static inline int cpu_to_coregroup_id(int cpu)

  #ifdef CONFIG_SMP
  #include <asm/cputable.h>
+#include <asm/smp.h>

  struct cpumask *cpu_coregroup_mask(int cpu);
  const struct cpumask *cpu_die_mask(int cpu);
  int cpu_die_id(int cpu);

+#define arch_llc_mask(cpu)     cpu_l2_cache_mask(cpu)
+
  #ifdef CONFIG_PPC64
-#include <asm/smp.h>

  #define topology_physical_package_id(cpu)      (cpu_to_chip_id(cpu))
  #define topology_sibling_cpumask(cpu) 
(per_cpu(cpu_sibling_map, cpu))
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index df2ceb54c970..6772eb0ce493 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -2063,12 +2063,18 @@ const struct cpumask *tl_mc_mask(struct 
sched_domain_topology_level *tl, int cpu
         return cpu_coregroup_mask(cpu);
  }

-#define llc_mask(cpu) cpu_coregroup_mask(cpu)
+#ifndef arch_llc_mask
+#define arch_llc_mask(cpu) cpu_coregroup_mask(cpu)
+#endif

  #else
-#define llc_mask(cpu) cpumask_of(cpu)
+#ifndef arch_llc_mask
+#define arch_llc_mask(cpu) cpumask_of(cpu)
+#endif
  #endif

+#define llc_mask(cpu) arch_llc_mask(cpu)
+
  const struct cpumask *tl_pkg_mask(struct sched_domain_topology_level 
*tl, int cpu)
  {
         return cpu_node_mask(cpu);


thanks,
Chenyu

>> +}
>> +
>>   static int __init init_big_cores(void)
>>   {
>>          int cpu;
>