From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Xin Zhao <jackzxcui1989@163.com>, <mingo@redhat.com>,
<peterz@infradead.org>, <juri.lelli@redhat.com>,
<vincent.guittot@linaro.org>, <dietmar.eggemann@arm.com>,
<rostedt@goodmis.org>, <bsegall@google.com>, <mgorman@suse.de>,
<vschneid@redhat.com>
Cc: <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH RESEND 01/10] sched/fair: Do not set_rd_overloaded() if rd->online != env->cpus
Date: Thu, 10 Sep 2026 14:00:43 +0530 [thread overview]
Message-ID: <7735bbdb-2465-4ed3-9486-be9b82f3f42a@amd.com> (raw)
In-Reply-To: <20260910042950.1619727-2-jackzxcui1989@163.com>
Hello Xin,
On 9/10/2026 9:59 AM, Xin Zhao wrote:
> In update_sg_lb_stats(), it only traverses sched_group that belongs to
> env->cpus, but env->cpus may not necessarily equal rd->online. This can
> lead to the incorrect clearing of the overloaded flag of rd. For example,
> if cpuA belongs to the online CPU mask of the rd but does not belong to
> env->cpus, and cpuA consistently maintains nr_running >= 2, while other
> CPUs in rd->online keep rq->nr_running <= 1, the overloaded flag of rd
> will not be set until next update of update_sd_lb_stats() for that rd.
> During this period, sched_balance_newidle() will prematurely return due to
> the incorrect assumption that the rd is in a non-overloaded state.
Only two cases manipulate env.cpus:
1. LBF_DST_PINNED: The CPU doing the load balancing clears itself from
env.cpus since pinned tasks cannot be moved to it and goes to
"more_balance" but "more_balance" does not recompute stats and never
reaches update_sd_lb_stats().
2. LBF_ALL_PINNED: CPU with no movable task is cleared from env.cpus.
How will rd->overload being set for a CPU that cannot be helped make
newidle balance any more efficient?
Since LBF_ALL_PINNED is known with busiest's rq_lock held, maybe you
can set a rq->flag and later consume it in add_nr_running() to
do set_rd_overloaded() selectively.
Now a combination of (1) -> (2) -> redo can actually leave the original
"dest_cpu" out of the env.cpus which might be problematic.
Vincent, do you know why we clear the original dest_cpu (the CPU doing
load balancing) from "env.cpus" in LBF_DST_PINNED?
We update the destination to env.new_dst_cpu, "busiest" is till the same
and instead of moving load from A -> B, we are moving it from A -> C.
Later, if we do a "redo", B can still be a valid target for a different
busiest CPU with movable tasks right?
>
> In update_sd_lb_stats(), add a check to verify whether rd->online is equal
> to env->cpus before calling set_rd_overloaded() to avoid such incorrect
> settings.
>
> Signed-off-by: Xin Zhao <jackzxcui1989@163.com>
> ---
> kernel/sched/fair.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index dcf860c59a14..13e873b1ef58 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12679,8 +12679,12 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
> env->fbq_type = fbq_classify_group(&sds->busiest_stat);
>
> if (!env->sd->parent) {
> - /* update overload indicator if we are at root domain */
> - set_rd_overloaded(env->dst_rq->rd, sg_overloaded);
> + /*
> + * Update overload indicator if we are at root domain.
> + * Note that env->cpus may change during sched_balance_rq().
> + */
> + if (cpumask_equal(env->dst_rq->rd->online, env->cpus))
> + set_rd_overloaded(env->dst_rq->rd, sg_overloaded);
>
> /* Update over-utilization (tipping point, U >= 0) indicator */
> set_rd_overutilized(env->dst_rq->rd, sg_overutilized);
--
Thanks and Regards,
Prateek
next prev parent reply other threads:[~2026-09-10 8:31 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 4:29 [RFC PATCH RESEND 00/10] sched/fair: A series of load balance patches to improve real-time performance of CFS tasks Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 01/10] sched/fair: Do not set_rd_overloaded() if rd->online != env->cpus Xin Zhao
2026-09-10 8:30 ` K Prateek Nayak [this message]
2026-09-10 13:45 ` Vincent Guittot
2026-09-11 1:06 ` Xin Zhao
2026-09-11 6:21 ` K Prateek Nayak
2026-09-12 1:46 ` Xin Zhao
2026-09-12 1:53 ` Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 02/10] scbed/fair: Remove duplicate check for busiest_cpu in active_load_balance_cpu_stop() Xin Zhao
2026-09-10 11:41 ` Kayra Cizmeci
2026-09-11 0:22 ` Xin Zhao
2026-09-11 9:20 ` Kayra Cizmeci
2026-09-10 4:29 ` [RFC PATCH RESEND 03/10] sched/fair: Clear active_balance at the end of active_load_balance_cpu_stop() Xin Zhao
2026-09-10 8:09 ` K Prateek Nayak
2026-09-10 14:15 ` Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 04/10] sched/fair: Add LB_PROMOTE feature to enhance real-time performance of fair tasks Xin Zhao
2026-09-11 12:32 ` Vincent Guittot
2026-09-12 4:28 ` Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 05/10] sched/fair: Introduce select_task_rq_fair_thin() to select rq when LB_PROMOTE Xin Zhao
2026-09-10 8:19 ` Vincent Guittot
2026-09-10 14:39 ` Xin Zhao
2026-09-10 15:31 ` Vincent Guittot
2026-09-10 15:56 ` Xin Zhao
2026-09-11 12:27 ` Vincent Guittot
2026-09-12 4:08 ` Xin Zhao
2026-09-10 12:00 ` Kayra Cizmeci
2026-09-10 14:56 ` Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 06/10] sched/fair: Modify active_load_balance_cpu_stop() to accommodate more scenarios Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 07/10] sched/fair: Trigger active balance if a CFS task is preempted when LB_PROMOTE Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 08/10] sched/fair: Do not check avg_idle to prematurely exit newly idle " Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 09/10] sched/fair: Not goto more_balance if newly idle and has pending task when LBF_NEED_BREAK Xin Zhao
2026-09-10 4:29 ` [RFC PATCH RESEND 10/10] sched/fair: Strive to find a task to migrate if newly idle when LB_PROMOTE Xin Zhao
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=7735bbdb-2465-4ed3-9486-be9b82f3f42a@amd.com \
--to=kprateek.nayak@amd.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=jackzxcui1989@163.com \
--cc=juri.lelli@redhat.com \
--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®