* Re: [PATCH v2] sched/topology: Balance cluster static-key updates
2026-10-21 4:38 [PATCH v2] sched/topology: Balance cluster static-key updates Hui Su
@ 2026-10-21 5:28 ` Kayra Cizmeci
2026-10-21 18:19 ` Tim Chen
2026-11-06 9:50 ` [tip: sched/urgent] " tip-bot2 for Hui Su
2 siblings, 0 replies; 5+ messages in thread
From: Kayra Cizmeci @ 2026-10-21 5:28 UTC (permalink / raw)
To: Hui Su
Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Barry Song, Tim Chen,
Chen Yu, Yicong Yang, linux-kernel
On Wed, Oct 21, 2026 at 01:38:34PM +0900, Hui Su wrote:
> build_sched_domains() increments sched_cluster_active once per newly
> built partition that contains an SD_CLUSTER domain.
> detach_destroy_domains() instead tested the global static key before
> decrementing it, so destroying a cluster-less partition could drop a
> reference owned by another partition and disable cluster-aware
> scheduling while SD_CLUSTER domains remained installed.
>
> Check the installed domains before detaching them, only while the key is
> enabled, and decrement only when the removed partition contains an
> SD_CLUSTER domain.
>
> With two cluster-bearing partitions installed, removing a singleton
> partition left the baseline count at 1 where 2 partitions still
> carried SD_CLUSTER; the fixed kernel correctly reports 2. On an
> asymmetric cluster topology where the parent partition gains no
> cluster domain, the baseline count drops to 0 while a cluster
> partition remains installed; the fixed kernel keeps it at 1.
>
> Fixes: 8881e1639f1f ("sched/fair: Scan cluster before scanning LLC in wake-up path")
> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
>
> Changes in v2:
> - Guard the SD_CLUSTER domain scan with sched_cluster_active to avoid an
> unnecessary loop on systems without cluster domains. (K Prateek Nayak)
>
> v1: https://lore.kernel.org/r/20261021034314.864713-1-sh_def@163.com
>
> kernel/sched/topology.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 3dab0253976fb..21e3ab1af4c85 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -3314,13 +3314,25 @@ int __init sched_init_domains(const struct cpumask *cpu_map)
> static void detach_destroy_domains(const struct cpumask *cpu_map)
> {
> unsigned int cpu = cpumask_any(cpu_map);
> + bool has_cluster = false;
> int i;
>
> if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
> static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
>
> - if (static_branch_unlikely(&sched_cluster_active))
> - static_branch_dec_cpuslocked(&sched_cluster_active);
> + if (static_branch_unlikely(&sched_cluster_active)) {
> + rcu_read_lock();
> + for_each_cpu(i, cpu_map) {
> + if (lowest_flag_domain(i, SD_CLUSTER)) {
> + has_cluster = true;
> + break;
> + }
> + }
> + rcu_read_unlock();
> +
> + if (has_cluster)
> + static_branch_dec_cpuslocked(&sched_cluster_active);
> + }
>
> rcu_read_lock();
> for_each_cpu(i, cpu_map)
> --
> 2.55.0
>
LGTM.
Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] sched/topology: Balance cluster static-key updates
2026-10-21 4:38 [PATCH v2] sched/topology: Balance cluster static-key updates Hui Su
2026-10-21 5:28 ` Kayra Cizmeci
@ 2026-10-21 18:19 ` Tim Chen
2026-10-22 4:05 ` Hui Su
2026-11-06 9:50 ` [tip: sched/urgent] " tip-bot2 for Hui Su
2 siblings, 1 reply; 5+ messages in thread
From: Tim Chen @ 2026-10-21 18:19 UTC (permalink / raw)
To: Hui Su, Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, K Prateek Nayak, Barry Song, Chen Yu,
Yicong Yang, linux-kernel
On Wed, 2026-10-21 at 13:38 +0900, Hui Su wrote:
> build_sched_domains() increments sched_cluster_active once per newly
> built partition that contains an SD_CLUSTER domain.
> detach_destroy_domains() instead tested the global static key before
> decrementing it, so destroying a cluster-less partition could drop a
> reference owned by another partition and disable cluster-aware
> scheduling while SD_CLUSTER domains remained installed.
>
> Check the installed domains before detaching them, only while the key is
> enabled, and decrement only when the removed partition contains an
> SD_CLUSTER domain.
>
> With two cluster-bearing partitions installed, removing a singleton
> partition left the baseline count at 1 where 2 partitions still
> carried SD_CLUSTER; the fixed kernel correctly reports 2. On an
> asymmetric cluster topology where the parent partition gains no
> cluster domain, the baseline count drops to 0 while a cluster
> partition remains installed; the fixed kernel keeps it at 1.
Good catch, the inc/dec are indeed unbalanced.
>
> Fixes: 8881e1639f1f ("sched/fair: Scan cluster before scanning LLC in wake-up path")
> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
>
> Changes in v2:
> - Guard the SD_CLUSTER domain scan with sched_cluster_active to avoid an
> unnecessary loop on systems without cluster domains. (K Prateek Nayak)
>
> v1: https://lore.kernel.org/r/20261021034314.864713-1-sh_def@163.com
>
> kernel/sched/topology.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 3dab0253976fb..21e3ab1af4c85 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -3314,13 +3314,25 @@ int __init sched_init_domains(const struct cpumask *cpu_map)
> static void detach_destroy_domains(const struct cpumask *cpu_map)
> {
> unsigned int cpu = cpumask_any(cpu_map);
> + bool has_cluster = false;
> int i;
>
> if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
> static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
>
> - if (static_branch_unlikely(&sched_cluster_active))
> - static_branch_dec_cpuslocked(&sched_cluster_active);
> + if (static_branch_unlikely(&sched_cluster_active)) {
> + rcu_read_lock();
> + for_each_cpu(i, cpu_map) {
> + if (lowest_flag_domain(i, SD_CLUSTER)) {
> + has_cluster = true;
> + break;
> + }
> + }
> + rcu_read_unlock();
> +
> + if (has_cluster)
> + static_branch_dec_cpuslocked(&sched_cluster_active);
> + }
>
We should be able to combine with rcu_read_lock/unlock() and cpu_map walk
for cpu_attach_domain(NULL, &def_root_domain, i) below.
So we do the rcu and cpu_map walk operation once in this function
instead of doing it twice. Something like this?
unsigned int cpu = cpumask_any(cpu_map);
bool has_cluster = false;
int i;
if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
rcu_read_lock();
for_each_cpu(i, cpu_map) {
if (static_branch_unlikely(&sched_cluster_active) &&
lowest_flag_domain(i, SD_CLUSTER))
has_cluster = true;
cpu_attach_domain(NULL, &def_root_domain, i);
}
rcu_read_unlock();
if (has_cluster)
static_branch_dec_cpuslocked(&sched_cluster_active);
Moving the dec after the detach loop should be safe. It only say
that a cluster domain exists, and cluster code that will still test
for actual SD_CLUSTER domain flag on the relevant domain.
I will also suggest that this fix be backported to the stable kernels
(cc stable@vger.kernel.org).
Thanks.
Tim
> rcu_read_lock();
> for_each_cpu(i, cpu_map)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] sched/topology: Balance cluster static-key updates
2026-10-21 18:19 ` Tim Chen
@ 2026-10-22 4:05 ` Hui Su
0 siblings, 0 replies; 5+ messages in thread
From: Hui Su @ 2026-10-22 4:05 UTC (permalink / raw)
To: Tim Chen
Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann,
rostedt, bsegall, mgorman, vschneid, kprateek.nayak, baohua,
yu.c.chen, yangyicong, linux-kernel
> We should be able to combine with rcu_read_lock/unlock() and cpu_map walk
> for cpu_attach_domain(NULL, &def_root_domain, i) below.
> So we do the rcu and cpu_map walk operation once in this function
> instead of doing it twice.
>
> Moving the dec after the detach loop should be safe. It only say
> that a cluster domain exists, and cluster code that will still test
> for actual SD_CLUSTER domain flag on the relevant domain.
>
> I will also suggest that this fix be backported to the stable kernels
> (cc stable@vger.kernel.org).
Hi Tim,
Agreed. Combining the cluster check with the existing detach loop avoids
the extra CPU mask walk and RCU read-side section.
I'll make this change in v3, move the static-key decrement after the
detach loop, and add Cc: stable@vger.kernel.org as suggested.
Thanks for the review!
Hui
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip: sched/urgent] sched/topology: Balance cluster static-key updates
2026-10-21 4:38 [PATCH v2] sched/topology: Balance cluster static-key updates Hui Su
2026-10-21 5:28 ` Kayra Cizmeci
2026-10-21 18:19 ` Tim Chen
@ 2026-11-06 9:50 ` tip-bot2 for Hui Su
2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Hui Su @ 2026-11-06 9:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: Hui Su, Peter Zijlstra (Intel), Ingo Molnar, K Prateek Nayak,
Kayra Cizmeci, x86, linux-kernel
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: 5ebdfe12ab96368d0339593129d4ce0bc2bd06f5
Gitweb: https://git.kernel.org/tip/5ebdfe12ab96368d0339593129d4ce0bc2bd06f5
Author: Hui Su <sh_def@163.com>
AuthorDate: Wed, 21 Oct 2026 13:38:34 +09:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 06 Nov 2026 10:04:57 +01:00
sched/topology: Balance cluster static-key updates
build_sched_domains() increments sched_cluster_active once per newly
built partition that contains an SD_CLUSTER domain.
detach_destroy_domains() instead tested the global static key before
decrementing it, so destroying a cluster-less partition could drop a
reference owned by another partition and disable cluster-aware
scheduling while SD_CLUSTER domains remained installed.
Check the installed domains before detaching them, only while the key is
enabled, and decrement only when the removed partition contains an
SD_CLUSTER domain.
With two cluster-bearing partitions installed, removing a singleton
partition left the baseline count at 1 where 2 partitions still
carried SD_CLUSTER; the fixed kernel correctly reports 2. On an
asymmetric cluster topology where the parent partition gains no
cluster domain, the baseline count drops to 0 while a cluster
partition remains installed; the fixed kernel keeps it at 1.
Fixes: 8881e1639f1f ("sched/fair: Scan cluster before scanning LLC in wake-up path")
Signed-off-by: Hui Su <sh_def@163.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>
Link: https://patch.msgid.link/20261021043834.1140951-1-sh_def@163.com
---
kernel/sched/topology.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 919d0fb..9d3a1b8 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -3332,13 +3332,25 @@ int __init sched_init_domains(const struct cpumask *cpu_map)
static void detach_destroy_domains(const struct cpumask *cpu_map)
{
unsigned int cpu = cpumask_any(cpu_map);
+ bool has_cluster = false;
int i;
if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
- if (static_branch_unlikely(&sched_cluster_active))
- static_branch_dec_cpuslocked(&sched_cluster_active);
+ if (static_branch_unlikely(&sched_cluster_active)) {
+ rcu_read_lock();
+ for_each_cpu(i, cpu_map) {
+ if (lowest_flag_domain(i, SD_CLUSTER)) {
+ has_cluster = true;
+ break;
+ }
+ }
+ rcu_read_unlock();
+
+ if (has_cluster)
+ static_branch_dec_cpuslocked(&sched_cluster_active);
+ }
rcu_read_lock();
for_each_cpu(i, cpu_map)
^ permalink raw reply related [flat|nested] 5+ messages in thread