Received-SPF: pass (google.com: domain of linux-kernel+bounces-25466-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <ZTz9RpZxfxysYCmt@gmail.com> <ZZwBi/YmnMqm7zrO@gmail.com>
 <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> <711c20cf-4aa7-4380-b076-195736bc4978@arm.com>
In-Reply-To: <711c20cf-4aa7-4380-b076-195736bc4978@arm.com>
From: Vincent Guittot <vincent.guittot@linaro.org>
Date: Sun, 14 Jan 2024 14:05:20 +0100
Message-ID: <CAKfTPtA8dQoT404AzHE_O9sP+ecx0vpKG_2ErjgusSUWRUgXxQ@mail.gmail.com>
Subject: Re: [GIT PULL] Scheduler changes for v6.8
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Wyes Karny <wkarny@gmail.com>, Linus Torvalds <torvalds@linux-foundation.org>, 
	Qais Yousef <qyousef@layalina.io>, 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>, 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

On Sun, 14 Jan 2024 at 14:02, Dietmar Eggemann <dietmar.eggemann@arm.com> w=
rote:
>
> On 14/01/2024 13:37, Wyes Karny wrote:
> > On Sun, Jan 14, 2024 at 12:18:06PM +0100, Vincent Guittot wrote:
> >> Hi Wyes,
> >>
> >> Le dimanche 14 janv. 2024 =C3=AF=C2=BF=C2=BD 14:42:40 (+0530), Wyes Ka=
rny a =C3=AF=C2=BF=C2=BDcrit :
> >>> On Wed, Jan 10, 2024 at 02:57:14PM -0800, Linus Torvalds wrote:
> >>>> On Wed, 10 Jan 2024 at 14:41, Linus Torvalds
> >>>> <torvalds@linux-foundation.org> wrote:
>
> [...]
>
> > Yeah, correct something was wrong in the bpftrace readings, max_cap is
> > not zero in traces.
> >
> >              git-5511    [001] d.h1.   427.159763: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >              git-5511    [001] d.h1.   427.163733: sugov_get_util: [DEB=
UG] : util 1024, sg_cpu->util 1024
> >              git-5511    [001] d.h1.   427.163735: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >              git-5511    [001] d.h1.   427.167706: sugov_get_util: [DEB=
UG] : util 1024, sg_cpu->util 1024
> >              git-5511    [001] d.h1.   427.167708: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >              git-5511    [001] d.h1.   427.171678: sugov_get_util: [DEB=
UG] : util 1024, sg_cpu->util 1024
> >              git-5511    [001] d.h1.   427.171679: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >              git-5511    [001] d.h1.   427.175653: sugov_get_util: [DEB=
UG] : util 1024, sg_cpu->util 1024
> >              git-5511    [001] d.h1.   427.175655: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >              git-5511    [001] d.s1.   427.175665: sugov_get_util: [DEB=
UG] : util 1024, sg_cpu->util 1024
> >              git-5511    [001] d.s1.   427.175665: get_next_freq.constp=
rop.0: [DEBUG] : freq 1400000, util 1024, max 1024
> >
> > Debug patch applied:
> >
> > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_sc=
hedutil.c
> > index 95c3c097083e..5c9b3e1de7a0 100644
> > --- a/kernel/sched/cpufreq_schedutil.c
> > +++ b/kernel/sched/cpufreq_schedutil.c
> > @@ -166,6 +166,7 @@ static unsigned int get_next_freq(struct sugov_poli=
cy *sg_policy,
> >
> >         freq =3D get_capacity_ref_freq(policy);
> >         freq =3D map_util_freq(util, freq, max);
> > +       trace_printk("[DEBUG] : freq %llu, util %llu, max %llu\n", freq=
, util, max);
> >
> >         if (freq =3D=3D sg_policy->cached_raw_freq && !sg_policy->need_=
freq_update)
> >                 return sg_policy->next_freq;
> > @@ -199,6 +200,7 @@ static void sugov_get_util(struct sugov_cpu *sg_cpu=
, unsigned long boost)
> >         util =3D max(util, boost);
> >         sg_cpu->bw_min =3D min;
> >         sg_cpu->util =3D sugov_effective_cpu_perf(sg_cpu->cpu, util, mi=
n, max);
> > +       trace_printk("[DEBUG] : util %llu, sg_cpu->util %llu\n", util, =
sg_cpu->util);
> >  }
> >
> >  /**
> >
> >
> > So, I guess map_util_freq going wrong somewhere.
>
> sugov_update_single_freq() -> get_next_freq() -> get_capacity_ref_freq()
>
> Is arch_scale_freq_invariant() true in both cases, so that
> get_capacity_ref_freq() returns 'policy->cpuinfo.max_freq' and not just
> 'policy->cur'?

That's also my assumption and the change that I sent shoulf fix it

>