On 10/14/26 2:14 AM, Tim Chen wrote:
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
Yes.
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().
facepalm :/
- 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.
Yes. I doubt anyone wants to try this experiment on Power9.
But yes, functionally updating the flags is needed.
@srikar,
That is also the reason for having the disabled. By default on P9,
llc will be at CACHE domain. But, one may want to experiment with =smt
which i doubt anyone will ever do.
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
Yes. That is what i prefer at this moment too. It will be cleaner.
Only thing is, need to get the masks right in different domains.
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
