From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Li kunyu <likunyu@open-hieco.net>, <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: [PATCH] sched/fair: Distribute burst-woken tasks when rq depth reaches 3x average
Date: Thu, 5 Nov 2026 09:14:15 +0530 [thread overview]
Message-ID: <7269c4e1-1e58-4600-bd01-94dc92e0f2b7@amd.com> (raw)
In-Reply-To: <20261104091842.266273-1-likunyu@open-hieco.net>
On 11/4/2026 2:48 PM, Li kunyu wrote:
> +/*
> + * Check if a CPU's rq depth reaches 3x the system-wide average tracked
> + * in root_domain (scaled by SCHED_CAPACITY_SCALE, floored at 1 task).
> + */
> +static inline bool rq_burst_overloaded(int cpu)
> +{
> + struct rq *rq = cpu_rq(cpu);
> + unsigned long avg;
> +
> + if (unlikely(!rq->rd))
> + return false;
> +
> + avg = max_t(unsigned long, READ_ONCE(rq->rd->avg_nr_running),
> + SCHED_CAPACITY_SCALE);
> +
> + return (READ_ONCE(rq->nr_running) << SCHED_CAPACITY_SHIFT) >= avg * 3;
You can just store (avg * 3) >> SCHED_CAPACITY_SHIFT in rd and skip
this math all the time.
> +}
> +
> +static inline void reset_periodic_lb_interval(int cpu)
> +{
> + struct sched_domain *sd;
> +
> + /*
> + * Only the most basic values are updated here to avoid excessive
> + * cost.
> + */
> + for_each_domain(cpu, sd) {
> + if (READ_ONCE(sd->balance_interval) > sd->min_interval)
> + WRITE_ONCE(sd->balance_interval, sd->min_interval);
Isn't 'sd->balance_interval' only stretched for LBF_ALL_PINNED cases?
Why does this make a difference?
> + }
> +}
> +
> /*
> * select_task_rq_fair: Select target runqueue for the waking task in domains
> * that have the relevant SD flag set. In practice, this is SD_BALANCE_WAKE,
> @@ -9972,11 +10004,19 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
>
> /* Slow path */
> if (unlikely(sd))
> - return sched_balance_find_dst_cpu(sd, p, cpu, prev_cpu, sd_flag);
> -
> + new_cpu = sched_balance_find_dst_cpu(sd, p, cpu, prev_cpu, sd_flag);
> /* Fast path */
> - if (wake_flags & WF_TTWU)
> - return select_idle_sibling(p, prev_cpu, new_cpu);
> + else if (wake_flags & WF_TTWU)
> + new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
Why even do select_idle_sibling() if you are not going to use that
data? Can't we check rq_burst_overloaded() beforehand?
> +
> + if (unlikely(rq_burst_overloaded(new_cpu))) {
> + int dist_cpu = cpumask_any_and_distribute(p->cpus_ptr, cpu_active_mask);
> +
> + if (likely(dist_cpu < nr_cpumask_bits)) {
> + reset_periodic_lb_interval(new_cpu);
This runs for every successful selection? Is that right? Why?
> + return dist_cpu;
> + }
> + }
>
> return new_cpu;
> }
> @@ -11685,6 +11725,7 @@ struct sg_lb_stats {
> struct sd_lb_stats {
> struct sched_group *busiest; /* Busiest group in this sd */
> struct sched_group *local; /* Local group in this sd */
> + unsigned long total_nr_running; /* Sum of nr_running in this sd */
> unsigned long total_load; /* Total load of all groups in sd */
> unsigned long total_capacity; /* Total capacity of all groups in sd */
> unsigned long avg_load; /* Average load across all groups in sd */
> @@ -12964,6 +13005,7 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
> /* Now, start updating sd_lb_stats */
> sds->total_load += sgs->group_load;
> sds->total_capacity += sgs->group_capacity;
> + sds->total_nr_running += sgs->sum_nr_running;
>
> sum_util += sgs->group_util;
> sg = sg->next;
> @@ -12982,11 +13024,31 @@ 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) {
On a large, busy system, load balancing at topmost domain is only
done once ever couple seconds and keeping the 'avg_nr_running' the
same for that long seems dodgy.
An overshoot shouldn't be worse than what it is now, and an
undershoot would at-least wait till 3 tasks on the CPU so it should
be fine?
> + unsigned int weight = env->sd->span_weight;
> +
> /* update overload indicator if we are at root domain */
> 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);
> +
> + /*
> + * We update the average task count at the top-level (root)
> + * domain, as this incurs the least overhead and comes
> + * closest to the ideal scenario. The trade-off is that we
> + * must accept some temporary data inaccuracy.
> + */
> + if (weight) {
> + unsigned long cur_avg;
> + unsigned long old_avg;
> +
> + cur_avg = (sds->total_nr_running << SCHED_CAPACITY_SHIFT) / weight;
> + old_avg = READ_ONCE(env->dst_rq->rd->avg_nr_running);
> + if (old_avg)
> + cur_avg = (old_avg * 7 + cur_avg) >> 3;
Is this an open-coded update_avg()?
> +
> + WRITE_ONCE(env->dst_rq->rd->avg_nr_running, cur_avg);
Can you do this selectively when old_avg != new_avg?
Also since you compare it with,
READ_ONCE(rq->nr_running) << SCHED_CAPACITY_SHIFT
Checking if (cur_avg * 3) >> SCHED_CAPACITY_SHIFT has changed and caching that
would be better and save on computation during wakeup.
> + }
> } else if (sg_overutilized) {
> set_rd_overutilized(env->dst_rq->rd, sg_overutilized);
> }
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index e656c7059bf8..4147e043407c 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1077,6 +1077,9 @@ struct root_domain {
> cpumask_var_t rto_mask;
> struct cpupri cpupri;
>
> + /* Scaled (SCHED_CAPACITY_SCALE) average per-CPU rq depth */
> + unsigned long avg_nr_running;
> +
> /*
> * NULL-terminated list of performance domains intersecting with the
> * CPUs of the rd. Protected by RCU.
--
Thanks and Regards,
Prateek
next prev parent reply other threads:[~2026-11-05 3:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 9:18 Li kunyu
2026-11-04 9:34 ` Li kunyu
2026-11-04 18:10 ` Kayra Cizmeci
2026-11-05 1:43 ` Li kunyu
2026-11-05 3:44 ` K Prateek Nayak [this message]
2026-11-05 11:24 ` 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=7269c4e1-1e58-4600-bd01-94dc92e0f2b7@amd.com \
--to=kprateek.nayak@amd.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=likunyu@open-hieco.net \
--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®