All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] sched/topology: Balance cluster static-key updates
@ 2026-10-21  4:38 Hui Su
  2026-10-21  5:28 ` Kayra Cizmeci
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Hui Su @ 2026-10-21  4:38 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot
  Cc: Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak, Barry Song, Tim Chen,
	Chen Yu, Yicong Yang, linux-kernel, Hui Su

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


^ permalink raw reply related	[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-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

end of thread, other threads:[~2026-11-06  9:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.