| Message ID | 20170705085905.6558-6-juri.lelli@arm.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On 05-07-17, 09:59, Juri Lelli wrote: > No assumption can be made upon the rate at which frequency updates get > triggered, as there are scheduling policies (like SCHED_DEADLINE) which > don't trigger them so frequently. > > Remove such assumption from the code, by always considering > SCHED_DEADLINE utilization signal as not stale. > > Signed-off-by: Juri Lelli <juri.lelli@arm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: Luca Abeni <luca.abeni@santannapisa.it> > Cc: Claudio Scordino <claudio@evidence.eu.com> > --- > kernel/sched/cpufreq_schedutil.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c > index e835fa886225..066b876d81e7 100644 > --- a/kernel/sched/cpufreq_schedutil.c > +++ b/kernel/sched/cpufreq_schedutil.c > @@ -267,17 +267,22 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time) > s64 delta_ns; > > /* > - * If the CPU utilization was last updated before the previous > - * frequency update and the time elapsed between the last update > - * of the CPU utilization and the last frequency update is long > - * enough, don't take the CPU into account as it probably is > - * idle now (and clear iowait_boost for it). > + * If the CFS CPU utilization was last updated before the > + * previous frequency update and the time elapsed between the > + * last update of the CPU utilization and the last frequency > + * update is long enough, reset iowait_boost and util_cfs, as > + * they are now probably stale. However, still consider the > + * CPU contribution if it has some DEADLINE utilization > + * (util_dl). > */ > delta_ns = time - j_sg_cpu->last_update; > if (delta_ns > TICK_NSEC) { > j_sg_cpu->iowait_boost = 0; > - continue; > + j_sg_cpu->util_cfs = 0; > + if (j_sg_cpu->util_dl == 0) > + continue; > } > + > if (j_sg_cpu->flags & SCHED_CPUFREQ_RT) > return policy->cpuinfo.max_freq; > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Wed, Jul 05, 2017 at 09:59:02AM +0100, Juri Lelli wrote: > delta_ns = time - j_sg_cpu->last_update; > if (delta_ns > TICK_NSEC) { > j_sg_cpu->iowait_boost = 0; > - continue; > + j_sg_cpu->util_cfs = 0; this is slighly confusing. Is this because we might not 'continue' with the new code? > + if (j_sg_cpu->util_dl == 0) > + continue; > } > + > if (j_sg_cpu->flags & SCHED_CPUFREQ_RT) > return policy->cpuinfo.max_freq; > > -- > 2.11.0 >
On 11/07/17 18:17, Peter Zijlstra wrote: > On Wed, Jul 05, 2017 at 09:59:02AM +0100, Juri Lelli wrote: > > delta_ns = time - j_sg_cpu->last_update; > > if (delta_ns > TICK_NSEC) { > > j_sg_cpu->iowait_boost = 0; > > - continue; > > + j_sg_cpu->util_cfs = 0; > > this is slighly confusing. Is this because we might not 'continue' with > the new code? > This is because, after TICK_NSEC, we only want to discard CFS contribution and (yes) continue (so don't take into account j_sg_cpu contribution) if DEADLINE contribution is zero as well. > > + if (j_sg_cpu->util_dl == 0) > > + continue; > > } > > + With this change we might not continue if some DEADLINE utilization is present for j_sg_cpu. > > if (j_sg_cpu->flags & SCHED_CPUFREQ_RT) > > return policy->cpuinfo.max_freq; > > > > -- > > 2.11.0 > >
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index e835fa886225..066b876d81e7 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -267,17 +267,22 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time) s64 delta_ns; /* - * If the CPU utilization was last updated before the previous - * frequency update and the time elapsed between the last update - * of the CPU utilization and the last frequency update is long - * enough, don't take the CPU into account as it probably is - * idle now (and clear iowait_boost for it). + * If the CFS CPU utilization was last updated before the + * previous frequency update and the time elapsed between the + * last update of the CPU utilization and the last frequency + * update is long enough, reset iowait_boost and util_cfs, as + * they are now probably stale. However, still consider the + * CPU contribution if it has some DEADLINE utilization + * (util_dl). */ delta_ns = time - j_sg_cpu->last_update; if (delta_ns > TICK_NSEC) { j_sg_cpu->iowait_boost = 0; - continue; + j_sg_cpu->util_cfs = 0; + if (j_sg_cpu->util_dl == 0) + continue; } + if (j_sg_cpu->flags & SCHED_CPUFREQ_RT) return policy->cpuinfo.max_freq;
No assumption can be made upon the rate at which frequency updates get triggered, as there are scheduling policies (like SCHED_DEADLINE) which don't trigger them so frequently. Remove such assumption from the code, by always considering SCHED_DEADLINE utilization signal as not stale. Signed-off-by: Juri Lelli <juri.lelli@arm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@kernel.org> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: Viresh Kumar <viresh.kumar@linaro.org> Cc: Luca Abeni <luca.abeni@santannapisa.it> Cc: Claudio Scordino <claudio@evidence.eu.com> --- kernel/sched/cpufreq_schedutil.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)