On Fri, Jun 08, 2018 at 02:09:47PM +0200, Vincent Guittot wrote:
> static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu)
> {
> struct rq *rq = cpu_rq(sg_cpu->cpu);
> + unsigned long util;
>
> if (rq->rt.rt_nr_running)
> return sg_cpu->max;
>
> + util = sg_cpu->util_dl;
> + util += sg_cpu->util_cfs;
> + util += sg_cpu->util_rt;
> +
> /*
> * Utilization required by DEADLINE must always be granted while, for
> * FAIR, we use blocked utilization of IDLE CPUs as a mechanism to
> @@ -197,7 +204,7 @@ static unsigned long sugov_aggregate_util(struct
> sugov_cpu *sg_cpu)
> * util_cfs + util_dl as requested freq. However, cpufreq is not yet
> * ready for such an interface. So, we only do the latter for now.
> */
> - return min(sg_cpu->max, (sg_cpu->util_dl + sg_cpu->util_cfs));
> + return min(sg_cpu->max, util);
> }
So this (and the dl etc. equivalents) result in exactly the problems
complained about last time, no?
What I proposed was something along the lines of:
util = 1024 * sg_cpu->util_cfs;
util /= (1024 - (sg_cpu->util_rt + sg_cpu->util_dl + ...));
return min(sg_cpu->max, util + sg_cpu->bw_dl);
Where we, instead of directly adding the various util signals.
I now see an email from Quentin asking if these things are not in fact
the same, but no, they are not. The difference is that the above only
affects the CFS signal and will re-normalize the utilization of an
'always' running task back to 1 by compensating for the stolen capacity.
But it will not, like these here patches, affect the OPP selection of
other classes. If there is no CFS utilization (or very little), then the
renormalization will not matter, and the existing DL bandwidth
compuation will be unaffected.
