Re: [RFC PATCH 3/6] sched/fair: SMT sibling exclusion after successful push

[Li kunyu]

On 2026/10/21 20:39, Peter Zijlstra wrote:
> On Tue, Oct 20, 2026 at 04:37:43PM +0800, Li kunyu wrote:
> 
>> @@ -13377,7 +13379,20 @@ static struct rq *sched_balance_find_src_rq(struct lb_env *env,
>>  	struct sched_domain __maybe_unused *sd_tmp;
>>  	unsigned int busiest_nr = 0;
>>  	int __maybe_unused dst_llc;
>> -	int i;
>> +	int i, smt_cpu, dst_cpu = env->dst_cpu;
>> +
>> +	if (sched_feat(SMT_BUDDY_ASSIST) && env->is_smt_move_task) {
>> +		/*
>> +		 * Remove the SMT sibling CPU from the mask to avoid the
>> +		 * extremely rare case of pulling tasks from it.
> 
> Your comment seems to think the SMT mask is only ever 2 cpus, we have a
> ton of architectures where this is not true.
> 
>> +		 */
>> +		for_each_cpu(smt_cpu, cpu_smt_mask(dst_cpu)) {
>> +			if (smt_cpu == dst_cpu)
>> +				continue;
>> +
>> +			cpumask_clear_cpu(smt_cpu, env->cpus);
>> +		}
> 
> This is terrifyingly horrible. You're using a loop and atomic bit clear
> to clear bits in a local cpumask.
> 
> 		cpumask_andnot(env->cpus, env->cpus, cpu_smt_mask(dst_cpu));
> 		__cpumask_set_cpu(dst_cpu, env->cpus);
> 

I have rewritten the code. I moved the clearing of the CPU in the mask to before
the traversal of the busiest scheduling group and adopted your clearing method.
The new code is as follows:

 static int sched_balance_rq(int this_cpu, struct rq *this_rq,
                        struct sched_domain *sd, enum cpu_idle_type idle,
-                       int *continue_balancing)
+                       int *continue_balancing, bool is_smt_move_task)
 {
        int ld_moved, cur_ld_moved, active_balance = 0;
        struct sched_domain *sd_parent = sd->parent;
@@ -13784,6 +13784,7 @@ static int sched_balance_rq(int this_cpu, struct rq
*this_rq,
                .tasks          = LIST_HEAD_INIT(env.tasks),
        };
        bool need_unlock = false;
+       bool is_smt_clean_cpu = is_smt_move_task;

        cpumask_and(cpus, sched_domain_span(sd), cpu_active_mask);

@@ -13803,6 +13804,15 @@ static int sched_balance_rq(int this_cpu, struct rq
*this_rq,
                need_unlock = true;
        }

+       /*
+        * Remove the SMT sibling CPU from the mask to avoid the extremely
+        * rare case of pulling tasks from it.
+        */
+       if (sched_feat(SMT_BUDDY_ASSIST) && is_smt_clean_cpu) {
+               cpumask_andnot(cpus, cpus, cpu_smt_mask(this_cpu));
+               is_smt_clean_cpu = false;
+       }
+
        group = sched_balance_find_src_group(&env);
        if (!group) {
                schedstat_inc(sd->lb_nobusyg[idle]);



-- 
thanks,
kunyu