Received-SPF: pass (google.com: domain of linux-kernel+bounces-25655-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <CAHk-=wgWcYX2oXKtgvNN2LLDXP7kXkbo-xTfumEjmPbjSer2RQ@mail.gmail.com>
 <CAHk-=wiXpsxMcQb7MhL-AxOityTajK0G8eWeBOzX-qBJ9X2DSw@mail.gmail.com>
 <CAHk-=wjK28MUqBZzBSMEM8vdJhDOuXGSWPmmp04GEt9CXtW6Pw@mail.gmail.com>
 <20240114091240.xzdvqk75ifgfj5yx@wyes-pc> <ZaPC7o44lEswxOXp@vingu-book>
 <20240114123759.pjs7ctexcpc6pshl@wyes-pc> <CAKfTPtCz+95dR38c_u6_7JbkVt=czj5N2dKYVV-zk9dgbk16hw@mail.gmail.com>
 <20240114151250.5wfexq44o3mdm3nh@airbuntu> <CAKfTPtAMxiTbvAYav1JQw+MhjzDPCZDrMLL2JOfsc0GWp+FnOA@mail.gmail.com>
 <20240114195815.nes4bn53tc25djbh@airbuntu> <20240114233728.hrmyelo66beaajhp@airbuntu>
In-Reply-To: <20240114233728.hrmyelo66beaajhp@airbuntu>
From: Wyes Karny <wkarny@gmail.com>
Date: Mon, 15 Jan 2024 11:55:04 +0530
Message-ID: <CAAE01kGAdczPmWZ5VqrF397FeOHexWfHDi9eYXv8LogDbuWiHQ@mail.gmail.com>
Subject: Re: [GIT PULL] Scheduler changes for v6.8
To: Qais Yousef <qyousef@layalina.io>
Cc: Vincent Guittot <vincent.guittot@linaro.org>, 
	Linus Torvalds <torvalds@linux-foundation.org>, Ingo Molnar <mingo@kernel.org>, 
	linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>, 
	Thomas Gleixner <tglx@linutronix.de>, Juri Lelli <juri.lelli@redhat.com>, 
	Dietmar Eggemann <dietmar.eggemann@arm.com>, Steven Rostedt <rostedt@goodmis.org>, 
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, 
	Daniel Bristot de Oliveira <bristot@redhat.com>, Valentin Schneider <vschneid@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Qais,

On Mon, Jan 15, 2024 at 5:07=E2=80=AFAM Qais Yousef <qyousef@layalina.io> w=
rote:
>
> On 01/14/24 19:58, Qais Yousef wrote:
>
> > > This is not correct because you will have to wait to reach full
> > > utilization at the current OPP possibly the lowest OPP before moving
> > > directly to max OPP
> >
> > Isn't this already the case? The ratio (util+headroom/max) will be less=
 than
> > 1 until util is 80% (with 25% headroom). And for all values <=3D 80% * =
max, we
> > will request a frequency smaller than/equal policy->cur, no?
> >
> > ie:
> >
> >       util =3D 600
> >       max =3D 1024
> >
> >       freq =3D 1.25 * 600 * policy->cur / 1024 =3D 0.73 * policy->cur
> >
> > (util+headroom/max) must be greater than 1 for us to start going above
> > policy->cur - which seems to have been working by accident IIUC.
> >
> > So yes my proposal is incorrect, but it seems the conversion is not rig=
ht to me
> > now.
> >
> > I could reproduce the problem now (thanks Wyes!). I have 3 freqs on my =
system
> >
> > 2.2GHz, 2.8GHz and 3.8GHz
> >
> > which (I believe) translates into capacities
> >
> > ~592, ~754, 1024
> >
> > which means we should pick 2.8GHz as soon as util * 1.25 > 592; which
> > translates into util =3D ~473.
> >
> > But what I see is that we go to 2.8GHz when we jump from 650 to 680 (se=
e
> > attached picture), which is what you'd expect since we apply two headro=
oms now,
> > which means the ratio (util+headroom/max) will be greater than 1 after =
go above
> > this value
> >
> >       1024 * 0.8 * 0.8 =3D ~655
> >
> > So I think the math makes sense logically, but we're missing some other
> > correction factor.
> >
> > When I re-enable CPPC I see for the same test that we go into 3.8GHz st=
raight
> > away. My test is simple busyloop via
> >
> >       cat /dev/zero > /dev/null
> >
> > I see the CPU util_avg is at 523 at fork. I expected us to run to 2.8GH=
z here
> > to be honest, but I am not sure if util_cfs_boost() and util_est() are =
maybe
> > causing us to be slightly above 523 and that's why we start with max fr=
eq.
> >
> > Or I've done the math wrong :-) But the two don't behave the same for t=
he same
> > kernel with and without CPPC.
>
> I think the relationship should be:
>
>         freq =3D util * f_curr / cap_curr

I guess to know the curr_cap correctly we need to know the max_freq,
which is not available when CPPC is disabled.

>
> (patch below)
>
> with that I see (almost) the expected behavior (picture attached). We go =
to
> 2.8GHz when we are above 500. But the move to 3.8GHz is a bit earlier at =
581
> (instead of 754 * 0.8 =3D 603). Not sure why. With 25% headroom 581 is 72=
6. So
> it's a tad too early.
>
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_sche=
dutil.c
> index 95c3c097083e..155f96a44fa0 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -123,7 +123,8 @@ static void sugov_deferred_update(struct sugov_policy=
 *sg_policy)
>   * Return: the reference CPU frequency to compute a capacity.
>   */
>  static __always_inline
> -unsigned long get_capacity_ref_freq(struct cpufreq_policy *policy)
> +unsigned long get_capacity_ref_freq(struct cpufreq_policy *policy,
> +                                   unsigned long *max)
>  {
>         unsigned int freq =3D arch_scale_freq_ref(policy->cpu);
>
> @@ -133,6 +134,9 @@ unsigned long get_capacity_ref_freq(struct cpufreq_po=
licy *policy)
>         if (arch_scale_freq_invariant())
>                 return policy->cpuinfo.max_freq;
>
> +       if (max)
> +               *max =3D policy->cur * (*max) / policy->cpuinfo.max_freq;

But when freq_invaiant is disabled we don't have policy->cpuinfo.max_freq.

Thanks,
Wyes
> +
>         return policy->cur;
>  }
>
> @@ -164,7 +168,7 @@ static unsigned int get_next_freq(struct sugov_policy=
 *sg_policy,
>         struct cpufreq_policy *policy =3D sg_policy->policy;
>         unsigned int freq;
>
> -       freq =3D get_capacity_ref_freq(policy);
> +       freq =3D get_capacity_ref_freq(policy, &max);
>         freq =3D map_util_freq(util, freq, max);
>
>         if (freq =3D=3D sg_policy->cached_raw_freq && !sg_policy->need_fr=
eq_update)


--=20
Thanks & Regards
Wyes