Re: [PATCH v2] sched/topology: Balance cluster static-key updates

[Tim Chen <tim.c.chen@linux.intel.com>]

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)