On Tue, 2026-10-13 at 15:28 +0530, Shrikanth Hegde wrote:
>
[snip]
>
> One way to fix it is with a change like below.
> But more thoughts after the patch.
>
> ---
> arch/powerpc/include/asm/topology.h | 7 +------
> arch/powerpc/kernel/smp.c | 23 +++++++++++++++++++++++
> include/linux/sched.h | 2 +-
> kernel/sched/topology.c | 7 +++++++
> 4 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/topology.h
> b/arch/powerpc/include/asm/topology.h
> index 44ec416fa489..f4555632e692 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -135,12 +135,7 @@ struct cpumask *cpu_coregroup_mask(int cpu);
> const struct cpumask *cpu_die_mask(int cpu);
> int cpu_die_id(int cpu);
>
> -/*
> - * Points to where the LLC is. On power9 this will point at CACHE
> - * domain, On others it will point to SMT domain. In all cases
> - * cpu_l2_cache_mask points to where LLC is
> - */
> -#define arch_llc_mask(cpu) cpu_l2_cache_mask(cpu)
> +const struct cpumask *arch_llc_mask(int cpu);
You will need to add
#define arch_llc_mask arch_llc_mask
here.
Otherwise kernel/sched/topology.c looks for a macro:
# ifndef arch_llc_mask
#define arch_llc_mask(cpu) cpu_coregroup_mask(cpu)
# endif
With the #define gone, the generic code falls back to
cpu_coregroup_mask().
>
> #ifdef CONFIG_PPC64
> #include <asm/smp.h>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 6a4ee7d3b8e5..139c54fc3e2d 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1864,6 +1864,28 @@ void __noreturn arch_cpu_idle_dead(void)
>
> #endif
>
> +/*
> + * Points to where the LLC is. On power9 this will point at CACHE
> + * domain, On others it will point to SMT domain. In all cases
> + * cpu_l2_cache_mask points to where LLC is.
> + *
> + * If there is llc_override debug knob, return appropriate mask
> + */
> +const struct cpumask *arch_llc_mask(int cpu)
> +{
> + switch (READ_ONCE(llc_override)) {
> + case LLC_OVERRIDE_SMT:
> + return cpu_smt_mask(cpu);
> + case LLC_OVERRIDE_MC:
> + return cpu_coregroup_mask(cpu);
> + case LLC_OVERRIDE_PKG:
> + return cpu_die_mask(cpu);
> + case LLC_OVERRIDE_DISABLED:
> + default:
> + return cpu_l2_cache_mask(cpu);
> + }
> +}
> +
The mask here has to match the span of the highest SD_SHARE_LLC
domain, and there seems to be some mismatches.
- PKG: On power pc, seems like cpu_die_mask() returns
cpu_coregroup_map when coregroups are
supported,
if (has_coregroup_support())
return per_cpu(cpu_coregroup_map, cpu);
else
return cpu_node_mask(cpu);
so this is the MC span while sd_llc sits at PKG. It
should be cpu_node_mask(), the same as tl_pkg_mask().
- SMT: patch 3 adds SD_SHARE_LLC only to MC/PKG, so for "smt" the
flags are as before. I think it is possible for P9 to have
shared_caches between two cores:
/*
* P9 has a slightly odd architecture where pairs of cores share an L2 cache.
* This topology makes it *much* cheaper to migrate tasks between adjacent
cores
* since the migrated task remains cache hot. We want to take advantage of
this
* at the scheduler level so an extra topology level is required.
*/
the CACHE level also has SD_SHARE_LLC, and highest_flag_domain() picks it, so
sd_llc spans
cpu_l2_cache_map. shared_caches is only set when that map is wider
than the core's threads, e.g. a P9 core pair sharing an L2:
CPU: 0 1 2 3 | 4 5 6 7
cpu_smt_mask(): core A | core B
sd_llc (CACHE): 0-7
The ids follow cpu_smt_mask(), so 0-3 and 4-7 get different ids
while sd_llc and sd_llc_size cover all eight, and
cpus_share_cache(0, 4) is false, which is incorrect.
> int arch_update_cpu_topology(void)
> {
> if (READ_ONCE(llc_override_changed)) {
> @@ -1923,6 +1945,7 @@ static ssize_t llc_override_set(struct file *file,
> const char __user *ubuf,
> pr_info("sched LLC override set to %s. Rebuilding sched
> domains\n",
> llc_override_str[override]);
>
> + sched_reset_llc_id();
The reset here seems racy without taking cpus_read_lock, and taking
sched_domains_mutex for each CPU and you could have a cpu hot plug event
or cpu set rebuild in between.
That's why I would go with the boot parameter. The llc ids are assigned once,
with the right mask, so there is nothing to reset or rebuild, no
change to kernel/sched/, and everything stays in arch/powerpc. Since
this is an experiment knob, needing a reboot to change it is a small
cost, and every run starts from a clean state.
Tim
> rebuild_sched_domains();
> }
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 7b91cae4a59c..4d7e2ee841a9 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2505,5 +2505,5 @@ extern void migrate_enable(void);
> #endif /* MODULE */
>
> DEFINE_LOCK_GUARD_0(migrate, migrate_disable(), migrate_enable())
> -
> +void sched_reset_llc_id(void);
> #endif
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index dd2b1d70ec9d..63c9bca4c5eb 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -3087,6 +3087,13 @@ void sched_domains_free_llc_id(int cpu)
> sched_domains_mutex_unlock();
> }
>
> +void sched_reset_llc_id(void)
> +{
> + int cpu;
> +
> + for_each_online_cpu(cpu)
> + sched_domains_free_llc_id(cpu);
> +}
> /*
> * Build sched domains for a given set of CPUs and attach the sched domains
> * to the individual CPUs
