linux-kernel - Re: [PATCH v3 1/2] sched/schedutil: Rework performance estimation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231121220955.uxk2zanxfemwyfz6@airbuntu>
Date:   Tue, 21 Nov 2023 22:09:55 +0000
From:   Qais Yousef <qyousef@...alina.io>
To:     Vincent Guittot <vincent.guittot@...aro.org>
Cc:     mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, vschneid@...hat.com,
        rafael@...nel.org, viresh.kumar@...aro.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        lukasz.luba@....com, wyes.karny@....com, beata.michalska@....com
Subject: Re: [PATCH v3 1/2] sched/schedutil: Rework performance estimation

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)

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.


Thanks!

--
Qais Yousef