Re: [RFC PATCH 1/6] sched/fair: Add SMT sibling push in periodic load balance

[Li kunyu]

On 2026/10/21 20:34, Peter Zijlstra wrote:
> On Tue, Oct 20, 2026 at 04:37:41PM +0800, Li kunyu wrote:
> 
> Just a few remarks on coding style while I try and make sense of the
> whole thing. Don't go rush out a new version with this stuff fixed
> before you've seen feedback on the higher level stuff.
> 
>> @@ -14226,6 +14244,94 @@ update_newidle_cost(struct sched_domain *sd, u64 cost, unsigned int success)
>>  	return false;
>>  }
>>  
>> +/*
>> + * An exciting thing is that we have implemented an efficient push
>> + * function process.
>> + *
>> + * It is applied in cycle load balancing and nohz_kick process.
>> + * It can help us solve the problem of high workload of CFS tasks in
>> + * the RQ queue.
>> + */
>> +static bool smt_buddy_move_task(struct rq *src_rq)
>> +{
>> +	bool is_overload, is_move = false;
>> +	int src_cpu = cpu_of(src_rq), smt_cpu;
>> +	unsigned long src_cpu_runnable, smt_cpu_runnable;
>> +	struct sched_domain *smt_sd;
>> +	struct task_struct *p;
>> +	struct rq *smt_rq;
>> +	struct rq_flags rf;
>> +
>> +	if (!sched_feat(SMT_BUDDY_ASSIST) || !sched_smt_active())
>> +		goto exit;
>> +
>> +restart:
>> +	if (src_rq->nr_running < 2)
>> +		goto exit;
> Did this want to be:
> 
> 	while (src_rq->nr_running > 1) {
> 
>> +
>> +	rcu_read_lock();
> 	scoped_guard (rcu) {
> 
>> +
>> +	is_overload = false;
>> +	src_cpu_runnable = cpu_runnable(src_rq);
>> +	for_each_cpu(smt_cpu, cpu_smt_mask(src_cpu)) {
>> +		if (smt_cpu == src_cpu)
>> +			continue;
>> +
>> +		smt_cpu_runnable = cpu_runnable(cpu_rq(smt_cpu));
>> +		if ((src_cpu_runnable * 100 > smt_cpu_runnable * sysctl_smt_buddy_imbalance_pct)
>> +			&& cpu_active(smt_cpu)) {
> This is horrifically bad indenting. Operators go at the end of the
> previous line and then you indent to the level of whatever ( is active.
> If you're a vim user 'cino=(0' gets you that.
> 
>> +			is_overload = true;
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!is_overload) {
>> +		rcu_read_unlock();
>> +		goto exit;
> This pattern should scream at you for doing it wrong.
> 
>> +	}
>> +
>> +	smt_sd = lowest_flag_domain(src_cpu, SD_SHARE_CPUCAPACITY);
>> +	if (!smt_sd) {
>> +		rcu_read_unlock();
>> +		goto exit;
> And again.
> 
>> +	}
>> +
>> +	rcu_read_unlock();
> 	}
>> +
>> +	rq_lock_irqsave(src_rq, &rf);
> 	scoped_guard (irqsave) {
> 		scoped_guard (rq_lock, src_rq)
> 

I configured Vim as you suggested and made the necessary modifications. In
addition, I made the following changes:

Changed rq->nr_running to rq->cfs.h_nr_runnable and made related changes so that
only CFS tasks that are currently running or already queued are counted and
migrated.

Added a condition requiring the current CPU's task count to be greater than that
of its SMT sibling CPUs, so as to avoid migrating too many newly created tasks
that have not yet started running.

Added a migration type, migrate_smt_task, which skips a given SMT sibling CPU
(only for tasks that do not meet CPU affinity requirements), thereby enabling
complete migration and checking of all SMT sibling CPUs.

Thank you for your guidance. Future patches will be more comprehensive and
better justified.

-- 
thanks,
kunyu