Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20231103131821.1176294-1-vincent.guittot@linaro.org>
 <20231103131821.1176294-2-vincent.guittot@linaro.org> <20231114205422.k5m6y4m5vnw7dvzj@airbuntu>
 <CAKfTPtDMEes6V2xRHavAwWrVuiZBdFAsaaxv9=-psAZCTPQWKg@mail.gmail.com>
 <20231121211725.gaekv6svnqdiq5l4@airbuntu> <CAKfTPtDzAZMcuWOYYOOAjCyvrOQiqyHZJBFVbACAvTqo+pU1gQ@mail.gmail.com>
 <20231121220955.uxk2zanxfemwyfz6@airbuntu>
In-Reply-To: <20231121220955.uxk2zanxfemwyfz6@airbuntu>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Thu, 23 Nov 2023 14:32:52 +0100
Message-ID: <CAKfTPtAHbYCyqA6jLqkoWgQ2X625tann8Mpy0QttgQo5OPvS9w@mail.gmail.com>
Subject: Re: [PATCH v3 1/2] sched/schedutil: Rework performance estimation
To:     Qais Yousef <qyousef@layalina.io>
Cc:     mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
        dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com,
        mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com,
        rafael@kernel.org, viresh.kumar@linaro.org,
        linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
        lukasz.luba@arm.com, wyes.karny@amd.com, beata.michalska@arm.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 23 Nov 2023 at 14:15, Qais Yousef <qyousef@layalina.io> wrote:
>
> On 11/23/23 08:47, Vincent Guittot wrote:
>
> > > > > And is it right to mix irq and uclamp_min with bw_min which is for DL? We might
> > > >
> > > > cpu_bw_dl() is not the actual utilization by DL task but the computed
> > > > bandwidth which can be seen as min performance level
> > >
> > > Yep. That's why I am not in favour of a dvfs headroom for DL.
> > >
> > > But what I meant here is that in effective_cpu_util(), where we populate min
> > > and max we have
> > >
> > >         if (min) {
> > >                 /*
> > >                  * The minimum utilization returns the highest level between:
> > >                  * - the computed DL bandwidth needed with the irq pressure which
> > >                  *   steals time to the deadline task.
> > >                  * - The minimum performance requirement for CFS and/or RT.
> > >                  */
> > >                 *min = max(irq + cpu_bw_dl(rq), uclamp_rq_get(rq, UCLAMP_MIN));
> > >
> > > So if there was an RT/CFS task requesting a UCLAMP_MIN of 1024 for example,
> > > bw_min will end up being too high, no?
> >
> > But at the end, we want at least uclamp_min for cfs or rt just like we
> > want at least DL bandwidth for DL tasks
>
> The issue I see is that we do
>
> static void sugov_get_util()
> {
> ..
>         util = effective_cpu_util(.., &min, ..); // min = max(irq + cpu_bw_dl(), rq_uclamp_min)
>         ..
>         sg_cpu->bw_min = min; // bw_min can pick the rq_uclamp_min. Shouldn't it be irq + cpu_bw_dl() only?
>         ..
> }
>
> If yes, why the comparison in ignore_dl_rate_limit() is still correct then?
>
>         if (cpu_bw_dl(cpu_rq(sg_cpu->cpu)) > sg_cpu->bw_min)

yes, this is to ensure enough performance for the deadline task when
the dl bandwidth increases without waiting the rate limit period which
would prevent the system from providing enough bandwidth to the
deadline scheduler. This remains true because it's still at least
above cpu_bw_dl()

>
> And does cpufreq_driver_adjust_perf() still need the sg_cpu->bw_min arg
> actually? sg_cpu->util already calculated based on sugov_effective_cpu_perf()
> which takes all constraints (including bw_min) into account.

cpufreq_driver_adjust_perf() is used for systems on which you can't
actually set an operating frequency but only a min and a desired
performance level and let the hw move freely in this range.

>
>
> Thanks!
>
> --
> Qais Yousef