| Message ID | 20180511131509.16275-2-patrick.bellasi@arm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Patrick, On 11 May 2018 at 15:15, Patrick Bellasi <patrick.bellasi@arm.com> wrote: > Since the refactoring introduced by: > > commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support") > > we aggregate FAIR utilization only if this class has runnable tasks. > This was mainly due to avoid the risk to stay on an high frequency just > because of the blocked utilization of a CPU not being properly decayed > while the CPU was idle. > > However, since: > > commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle") > > the FAIR blocked utilization is properly decayed also for IDLE CPUs. > > This allows us to use the FAIR blocked utilization as a safe mechanism > to gracefully reduce the frequency only if no FAIR tasks show up on a > CPU for a reasonable period of time. > > Moreover, we also reduce the frequency drops of CPUs running periodic > tasks which, depending on the task periodicity and the time required > for a frequency switch, was increasing the chances to introduce some > undesirable performance variations. > > Reported-by: Vincent Guittot <vincent.guittot@linaro.org> > Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Vincent Guittot <vincent.guittot@linaro.org> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: Joel Fernandes <joelaf@google.com> > Cc: linux-kernel@vger.kernel.org > Cc: linux-pm@vger.kernel.org With this patch, I can't see the spurious OPP changes that I was seeing before FWIW Acked-by: Vincent Guittot <vincent.guittot@linaro.org> Tested-by: Vincent Guittot <vincent.guittot@linaro.org> Regards, Vincent > > --- > > Changes in v2: > - add "Acked-by" Viresh tag > --- > kernel/sched/cpufreq_schedutil.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > > diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c > index d2c6083304b4..a74d05160e66 100644 > --- a/kernel/sched/cpufreq_schedutil.c > +++ b/kernel/sched/cpufreq_schedutil.c > @@ -183,22 +183,21 @@ static void sugov_get_util(struct sugov_cpu *sg_cpu) > 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) { > - util = sg_cpu->max; > - } else { > - util = sg_cpu->util_dl; > - if (rq->cfs.h_nr_running) > - util += sg_cpu->util_cfs; > - } > + if (rq->rt.rt_nr_running) > + return sg_cpu->max; > > /* > + * Utilization required by DEADLINE must always be granted while, for > + * FAIR, we use blocked utilization of IDLE CPUs as a mechanism to > + * gracefully reduce the frequency when no tasks show up for longer > + * periods of time. > + * > * 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. > */ > - return min(util, sg_cpu->max); > + return min(sg_cpu->max, (sg_cpu->util_dl + sg_cpu->util_cfs)); > } > > static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, unsigned int flags) > -- > 2.15.1 >
On 14-May 11:16, Vincent Guittot wrote: > Hi Patrick, Hi Vincent, > On 11 May 2018 at 15:15, Patrick Bellasi <patrick.bellasi@arm.com> wrote: > > Since the refactoring introduced by: > > > > commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support") > > > > we aggregate FAIR utilization only if this class has runnable tasks. > > This was mainly due to avoid the risk to stay on an high frequency just > > because of the blocked utilization of a CPU not being properly decayed > > while the CPU was idle. > > > > However, since: > > > > commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle") > > > > the FAIR blocked utilization is properly decayed also for IDLE CPUs. > > > > This allows us to use the FAIR blocked utilization as a safe mechanism > > to gracefully reduce the frequency only if no FAIR tasks show up on a > > CPU for a reasonable period of time. > > > > Moreover, we also reduce the frequency drops of CPUs running periodic > > tasks which, depending on the task periodicity and the time required > > for a frequency switch, was increasing the chances to introduce some > > undesirable performance variations. > > > > Reported-by: Vincent Guittot <vincent.guittot@linaro.org> > > Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com> > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Cc: Vincent Guittot <vincent.guittot@linaro.org> > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > > Cc: Joel Fernandes <joelaf@google.com> > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-pm@vger.kernel.org > > With this patch, I can't see the spurious OPP changes that I was seeing before Cool thanks... regarding OPP updates I've added some more comments in my reply to Joel's comments to my last patch of this series. Would be nice if you can have a look... toward the end there are some considerations about schedutil updates (indirectly) triggered by your patches for blocked load updates on IDLE CPUs. > FWIW > Acked-by: Vincent Guittot <vincent.guittot@linaro.org> > Tested-by: Vincent Guittot <vincent.guittot@linaro.org> Thanks for testing, will add these to the next respin.
On 14 May 2018 at 18:48, Patrick Bellasi <patrick.bellasi@arm.com> wrote: > On 14-May 11:16, Vincent Guittot wrote: >> Hi Patrick, > > Hi Vincent, > >> On 11 May 2018 at 15:15, Patrick Bellasi <patrick.bellasi@arm.com> wrote: >> > Since the refactoring introduced by: >> > >> > commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support") >> > >> > we aggregate FAIR utilization only if this class has runnable tasks. >> > This was mainly due to avoid the risk to stay on an high frequency just >> > because of the blocked utilization of a CPU not being properly decayed >> > while the CPU was idle. >> > >> > However, since: >> > >> > commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle") >> > >> > the FAIR blocked utilization is properly decayed also for IDLE CPUs. >> > >> > This allows us to use the FAIR blocked utilization as a safe mechanism >> > to gracefully reduce the frequency only if no FAIR tasks show up on a >> > CPU for a reasonable period of time. >> > >> > Moreover, we also reduce the frequency drops of CPUs running periodic >> > tasks which, depending on the task periodicity and the time required >> > for a frequency switch, was increasing the chances to introduce some >> > undesirable performance variations. >> > >> > Reported-by: Vincent Guittot <vincent.guittot@linaro.org> >> > Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com> >> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> >> > Cc: Ingo Molnar <mingo@redhat.com> >> > Cc: Peter Zijlstra <peterz@infradead.org> >> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> > Cc: Vincent Guittot <vincent.guittot@linaro.org> >> > Cc: Viresh Kumar <viresh.kumar@linaro.org> >> > Cc: Joel Fernandes <joelaf@google.com> >> > Cc: linux-kernel@vger.kernel.org >> > Cc: linux-pm@vger.kernel.org >> >> With this patch, I can't see the spurious OPP changes that I was seeing before > > Cool thanks... regarding OPP updates I've added some more comments in > my reply to Joel's comments to my last patch of this series. > > Would be nice if you can have a look... toward the end there are some > considerations about schedutil updates (indirectly) triggered by your > patches for blocked load updates on IDLE CPUs. I have started to have a look at the 3rd patch and was checking if there were some hole and your proposal regarding the update of blocked load and the removed utilization I will read your latest comment. > >> FWIW >> Acked-by: Vincent Guittot <vincent.guittot@linaro.org> >> Tested-by: Vincent Guittot <vincent.guittot@linaro.org> > > Thanks for testing, will add these to the next respin. > > -- > #include <best/regards.h> > > Patrick Bellasi
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index d2c6083304b4..a74d05160e66 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -183,22 +183,21 @@ static void sugov_get_util(struct sugov_cpu *sg_cpu) 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) { - util = sg_cpu->max; - } else { - util = sg_cpu->util_dl; - if (rq->cfs.h_nr_running) - util += sg_cpu->util_cfs; - } + if (rq->rt.rt_nr_running) + return sg_cpu->max; /* + * Utilization required by DEADLINE must always be granted while, for + * FAIR, we use blocked utilization of IDLE CPUs as a mechanism to + * gracefully reduce the frequency when no tasks show up for longer + * periods of time. + * * 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. */ - return min(util, sg_cpu->max); + return min(sg_cpu->max, (sg_cpu->util_dl + sg_cpu->util_cfs)); } static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, unsigned int flags)
Since the refactoring introduced by: commit 8f111bc357aa ("cpufreq/schedutil: Rewrite CPUFREQ_RT support") we aggregate FAIR utilization only if this class has runnable tasks. This was mainly due to avoid the risk to stay on an high frequency just because of the blocked utilization of a CPU not being properly decayed while the CPU was idle. However, since: commit 31e77c93e432 ("sched/fair: Update blocked load when newly idle") the FAIR blocked utilization is properly decayed also for IDLE CPUs. This allows us to use the FAIR blocked utilization as a safe mechanism to gracefully reduce the frequency only if no FAIR tasks show up on a CPU for a reasonable period of time. Moreover, we also reduce the frequency drops of CPUs running periodic tasks which, depending on the task periodicity and the time required for a frequency switch, was increasing the chances to introduce some undesirable performance variations. Reported-by: Vincent Guittot <vincent.guittot@linaro.org> Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: Vincent Guittot <vincent.guittot@linaro.org> Cc: Viresh Kumar <viresh.kumar@linaro.org> Cc: Joel Fernandes <joelaf@google.com> Cc: linux-kernel@vger.kernel.org Cc: linux-pm@vger.kernel.org --- Changes in v2: - add "Acked-by" Viresh tag --- kernel/sched/cpufreq_schedutil.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)