Messages in this thread | | | Date | Tue, 5 Dec 2017 16:24:24 +0100 | From | Juri Lelli <> | Subject | Re: [RFC PATCH v2 1/8] sched/cpufreq_schedutil: make use of DEADLINE utilization signal |
| |
Hi,
On 05/12/17 15:09, Patrick Bellasi wrote: > Hi Juri, >
[...]
> > static void sugov_get_util(unsigned long *util, unsigned long *max, int cpu) > > { > > struct rq *rq = cpu_rq(cpu); > > - unsigned long cfs_max; > > + unsigned long dl_util = (rq->dl.running_bw * SCHED_CAPACITY_SCALE) > > + >> BW_SHIFT; > > What about using a pair of getter methods (e.g. cpu_util_{cfs,dl}) to > be defined in kernel/sched/sched.h? > > This would help to hide class-specific signals mangling from cpufreq. > And here we can have something "more abstract" like: > > unsigned long util_cfs = cpu_util_cfs(rq); > unsigned long util_dl = cpu_util_dl(rq);
LGTM. I'll cook something for next spin.
> > > > > - cfs_max = arch_scale_cpu_capacity(NULL, cpu); > > + *max = arch_scale_cpu_capacity(NULL, cpu); > > > > - *util = min(rq->cfs.avg.util_avg, cfs_max); > > - *max = cfs_max; > > + /* > > + * Ideally we would like to set util_dl as min/guaranteed freq and > > + * 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. > > + */ > > Maybe I don't completely get the above comment, but to me it is not > really required. > > When you say that "util_dl" should be set to a min/guaranteed freq > are you not actually talking about a DL implementation detail? > > From the cpufreq standpoint instead, we should always set a capacity > which can accommodate util_dl + util_cfs.
It's more for platforms which supports such combination of values for frequency requests (CPPC like, AFAIU). The idea being that util_dl is what the system has to always guarantee, but it could go up to the sum if feasible.
> > We don't care about the meaning of util_dl and we should always assume > (by default) that the signal is properly updated by the scheduling > class... which unfortunately does not always happen for CFS. > > > > + *util = min(rq->cfs.avg.util_avg + dl_util, *max); > > With the above proposal, here also we will have: > > *util = min(util_cfs + util_dl, *max);
Looks cleaner.
Thanks,
- Juri
| |