mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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


  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®