Received-SPF: pass (google.com: domain of linux-kernel+bounces-205593-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240606070645.3295-1-xuewen.yan@unisoc.com> <0763f870-e30c-46cf-aefa-b879f2ebdba4@arm.com>
In-Reply-To: <0763f870-e30c-46cf-aefa-b879f2ebdba4@arm.com>
From: Xuewen Yan <xuewen.yan94@gmail.com>
Date: Fri, 7 Jun 2024 16:20:49 +0800
Message-ID: <CAB8ipk_TjqoNetBZ7dbjRxuBHAP=nz9=ZNomnjnaCEikLQSK2A@mail.gmail.com>
Subject: Re: [PATCH] sched/fair: Prevent cpu_busy_time from exceeding actual_cpu_capacity
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Xuewen Yan <xuewen.yan@unisoc.com>, vincent.guittot@linaro.org, mingo@redhat.com, 
	peterz@infradead.org, juri.lelli@redhat.com, rostedt@goodmis.org, 
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com, 
	vincent.donnefort@arm.com, qyousef@layalina.io, ke.wang@unisoc.com, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Dietmar

On Fri, Jun 7, 2024 at 3:19=E2=80=AFPM Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
>
> On 06/06/2024 09:06, Xuewen Yan wrote:
> > Because the effective_cpu_util() would return a util which
> > maybe bigger than the actual_cpu_capacity, this could cause
> > the pd_busy_time calculation errors.
>
> Doesn't return effective_cpu_util() either scale or min(scale, util)
> with scale =3D arch_scale_cpu_capacity(cpu)? So the util sum over the PD
> cannot exceed eenv->cpu_cap?

In effective_cpu_util, the scale =3D arch_scale_cpu_capacity(cpu);
 Although there is the clamp of eenv->pd_cap, but let us consider the
following simple scenario:
The pd cpus are 4-7, and the arch_scale_capacity is 1024, and because
of cpufreq-limit,
the cpu_actual_cap =3D 512. Then the eenv->cpu_cap =3D 512, the eenv->pd_ca=
p =3D 2048;
effective_cpu_util(4) =3D 1024;
effective_cpu_util(5) =3D 1024;
effective_cpu_util(6) =3D 256;
effective_cpu_util(7) =3D 0;

Without this patch, the eenv->pd_busy_time =3D 2048, because the clamp
of eenv->pd_cap.
However, indeed, the cpu4 and cpu5's util would not exceed the actual_cap(5=
12),
so the cpu_util4/5 =3D 512, instead of 1024.
And with this patch, the eenv->pd_busy_time =3D 512+512+256 =3D 1280.
And the 1280 should be more reasonable.

BR
--
xuewen

> Looks like this was the case with 3e8c6c9aac42 already.
>
> > So clamp the cpu_busy_time with the eenv->cpu_cap, which is
> > the actual_cpu_capacity.
> >
> > Fixes: 3e8c6c9aac42 ("sched/fair: Remove task_util from effective utili=
zation in feec()")
> > Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> > ---
> >  kernel/sched/fair.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 8a5b1ae0aa55..8939d725023a 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7870,7 +7870,9 @@ static inline void eenv_pd_busy_time(struct energ=
y_env *eenv,
> >       for_each_cpu(cpu, pd_cpus) {
> >               unsigned long util =3D cpu_util(cpu, p, -1, 0);
> >
> > -             busy_time +=3D effective_cpu_util(cpu, util, NULL, NULL);
> > +             util =3D effective_cpu_util(cpu, util, NULL, NULL);
> > +             util =3D min(eenv->cpu_cap, util);
> > +             busy_time +=3D util;
> >       }
> >
> >       eenv->pd_busy_time =3D min(eenv->pd_cap, busy_time);
>