mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Li kunyu <likunyu@open-hieco.net>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, juri.lelli@redhat.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
	vschneid@redhat.com, kprateek.nayak@amd.com,
	linux-kernel@vger.kernel.org,
	"Li Wang (Hieco)" <li.wang@linux.dev>
Subject: Re: [RFC PATCH 1/6] sched/fair: Add SMT sibling push in periodic load balance
Date: Tue, 27 Oct 2026 19:10:08 +0800	[thread overview]
Message-ID: <291b154d-16e3-9eea-8d61-910cf1f4eb7c@open-hieco.net> (raw)
In-Reply-To: <20261021123438.GG2823843@noisy.programming.kicks-ass.net>


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

  reply	other threads:[~2026-10-27 11:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20  8:37 [RFC PATCH 0/6] sched/fair: Introduce the SMT_BUDDY_ASSIST feature Li kunyu
2026-10-20  8:37 ` [RFC PATCH 1/6] sched/fair: Add SMT sibling push in periodic load balance Li kunyu
2026-10-21 12:34   ` Peter Zijlstra
2026-10-27 11:10     ` Li kunyu [this message]
2026-10-20  8:37 ` [RFC PATCH 2/6] sched/fair: Add is_smt_buddy_run() to skip SMT domain Li kunyu
2026-10-21 12:35   ` Peter Zijlstra
2026-10-27 11:13     ` Li kunyu
2026-10-20  8:37 ` [RFC PATCH 3/6] sched/fair: SMT sibling exclusion after successful push Li kunyu
2026-10-21 12:39   ` Peter Zijlstra
2026-10-27 11:18     ` Li kunyu
2026-10-20  8:37 ` [RFC PATCH 4/6] sched/fair: Restrict newidle balance to LLC domain Li kunyu
2026-10-21 12:42   ` Peter Zijlstra
2026-10-28  2:16     ` Li kunyu
2026-10-20  8:37 ` [RFC PATCH 5/6] sched/fair: Add task burst threshold for large-scale bursts Li kunyu
2026-10-21 12:54   ` Peter Zijlstra
2026-10-28  2:54     ` Li kunyu
2026-10-20  8:37 ` [RFC PATCH 6/6] sched/docs: Add documentation for SMT Buddy Assist feature Li kunyu
2026-10-21 12:55   ` Peter Zijlstra
2026-10-21 12:44 ` [RFC PATCH 0/6] sched/fair: Introduce the SMT_BUDDY_ASSIST feature Kayra Cizmeci
2026-10-27 10:05   ` Li kunyu
2026-10-22  7:40 ` Peter Zijlstra
2026-10-22 10:08   ` [RFC PATCH 0/6] sched/fair: Introduce the SMT BUDDY ASSIST feature Li kunyu
2026-10-22 16:02     ` Chen Yu
2026-10-28  1:58       ` Li kunyu
2026-10-22 20:25 ` [RFC PATCH 0/6] sched/fair: Introduce the SMT_BUDDY_ASSIST feature Tim Chen
2026-10-27 10:44   ` Li kunyu
2026-10-27 20:59     ` Tim Chen
2026-10-28  3:39       ` Li kunyu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=291b154d-16e3-9eea-8d61-910cf1f4eb7c@open-hieco.net \
    --to=likunyu@open-hieco.net \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=li.wang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®