linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Quentin Perret <quentin.perret@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Thara Gopinath <thara.gopinath@linaro.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Zhang Rui <rui.zhang@intel.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Amit Kachhap <amit.kachhap@gmail.com>,
	viresh kumar <viresh.kumar@linaro.org>,
	Javi Merino <javi.merino@kernel.org>,
	Eduardo Valentin <edubezval@gmail.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	"open list:THERMAL" <linux-pm@vger.kernel.org>,
	Ionela Voinescu <ionela.voinescu@arm.com>
Subject: Re: [RFC PATCH 0/7] Introduce thermal pressure
Date: Wed, 10 Oct 2018 11:36:25 +0100	[thread overview]
Message-ID: <20181010103623.ttjexasymdpi66lu@queper01-lin> (raw)
In-Reply-To: <CAKfTPtA+FWrMnerQhrNQhvmvVSK_S86da=8shpdET-807zZgVg@mail.gmail.com>

On Wednesday 10 Oct 2018 at 12:14:32 (+0200), Vincent Guittot wrote:
> On Wed, 10 Oct 2018 at 11:55, Quentin Perret <quentin.perret@arm.com> wrote:
> >
> > Hi Vincent,
> >
> > On Wednesday 10 Oct 2018 at 10:50:05 (+0200), Vincent Guittot wrote:
> > > The problem with reflecting directly the capping is that it happens
> > > far more often than the pace at which cpu_capacity_orig is updated in
> > > the scheduler.
> >
> > Hmm, how can you be so sure ? That most likely depends on the workload,
> > the platform and the thermal governor. Some platforms heat up slowly,
> > some quickly. The pace at which the thermal governor will change things
> > should depend on that I assume.
> >
> > > This means that at the moment when scheduler uses the
> > > value, it might not be correct anymore.
> >
> > And OTOH, when you remove a cap for example, it will take time before
> > the scheduler can see the newly available capacity if you need to wait
> > for the signal to decay. So you are using a wrong information too in
> > that scenario.
> 
> But we stay consistant with all other metrics. The same happen when a
> task decide to stay idle for a long time after some activity... You
> will have to wait for the signal to decay

Because you see that as a task going idle. You could also say that
removing the cap from a CPU is equivalent to migrating a task off that
CPU. In this case you should see the newly available cap pretty much
immediately.

Also, I feel like the whole thermal capping story could interest the
Intel folks who, IIRC, were discussing how to represent their 'boosted'
OPPs in the scheduler some time ago. You can see the Intel boost thing
as a cap I think -- some OPPs can be inaccessible in some case. I'd be
interested to see what's their take on this.

> > > Then, this value are also used
> > > when building the sched_domain and setting max_cpu_capacity which
> > > would also implies the rebuilt the sched_domain topology ...
> >
> > Wait what ? I thought the thermal cap was reflected in capacity_of, not
> > capacity_orig_of ... You need to rebuild the sched_domain in case of
> > thermal pressure ?

I can't locate where you need to do this in the series. Do you actually
need to rebuild the sd hierarchy ?

> > Hmm, let me have a closer look at the patches, I must have missed
> > something ...
> >
> > > The pace of changing the capping is to fast to reflect that in the
> > > whole scheduler topology
> >
> > That's probably true in some cases, but it'd be cool to have numbers to
> > back up that statement, I think.
> >
> > Now, if you do need to rebuild the sched domain topology every time you
> > update the thermal pressure, I think the PELT HL is _way_ too short for
> > that ... You can't rebuild the whole thing every 32ms or so. Or am I
> > misunderstanding something ?
> >
> > > > Thara, have you tried to experiment with a simpler implementation as
> > > > suggested by Ingo ?
> > > >
> > > > Also, assuming that we do want to average things, do we actually want to
> > > > tie the thermal ramp-up time to the PELT half life ? That provides
> > > > nice maths properties wrt the other signals, but it's not obvious to me
> > > > that this thermal 'constant' should be the same on all platforms. Or
> > > > maybe it should ?
> > >
> > > The main interest of using PELT signal is that thermal pressure will
> > > evolve at the same pace as other signals used in the scheduler.
> >
> > Right, I think this is a nice property too (assuming that we actually
> > want to average things out).
> >
> > > With
> > > thermal pressure, we have the exact same problem as with RT tasks. The
> > > thermal will cap the max frequency which will cap the utilization of
> > > the tasks running on the CPU
> >
> > Well, the nature of the signal is slightly different IMO. Yes it's
> > capacity, but you're no actually measuring time spent on the CPU. All
> > other PELT signals are based on time, this thermal thing isn't, so it is
> > kinda different in a way. And I'm still wondering if it could be helpful
> 
> hmmm ... it is based on time too.

You're not actually measuring the time spent on the CPU by the 'thermal
task'. There is no such thing as a 'thermal task'. You're just trying to
model things like that, but the thermal stuff isn't actually
interrupting running tasks to eat CPU cycles. It just makes thing run
slower, which isn't exactly the same thing IMO.

But maybe that's a detail.

> Both signals (current ones and thermal one) are really close. The main
> difference with other utilization signal is that instead of providing
> a running/not running boolean that is then weighted by the current
> capacity, the signal uses direclty the capped max capacity to reflect
> the amount of cycle that is stolen by thermal mitigation.
> 
> > to be able to have a different HL for that thermal signal. That would
> > 'break' the nice maths properties we have, yes, but is it a problem or is
> > it actually helpful to cope with the thermal characteristics of
> > different platforms ?
> 
> If you don't use the sign kind of signal with the same responsiveness,
> you will start to see some OPP toggles as an example when the thermal
> state change because one metrics will change faster than the other one
> and you can't have a correct view of the system. Same problem was
> happening with rt task.

