Re: [PATCH 0/4] powerpc/topology: Introduce llc_override debugfs

On 10/14/26 2:14 AM, Tim Chen wrote:

On Tue, 2026-10-13 at 15:28 +0530, Shrikanth Hegde wrote:

quoted

[snip]

quoted

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().

quoted

   
   #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 :/

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.

quoted

   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.

quoted

   		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