linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Thara Gopinath <thara.gopinath@linaro.org>,
	Lukasz Luba <lukasz.luba@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH] base: arch_topology: Use policy->max to calculate freq_factor
Date: Thu, 2 Dec 2021 17:31:53 +0100	[thread overview]
Message-ID: <CAJZ5v0hRvsoEZj45OWe34uhAPj+J1rJWq5Wff4R0f_BYEuU5wA@mail.gmail.com> (raw)
In-Reply-To: <20211202105027.GA1180274@e123083-lin>

On Thu, Dec 2, 2021 at 11:50 AM Morten Rasmussen
<morten.rasmussen@arm.com> wrote:
>
> On Wed, Nov 17, 2021 at 06:59:05PM +0100, Rafael J. Wysocki wrote:
> > On Wed, Nov 17, 2021 at 6:01 PM Thara Gopinath
> > <thara.gopinath@linaro.org> wrote:
> > >
> > > Hi,
> > >
> > > On 11/17/21 7:49 AM, Rafael J. Wysocki wrote:
> > > > On Wed, Nov 17, 2021 at 11:46 AM Lukasz Luba <lukasz.luba@arm.com> wrote:
> > > >>
> > > >> Hi Rafael,
> > > >>
> > > >> On 11/16/21 7:05 PM, Rafael J. Wysocki wrote:
> > > >>> On Mon, Nov 15, 2021 at 9:10 PM Thara Gopinath
> > > >>> <thara.gopinath@linaro.org> wrote:
> > > >>>>
> > > >>>> cpuinfo.max_freq can reflect boost frequency if enabled during boot.  Since
> > > >>>> we don't consider boost frequencies while calculating cpu capacities, use
> > > >>>> policy->max to populate the freq_factor during boot up.
> > > >>>
> > > >>> I'm not sure about this.  schedutil uses cpuinfo.max_freq as the max frequency.
> > > >>
> > > >> Agree it's tricky how we treat the boost frequencies and also combine
> > > >> them with thermal pressure.
> > > >> We probably would have consider these design bits:
> > > >> 1. Should thermal pressure include boost frequency?
> > > >
> > > > Well, I guess so.
> > > >
> > > > Running at a boost frequency certainly increases thermal pressure.
> > > >
> > > >> 2. Should max capacity 1024 be a boost frequency so scheduler
> > > >>      would see it explicitly?
> > > >
> > > > That's what it is now if cpuinfo.max_freq is a boost frequency.
> > > >
> > > >> - if no, then schedutil could still request boost freq thanks to
> > > >>     map_util_perf() where we add 25% to the util and then
> > > >>     map_util_freq() would return a boost freq when util was > 1024
> > > >>
> > > >>
> > > >> I can see in schedutil only one place when cpuinfo.max_freq is used:
> > > >> get_next_freq(). If the value stored in there is a boost,
> > > >> then don't we get a higher freq value for the same util?
> > > >
> > > > Yes. we do, which basically is my point.
> > > >
> > > > The schedutil's response is proportional to cpuinfo.max_freq and that
> > > > needs to be taken into account for the results to be consistent.
> > >
> > > So IIUC, cpuinfo.max_freq is always supposed to be the highest supported
> > > frequency of a cpu, irrespective of whether boost is enabled or not.
> > > Where as policy->max is the currently available maximum cpu frequency
> > > which can be equal to cpuinfo.max_freq or lower (depending on whether
> > > boost is enabled, whether there is a constraint on policy->max placed by
> > > thermal etc).
> >
> > It may also depend on the limit set by user space.
> >
> > > So in this case isn't it better for schedutil to consider
> > > policy->max instead of cpuinfo.max ?
> >
> > Not really.
> >
> > In that case setting policy->max to 1/2 of cpuinfo.max_freq would
> > cause schedutil to choose 1/4 of cpuinfo.max_freq for 50% utilization
> > which would be rather unexpected.
> >
> > policy->max is a cap, not the current maximum capacity.
> >
> > > Like you mentioned above same
> > > utilization will relate to different frequencies depending on the
> > > maximum frequency.
> >
> > Which is not how it is expected (and defined) to work, though.
> >
> > If you really want to play with the current maximum capacity, you need
> > to change it whenever boost is disabled or enabled - and there is a
> > mechanism for updating cpufinfo.max_freq in such cases.
>
> I don't see why we would want to change max capacity on the fly. It is
> not a cheap operation as we would need to normalize the capacity for all
> CPUs if the CPU(s) with capacity = 1024 changes its capacity. Worst case
> we even have to rebuild the sched_domain hierarchy to update flags. The
> update would also temporarily mess with load and utilization signals, so
> not a cheap operation.

I didn't say it was cheap. :-)

However, boost frequencies are not disabled and enabled very often, so
it may be acceptable to do it then.  I actually don't know.

The point is that if you set the max capacity to correspond to the max
boosted perf and it is never reached (because boost is disabled), the
scaling will cause CPUs to appear as underutilized, but in fact there
is no spare capacity in the system.

Conversely, if the max capacity corresponds to the max non-boost perf
and boost is used very often, the scaling will cause the CPUs to
appear to be 100% loaded, but there may be still spare capacity in the
system.

  reply	other threads:[~2021-12-02 16:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 20:10 [PATCH] base: arch_topology: Use policy->max to calculate freq_factor Thara Gopinath
2021-11-16 19:05 ` Rafael J. Wysocki
2021-11-17 10:46   ` Lukasz Luba
2021-11-17 12:49     ` Rafael J. Wysocki
2021-11-17 15:08       ` Lukasz Luba
2021-11-17 15:17         ` Rafael J. Wysocki
2021-11-17 15:47           ` Lukasz Luba
2021-11-17 17:01       ` Thara Gopinath
2021-11-17 17:59         ` Rafael J. Wysocki
2021-12-02 10:50           ` Morten Rasmussen
2021-12-02 16:31             ` Rafael J. Wysocki [this message]
2021-12-03  9:48               ` Morten Rasmussen
2021-12-03 15:07                 ` Rafael J. Wysocki
2021-12-08  9:20                   ` Morten Rasmussen
2021-11-17 11:21   ` Dietmar Eggemann
2021-11-17 10:12 ` Lukasz Luba
2021-11-24 16:22   ` 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=CAJZ5v0hRvsoEZj45OWe34uhAPj+J1rJWq5Wff4R0f_BYEuU5wA@mail.gmail.com \
    --to=rafael@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=morten.rasmussen@arm.com \
    --cc=sudeep.holla@arm.com \
    --cc=thara.gopinath@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).