Well, that wasn't the problem with rt tasks. The problem with RT tasks
was that the time they spend on the CPU wasn't accounted _at all_ when
selecting frequency for CFS, not that the accounting was at a different
pace ...

Thanks,
Quentin

  reply	other threads:[~2018-10-10 10:36 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20181009162509epcas1p4fdd2e23039caa24586a4a52c6d2e7336@epcas1p4.samsung.com>
2018-10-09 16:24 ` [RFC PATCH 0/7] Introduce thermal pressure Thara Gopinath
2018-10-09 16:24   ` [RFC PATCH 1/7] sched/pelt.c: Add option to make load and util calculations frequency invariant Thara Gopinath
2018-10-09 16:24   ` [RFC PATCH 2/7] sched/pelt.c: Add support to track thermal pressure Thara Gopinath
2018-10-09 16:24   ` [RFC PATCH 3/7] sched: Add infrastructure to store and update instantaneous " Thara Gopinath
2018-10-09 16:24   ` [RFC PATCH 4/7] sched: Initialize per cpu thermal pressure structure Thara Gopinath
2018-10-09 16:25   ` [RFC PATCH 5/7] sched/fair: Enable CFS periodic tick to update thermal pressure Thara Gopinath
2018-12-04 15:43     ` Vincent Guittot
2018-10-09 16:25   ` [RFC PATCH 6/7] sched/fair: update cpu_capcity to reflect " Thara Gopinath
2018-10-10  5:57     ` Javi Merino
2018-10-10 14:22       ` Thara Gopinath
2018-10-09 16:25   ` [RFC PATCH 7/7] thermal/cpu-cooling: Update thermal pressure in case of a maximum frequency capping Thara Gopinath
2018-10-10  5:44   ` [RFC PATCH 0/7] Introduce thermal pressure Javi Merino
2018-10-10 14:15     ` Thara Gopinath
2018-10-10  6:17   ` Ingo Molnar
2018-10-10  8:29     ` Quentin Perret
2018-10-10  8:50       ` Vincent Guittot
2018-10-10  9:55         ` Quentin Perret
2018-10-10 10:14           ` Vincent Guittot
2018-10-10 10:36             ` Quentin Perret [this message]
2018-10-10 12:04               ` Vincent Guittot
2018-10-10 12:23                 ` Juri Lelli
2018-10-10 12:34                   ` Vincent Guittot
2018-10-10 12:50                     ` Juri Lelli
2018-10-10 13:08                       ` Vincent Guittot
2018-10-10 13:34                         ` Juri Lelli
2018-10-10 13:38                           ` Vincent Guittot
2018-10-10 17:08                           ` Thara Gopinath
2018-10-10 13:11                       ` Quentin Perret
2018-10-10 13:05                 ` Quentin Perret
2018-10-10 13:27                   ` Vincent Guittot
2018-10-10 13:47                     ` Quentin Perret
2018-10-10 15:19                       ` Vincent Guittot
2018-10-10 16:15                       ` Ionela Voinescu
2018-10-10 17:03           ` Thara Gopinath
2018-10-10 15:43     ` Thara Gopinath
2018-10-16  7:33       ` Ingo Molnar
2018-10-16  9:28         ` Lukasz Luba
2018-10-17 16:21         ` Thara Gopinath
2018-10-18  6:48           ` Ingo Molnar
2018-10-18  7:08             ` Rafael J. Wysocki
2018-10-18  7:50               ` Ingo Molnar
2018-10-18  8:14                 ` Rafael J. Wysocki
2018-10-18  9:35                   ` [PATCH 1/2] sched/cpufreq: Reorganize the cpufreq files Daniel Lezcano
2018-10-18  9:35                     ` [PATCH 2/2] sched/cpufreq: Add the SPDX tags Daniel Lezcano
2018-10-18  9:42                     ` [PATCH 1/2] sched/cpufreq: Reorganize the cpufreq files Rafael J. Wysocki
2018-10-18  9:54                       ` Daniel Lezcano
2018-10-18 10:06                         ` Rafael J. Wysocki
2018-10-18 10:13                           ` Daniel Lezcano
2018-10-18  9:45                     ` Daniel Lezcano
2018-10-19  5:24                     ` kbuild test robot
2018-10-19  5:52                     ` kbuild test robot
2018-10-18  9:44                   ` [PATCH V2 " Daniel Lezcano
2018-10-18  9:44                     ` [PATCH V2 2/2] sched/cpufreq: Add the SPDX tags Daniel Lezcano
2018-10-18 16:17             ` [RFC PATCH 0/7] Introduce thermal pressure Thara Gopinath
2018-10-19  8:02               ` Ingo Molnar
2018-10-19 11:29                 ` Valentin Schneider
2018-10-10 15:35   ` Lukasz Luba
2018-10-10 16:54     ` Daniel Lezcano
2018-10-11  7:35       ` Lukasz Luba
2018-10-11  8:23         ` Daniel Lezcano
2018-10-12  9:37           ` Lukasz Luba
2018-10-10 17:30     ` Thara Gopinath
2018-10-11 11:10       ` Lukasz Luba
2018-10-16 17:11         ` Vincent Guittot
2018-10-17 16:24           ` Thara Gopinath
2018-10-18  8:00             ` Lukasz Luba
2018-10-18  8:12           ` Lukasz Luba

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=20181010103623.ttjexasymdpi66lu@queper01-lin \
    --to=quentin.perret@arm.com \
    --cc=amit.kachhap@gmail.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=edubezval@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ionela.voinescu@arm.com \
    --cc=javi.merino@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=thara.gopinath@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@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).