From: "Li, Aubrey" <aubrey.li@linux.intel.com>
To: Peter Zijlstra <peterz@infradead.org>,
mgorman@techsingularity.net, vincent.guittot@linaro.org
Cc: linux-kernel@vger.kernel.org, mingo@redhat.com,
juri.lelli@redhat.com, valentin.schneider@arm.com,
qais.yousef@arm.com, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com,
tim.c.chen@linux.intel.com, benbjiang@gmail.com
Subject: Re: [RFC][PATCH 1/5] sched/fair: Fix select_idle_cpu()s cost accounting
Date: Tue, 15 Dec 2020 11:36:35 +0800 [thread overview]
Message-ID: <c4e31235-e1fb-52ac-99a8-ae943ee0de54@linux.intel.com> (raw)
In-Reply-To: <20201214170017.877557652@infradead.org>
On 2020/12/15 0:48, Peter Zijlstra wrote:
> We compute the average cost of the total scan, but then use it as a
> per-cpu scan cost when computing the scan proportion. Fix this by
> properly computing a per-cpu scan cost.
>
> This also fixes a bug where we would terminate early (!--nr, case) and
> not account that cost at all.
I'm a bit worried this may introduce a regression under heavy load.
The overhead of adding another cpu_clock() and calculation becomes
significant when sis_scan is throttled by nr.
I'm not sure if it's a good idea to not account the scan cost at all
when sis_scan is throttled, that is, remove the first cpu_clock() as
well. The avg scan cost remains the value when the system is not very
busy, and when the load comes down and span avg idle > span avg cost,
we account the cost again. This should make select_idle_cpu() a bit
faster when the load is very high.
Thanks,
-Aubrey
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> kernel/sched/fair.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6144,10 +6144,10 @@ static inline int select_idle_smt(struct
> static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int target)
> {
> struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> + int cpu, loops = 1, nr = INT_MAX;
> + int this = smp_processor_id();
> struct sched_domain *this_sd;
> u64 time;
> - int this = smp_processor_id();
> - int cpu, nr = INT_MAX;
>
> this_sd = rcu_dereference(*this_cpu_ptr(&sd_llc));
> if (!this_sd)
> @@ -6175,14 +6175,19 @@ static int select_idle_cpu(struct task_s
> }
>
> for_each_cpu_wrap(cpu, cpus, target) {
> - if (!--nr)
> - return -1;
> if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> break;
> +
> + if (loops >= nr) {
> + cpu = -1;
> + break;
> + }
> + loops++;
> }
>
> if (sched_feat(SIS_PROP)) {
> time = cpu_clock(this) - time;
> + time = div_u64(time, loops);
> update_avg(&this_sd->avg_scan_cost, time);
> }
>
>
>
next prev parent reply other threads:[~2020-12-15 3:38 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-14 16:48 [RFC][PATCH 0/5] select_idle_sibling() wreckage Peter Zijlstra
2020-12-14 16:48 ` [RFC][PATCH 1/5] sched/fair: Fix select_idle_cpu()s cost accounting Peter Zijlstra
2020-12-15 3:36 ` Li, Aubrey [this message]
2020-12-15 7:59 ` Peter Zijlstra
2020-12-15 11:45 ` Mel Gorman
2020-12-15 12:13 ` Li, Aubrey
2021-01-08 10:27 ` Mel Gorman
2021-01-08 13:01 ` Qais Yousef
2021-01-08 13:47 ` Mel Gorman
2021-01-08 13:41 ` Vincent Guittot
2021-01-08 14:40 ` Mel Gorman
2021-01-08 15:10 ` Vincent Guittot
2021-01-08 16:14 ` Mel Gorman
2021-01-11 14:36 ` Vincent Guittot
2021-01-11 15:58 ` Mel Gorman
2021-01-08 19:45 ` Peter Zijlstra
2021-01-09 14:12 ` Mel Gorman
2021-01-11 14:39 ` Vincent Guittot
2021-01-08 19:49 ` Peter Zijlstra
2021-01-11 14:52 ` Vincent Guittot
2021-01-08 20:21 ` Peter Zijlstra
2021-01-09 13:59 ` Mel Gorman
2020-12-14 16:48 ` [RFC][PATCH 2/5] sched/fair: Make select_idle_cpu() proportional to cores Peter Zijlstra
2020-12-23 13:31 ` Vincent Guittot
2020-12-14 16:48 ` [RFC][PATCH 3/5] sched/fair: Remove select_idle_smt() Peter Zijlstra
2020-12-14 16:48 ` [RFC][PATCH 4/5] sched/fair: Merge select_idle_core/cpu() Peter Zijlstra
2020-12-14 16:48 ` [RFC][PATCH 5/5] sched/fair: SIS_PROP the idle core scan Peter Zijlstra
2020-12-16 12:59 ` [RFC][PATCH 0/5] select_idle_sibling() wreckage Li, Aubrey
2020-12-16 18:07 ` Vincent Guittot
2020-12-23 13:23 ` Vincent Guittot
2021-01-04 15:40 ` Mel Gorman
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=c4e31235-e1fb-52ac-99a8-ae943ee0de54@linux.intel.com \
--to=aubrey.li@linux.intel.com \
--cc=benbjiang@gmail.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@techsingularity.net \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=qais.yousef@arm.com \
--cc=rostedt@goodmis.org \
--cc=tim.c.chen@linux.intel.com \
--cc=valentin.schneider@arm.com \
--cc=vincent.guittot@linaro.org \
/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;
as well as URLs for NNTP newsgroup(s).