From: Xuewen Yan <xuewen.yan94@gmail.com>
To: Pierre <pierre.gondois@arm.com>
Cc: Dietmar Eggemann <Dietmar.Eggemann@arm.com>,
Quentin Perret <qperret@google.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Steven Rostedt <rostedt@goodmis.org>,
Benjamin Segall <bsegall@google.com>,
Mel Gorman <mgorman@suse.de>,
Daniel Bristot de Oliveira <bristot@redhat.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Chunyan Zhang <zhang.lyra@gmail.com>,
Ryan Y <xuewyan@foxmail.com>
Subject: Re: [PATCH] sched/fair: use signed long when compute energy delta in eas
Date: Fri, 9 Apr 2021 10:20:52 +0800 [thread overview]
Message-ID: <CAB8ipk_J-W35+iZqsmAm+VO2=OUXRR3EsW6BuhYmvmBiPcHxMA@mail.gmail.com> (raw)
In-Reply-To: <d85a1db9-4680-1d31-d3e7-6a081f652c5e@arm.com>
Hi
>
> Hi,
> > Hi
> >
> > On Wed, Apr 7, 2021 at 10:11 PM Pierre <pierre.gondois@arm.com> wrote:
> > >
> > > Hi,
> > > > I test the patch, but the overflow still exists.
> > > > In the "sched/fair: Use pd_cache to speed up
> > find_energy_efficient_cpu()"
> > > > I wonder why recompute the cpu util when cpu==dst_cpu in
> > compute_energy(),
> > > > when the dst_cpu's util change, it also would cause the overflow.
> > >
> > > The patches aim to cache the energy values for the CPUs whose
> > > utilization is not modified (so we don't have to compute it multiple
> > > times). The values cached are the 'base values' of the CPUs, i.e. when
> > > the task is not placed on the CPU. When (cpu==dst_cpu) in
> > > compute_energy(), it means the energy values need to be updated instead
> > > of using the cached ones.
> > >
> > well, is it better to use the task_util(p) + cache values ? but in
> > this case, the cache
> > values may need more parameters.
>
> This patch-set is not significantly improving the execution time of
> feec(). The results we have so far are an improvement of 5-10% in
> execution time, with feec() being executed in < 10us. So the gain is not
> spectacular.
well, I meaned to cache all util value and compute energy with caches, when
(cpu==dst_cpu), use caches instead of updating util, and do not get
util with function:
"effective_cpu_util()", to compute util with cache.
I add more parameters into pd_cache:
struct pd_cache {
unsigned long util;
unsigned long util_est;
unsigned long util_cfs;
unsigned long util_irq;
unsigned long util_rt;
unsigned long util_dl;
unsigned long bw_dl;
unsigned long freq_util;
unsigned long nrg_util;
};
In this way, it can avoid util update while feec. I tested with it,
and the negative delta disappeared.
Maybe this is not a good method, but it does work.
>
> >
> > > You are right, there is still a possibility to have a negative delta
> > > with the patches at:
> > >
> > https://gitlab.arm.com/linux-arm/linux-power/-/commits/eas/next/integration-20210129
> > <https://gitlab.arm.com/linux-arm/linux-power/-/commits/eas/next/integration-20210129>
> > > Adding a check before subtracting the values, and bailing out in such
> > > case would avoid this, such as at:
> > > https://gitlab.arm.com/linux-arm/linux-pg/-/commits/feec_bail_out/
> > <https://gitlab.arm.com/linux-arm/linux-pg/-/commits/feec_bail_out/>
> > >
> > In your patch, you bail out the case by "go to fail", that means you
> > don't use eas in such
> > case. However, in the actual scene, the case often occurr when select
> > cpu for small task.
> > As a result, the small task would not select cpu according to the eas,
> > it may affect
> > power consumption?
> With this patch (bailing out), the percentage of feec() returning due to
> a negative delta I get are:
> on a Juno-r2, with 2 big CPUs and 4 CPUs (capacity of 383), with a
> workload running during 5s with task having a period of 16 ms and:
> - 50 tasks at 1%: 0.14%
> - 30 tasks at 1%: 0.54%
> - 10 tasks at 1%: < 0.1%
> - 30 tasks at 5%: < 0.1%
> - 10 tasks at 5%: < 0.1%
> It doesn't happen so often to me.If we bail out of feec(), the task will
> still have another opportunity in the next call. However I agree this
> can lead to a bad placement when this happens.
> >
> > > I think a similar modification should be done in your patch. Even though
> > > this is a good idea to group the calls to compute_energy() to reduce the
> > > chances of having updates of utilization values in between the
> > > compute_energy() calls,
> > > there is still a chance to have updates. I think it happened when I
> > > applied your patch.
> > >
> > > About changing the delta(s) from 'unsigned long' to 'long', I am not
> > > sure of the meaning of having a negative delta. I thing it would be
> > > better to check and fail before it happens instead.
> > >
> > > Regards
> > >
>
>
>
next prev parent reply other threads:[~2021-04-09 2:22 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-30 5:21 [PATCH] sched/fair: use signed long when compute energy delta in eas Xuewen Yan
2021-03-30 9:45 ` Quentin Perret
2021-04-01 18:07 ` Dietmar Eggemann
2021-04-06 10:59 ` Xuewen Yan
2021-04-07 14:11 ` Pierre
2021-04-08 5:41 ` Xuewen Yan
2021-04-08 9:33 ` Pierre
2021-04-09 2:20 ` Xuewen Yan [this message]
2021-04-12 10:26 ` Pierre Gondois
2021-04-12 10:52 ` Xuewen Yan
2021-04-12 17:14 ` Pierre Gondois
2021-04-13 1:50 ` Xuewen Yan
2021-04-13 10:58 ` Pierre Gondois
2021-04-13 11:59 ` Xuewen Yan
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='CAB8ipk_J-W35+iZqsmAm+VO2=OUXRR3EsW6BuhYmvmBiPcHxMA@mail.gmail.com' \
--to=xuewen.yan94@gmail.com \
--cc=Dietmar.Eggemann@arm.com \
--cc=bristot@redhat.com \
--cc=bsegall@google.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=pierre.gondois@arm.com \
--cc=qperret@google.com \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=xuewyan@foxmail.com \
--cc=zhang.lyra@gmail.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;
as well as URLs for NNTP newsgroup(s).