[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKfTPtDWFe_5Vt8X48ZQLwX1tnjCcktWFOqtacjPhUSpO4Aj3Q@mail.gmail.com>
Date: Fri, 6 Sep 2024 09:08:50 +0200
From: Vincent Guittot <vincent.guittot@...aro.org>
To: Pierre Gondois <pierre.gondois@....com>
Cc: mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
mgorman@...e.de, vschneid@...hat.com, lukasz.luba@....com,
rafael.j.wysocki@...el.com, linux-kernel@...r.kernel.org, qyousef@...alina.io,
hongyan.xia2@....com
Subject: Re: [PATCH 3/5] sched/fair: Rework feec() to use cost instead of
spare capacity
On Wed, 4 Sept 2024 at 17:07, Pierre Gondois <pierre.gondois@....com> wrote:
>
>
>
> On 8/30/24 15:03, Vincent Guittot wrote:
> > feec() looks for the CPU with highest spare capacity in a PD assuming that
> > it will be the best CPU from a energy efficiency PoV because it will
> > require the smallest increase of OPP. Although this is true generally
> > speaking, this policy also filters some others CPUs which will be as
> > efficients because of using the same OPP.
> > In fact, we really care about the cost of the new OPP that will be
> > selected to handle the waking task. In many cases, several CPUs will end
> > up selecting the same OPP and as a result using the same energy cost. In
> > these cases, we can use other metrics to select the best CPU for the same
> > energy cost.
> >
> > Rework feec() to look 1st for the lowest cost in a PD and then the most
> > performant CPU between CPUs.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@...aro.org>
> > ---
> > kernel/sched/fair.c | 466 +++++++++++++++++++++++---------------------
> > 1 file changed, 244 insertions(+), 222 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index e67d6029b269..2273eecf6086 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > -/*
> > - * compute_energy(): Use the Energy Model to estimate the energy that @pd would
> > - * consume for a given utilization landscape @eenv. When @dst_cpu < 0, the task
> > - * contribution is ignored.
> > - */
> > -static inline unsigned long
> > -compute_energy(struct energy_env *eenv, struct perf_domain *pd,
> > - struct cpumask *pd_cpus, struct task_struct *p, int dst_cpu)
> > +/*Check if the CPU can handle the waking task */
> > +static int check_cpu_with_task(struct task_struct *p, int cpu)
> > {
> > - unsigned long max_util = eenv_pd_max_util(eenv, pd_cpus, p, dst_cpu);
> > - unsigned long busy_time = eenv->pd_busy_time;
> > - unsigned long energy;
> > + unsigned long p_util_min = uclamp_is_used() ? uclamp_eff_value(p, UCLAMP_MIN) : 0;
> > + unsigned long p_util_max = uclamp_is_used() ? uclamp_eff_value(p, UCLAMP_MAX) : 1024;
> > + unsigned long util_min = p_util_min;
> > + unsigned long util_max = p_util_max;
> > + unsigned long util = cpu_util(cpu, p, cpu, 0);
> > + struct rq *rq = cpu_rq(cpu);
> >
> > - if (dst_cpu >= 0)
> > - busy_time = min(eenv->pd_cap, busy_time + eenv->task_busy_time);
> > + /*
> > + * Skip CPUs that cannot satisfy the capacity request.
> > + * IOW, placing the task there would make the CPU
> > + * overutilized. Take uclamp into account to see how
> > + * much capacity we can get out of the CPU; this is
> > + * aligned with sched_cpu_util().
> > + */
> > + if (uclamp_is_used() && !uclamp_rq_is_idle(rq)) {
> > + unsigned long rq_util_min, rq_util_max;
> > + /*
> > + * Open code uclamp_rq_util_with() except for
> > + * the clamp() part. I.e.: apply max aggregation
> > + * only. util_fits_cpu() logic requires to
> > + * operate on non clamped util but must use the
> > + * max-aggregated uclamp_{min, max}.
> > + */
> > + rq_util_min = uclamp_rq_get(rq, UCLAMP_MIN);
> > + rq_util_max = uclamp_rq_get(rq, UCLAMP_MAX);
> > + util_min = max(rq_util_min, p_util_min);
> > + util_max = max(rq_util_max, p_util_max);
> > + }
> > + return util_fits_cpu(util, util_min, util_max, cpu);
> > +}
> >
> > - energy = em_cpu_energy(pd->em_pd, max_util, busy_time, eenv->cpu_cap);
>
> I think em_cpu_energy() would need to be removed with this patch,
> if there are no more references to it.
Yes, I will add a patch to cleanup unused function
>
Powered by blists - more mailing lists