Re: [BUG] sched/cache: "Make LLC id continuous" causes NULL cpumask

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 5/27/26 11:37 PM, Shrikanth Hegde wrote:
> Hi Chen, Prateek.
> 
> On 5/27/26 9:35 PM, Chen, Yu C wrote:
>> Hi Shrikanth,
>>
>> On 5/27/2026 3:01 PM, Shrikanth Hegde wrote:
>>> Hi Chen, Prateek.
>>>
>>> I got back to work today, sorry for delay.
>>> I am trying to go through the mails.
>>> Apologies in case i have missed any bits.
>>>
>>
>> Thanks for taking a look at this!
>>
>>> On 5/26/26 7:38 PM, Chen Yu wrote:
>>>> Hi Prateek,
>>>>
>>>> On Tue, 26 May 2026 11:23:59 +0530, K Prateek Nayak 
>>>> <kprateek.nayak@amd.com> wrote:
>>>>> Hello Srikar,
>>>>>
>>>>> On 5/26/2026 10:28 AM, Srikar Dronamraju wrote:
>>>>>> L2 Cache reported here is for SMT8 Core aka CACHE domain.
>>>>>
>>>>> Apart for the scheduler, nothing in tree currently cares about
>>>>> cpu_coregroup_mask() except for drivers/base/arch_topology.c but
>>>>> Power doesn't select GENERIC_ARCH_TOPOLOGY.
>>>>>
>>>>> Why can't Power have an internal mask for MC domain (tl_mc_mask) and
>>>>> the scheduler can use cpu_coregroup_mask() for the actual LLc? (The L2
>>>>> mask in this case.)
>>>
>>> This seems wrong. there is no notion that coregroup_mask
>>> (MC domain) has to point at LLC domain.
>>>
>>> For example, on Shared LPAR, there is no MC domain and LLC is at SMT 
>>> core level.
>>> In that case coregroup_mask has point at SMT mask is wrong.
>>>
>>
>> On Shared LPAR, highest_flag_domain(SD_SHARE_LLC) selected the
>> SMT domain(L2 shared)prior to commit b5ea300a17e3.
>> Prateek suggested changing cpu_coregroup_mask() to use
>> cpu_l2_cache_mask(), which makes the LLC mask cover the same range.
>> sd_llc, size and grouping remain unchanged. Only sd_llc_id becomes
>> contiguous, which aligns with the intent of this commit.
>>
>> But yes, the naming is confusing. cpu_coregroup_mask suggests a
>> "group of cores", but after the change, it only covers threads
>> within a single SMT core.
>>
> 
> Yes. Though it might achieve the same effect, keeping it explicit may
> help in maintaining it better.
> 
> On PowerPC, there are these subtleties of Shared LPAR where MC domain
> per se doesn't exit etc, and on power9 and earlier has different 
> topologies. Maybe one could figure it all out and simplifies them into
> a few masks. But yes, it is slightly different as of today.
> 
>>> If we need a mask to point to the LLC mask which arch has to return, 
>>> then we would
>>> need a new api say cpu_llc_mask ? that can point accordingly.
>>>
>>
>> Do you mean something like this?
>> https://lore.kernel.org/lkml/8d14c844-b4a8-4af6- 
>> acab-2cfdd42225be@intel.com/
> 
> Yes. This is something i prefer, but not to make it point to l2 mask 
> always. See below diff which now make it boot on Shared Processor LPAR
> where i see panic without it.
> 
> Need to add proper comments where appropriate.
> 
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/ 
> asm/topology.h
> index 66ed5fe1b718..bd1db3b1dbb0 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -131,6 +131,9 @@ static inline int cpu_to_coregroup_id(int cpu)
>   #ifdef CONFIG_SMP
>   #include <asm/cputable.h>
> 
> +const struct cpumask *arch_llc_mask(int cpu);
> +#define arch_llc_mask  arch_llc_mask
> +
>   struct cpumask *cpu_coregroup_mask(int cpu);
>   const struct cpumask *cpu_die_mask(int cpu);
>   int cpu_die_id(int cpu);
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 3467f86fd78f..26c15c786c55 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1101,6 +1101,13 @@ const struct cpumask *cpu_die_mask(int cpu)
>   }
>   EXPORT_SYMBOL_GPL(cpu_die_mask);
> 
> +const struct cpumask *arch_llc_mask(int cpu)
> +{
> +       if (has_coregroup_support())
> +               return cpu_coregroup_mask(cpu);
> +       return cpu_smallcore_mask(cpu);


This function body needs change, since LLC is not at MC.
and I didn't account for power9.

Rest of the structure is what i would prefer the direction to go.
This will help future architectures too to account for their specific
needs.

What do you think?

> +}
> +
>   int cpu_die_id(int cpu)
>   {
>          if (has_coregroup_support())
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index df2ceb54c970..3b5155121276 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -2063,7 +2063,11 @@ const struct cpumask *tl_mc_mask(struct 
> sched_domain_topology_level *tl, int cpu
>          return cpu_coregroup_mask(cpu);
>   }
> 
> +#ifndef arch_llc_mask
>   #define llc_mask(cpu) cpu_coregroup_mask(cpu)
> +#else
> +#define llc_mask(cpu) arch_llc_mask(cpu)
> +#endif
> 
>   #else
>   #define llc_mask(cpu) cpumask_of(cpu)
> 
> (One more subtlety; crash would be seen only with NR_CPUS=8192 as 
> CPUMASK_OFFSTACK=y, but that's a different concern altogether.)
> 
> 
>>
>>> I don't like mixing MC domain and LLC into one bit.
>>>
>>
>> [ ... ]
>>
>>>>   struct cpumask *cpu_coregroup_mask(int cpu)
>>>>   {
>>>> -    return per_cpu(cpu_coregroup_map, cpu);
>>>> +    return cpu_l2_cache_mask(cpu);
>>>> +}
>>>
>>> This looks wrong to me too. In different hardware topologies
>>> there maybe distinction between coregroup and l2 mask.
>>>
>>> Let me go through the code and see if there is better way.
>>>
>>
>> Sure, please go ahead - I'm on board with the direction
>> you settle on.
>>
>> thanks,
>> Chenyu
>>
>