Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1604238
| From | Vincent Guittot <vincent.guittot@linaro.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs |
| Date | 2017-03-20 09:30 +0100 |
| Message-ID | <tn0Rs-22G-5@gated-at.bofh.it> (permalink) |
| References | <tmJnz-6kS-3@gated-at.bofh.it> <tmJnz-6kS-7@gated-at.bofh.it> <tmWE9-7lm-1@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 20 March 2017 at 04:57, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 19-03-17, 14:34, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> The PELT metric used by the schedutil governor underestimates the
>> CPU utilization in some cases. The reason for that may be time spent
>> in interrupt handlers and similar which is not accounted for by PELT.
Are you sure of the root cause described above (time stolen by irq
handler) or is it just a hypotheses ? That would be good to be sure of
the root cause
Furthermore, IIRC the time spent in irq context is also accounted as
run time for the running cfs task but not RT and deadline task running
time
So I'm not really aligned with the description of your problem: PELT
metric underestimates the load of the CPU. The PELT is just about
tracking CFS task utilization but not whole CPU utilization and
according to your description of the problem (time stolen by irq),
your problem doesn't come from an underestimation of CFS task but from
time spent in something else but not accounted in the value used by
schedutil
That would be good to be sure of what is running during this not
accounted time and find a way to account them
>>
>> That can be easily demonstrated by running kernel compilation on
>> a Sandy Bridge Intel processor, running turbostat in parallel with
>> it and looking at the values written to the MSR_IA32_PERF_CTL
>> register. Namely, the expected result would be that when all CPUs
>> were 100% busy, all of them would be requested to run in the maximum
>> P-state, but observation shows that this clearly isn't the case.
>> The CPUs run in the maximum P-state for a while and then are
>> requested to run slower and go back to the maximum P-state after
>> a while again. That causes the actual frequency of the processor to
>> visibly oscillate below the sustainable maximum in a jittery fashion
>> which clearly is not desirable.
>>
>> To work around this issue use the observation that, from the
>> schedutil governor's perspective, CPUs that are never idle should
>> always run at the maximum frequency and make that happen.
>>
>> To that end, add a counter of idle calls to struct sugov_cpu and
>> modify cpuidle_idle_call() to increment that counter every time it
>> is about to put the given CPU into an idle state. Next, make the
>> schedutil governor look at that counter for the current CPU every
>> time before it is about to start heavy computations. If the counter
>> has not changed for over SUGOV_BUSY_THRESHOLD time (equal to 50 ms),
>> the CPU has not been idle for at least that long and the governor
>> will choose the maximum frequency for it without looking at the PELT
>> metric at all.
>
> Looks like we are fixing a PELT problem with a schedutil Hack :)
I would not say that it's a PELT problem (at least based on current
description) but more that we don't track all
activities of CPU
>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> ---
>> include/linux/sched/cpufreq.h | 6 ++++++
>> kernel/sched/cpufreq_schedutil.c | 38 ++++++++++++++++++++++++++++++++++++--
>> kernel/sched/idle.c | 3 +++
>> 3 files changed, 45 insertions(+), 2 deletions(-)
>>
>> Index: linux-pm/kernel/sched/cpufreq_schedutil.c
>> ===================================================================
>> --- linux-pm.orig/kernel/sched/cpufreq_schedutil.c
>> +++ linux-pm/kernel/sched/cpufreq_schedutil.c
>> @@ -20,6 +20,7 @@
>> #include "sched.h"
>>
>> #define SUGOV_KTHREAD_PRIORITY 50
>> +#define SUGOV_BUSY_THRESHOLD (50 * NSEC_PER_MSEC)
>>
>> struct sugov_tunables {
>> struct gov_attr_set attr_set;
>> @@ -55,6 +56,9 @@ struct sugov_cpu {
>>
>> unsigned long iowait_boost;
>> unsigned long iowait_boost_max;
>> + unsigned long idle_calls;
>> + unsigned long saved_idle_calls;
>> + u64 busy_start;
>> u64 last_update;
>>
>> /* The fields below are only needed when sharing a policy. */
>> @@ -192,6 +196,34 @@ static void sugov_iowait_boost(struct su
>> sg_cpu->iowait_boost >>= 1;
>> }
>>
>> +void cpufreq_schedutil_idle_call(void)
>> +{
>> + struct sugov_cpu *sg_cpu = this_cpu_ptr(&sugov_cpu);
>> +
>> + sg_cpu->idle_calls++;
>> +}
>> +
>> +static bool sugov_cpu_is_busy(struct sugov_cpu *sg_cpu)
>> +{
>> + if (sg_cpu->idle_calls != sg_cpu->saved_idle_calls) {
>> + sg_cpu->busy_start = 0;
>> + return false;
>> + }
>> +
>> + if (!sg_cpu->busy_start) {
>> + sg_cpu->busy_start = sg_cpu->last_update;
>> + return false;
>> + }
>> +
>> + return sg_cpu->last_update - sg_cpu->busy_start > SUGOV_BUSY_THRESHOLD;
>> +}
>> +
>> +static void sugov_save_idle_calls(struct sugov_cpu *sg_cpu)
>> +{
>> + if (!sg_cpu->busy_start)
>> + sg_cpu->saved_idle_calls = sg_cpu->idle_calls;
>
> Why aren't we doing this in sugov_cpu_is_busy() itself ? And isn't it possible
> for idle_calls to get incremented by this time?
>
>> +}
>> +
>> static void sugov_update_single(struct update_util_data *hook, u64 time,
>> unsigned int flags)
>> {
>> @@ -207,7 +239,7 @@ static void sugov_update_single(struct u
>> if (!sugov_should_update_freq(sg_policy, time))
>> return;
>>
>> - if (flags & SCHED_CPUFREQ_RT_DL) {
>> + if ((flags & SCHED_CPUFREQ_RT_DL) || sugov_cpu_is_busy(sg_cpu)) {
>> next_f = policy->cpuinfo.max_freq;
>> } else {
>> sugov_get_util(&util, &max);
>> @@ -215,6 +247,7 @@ static void sugov_update_single(struct u
>> next_f = get_next_freq(sg_policy, util, max);
>> }
>> sugov_update_commit(sg_policy, time, next_f);
>> + sugov_save_idle_calls(sg_cpu);
>> }
>>
>> static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu)
>> @@ -278,12 +311,13 @@ static void sugov_update_shared(struct u
>> sg_cpu->last_update = time;
>>
>> if (sugov_should_update_freq(sg_policy, time)) {
>> - if (flags & SCHED_CPUFREQ_RT_DL)
>> + if ((flags & SCHED_CPUFREQ_RT_DL) || sugov_cpu_is_busy(sg_cpu))
>
> What about others CPUs in this policy?
>
>> next_f = sg_policy->policy->cpuinfo.max_freq;
>> else
>> next_f = sugov_next_freq_shared(sg_cpu);
>>
>> sugov_update_commit(sg_policy, time, next_f);
>> + sugov_save_idle_calls(sg_cpu);
>> }
>>
>> raw_spin_unlock(&sg_policy->update_lock);
>
> --
> viresh
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH 0/2] cpufreq: schedutil: Fix and optimization "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-19 14:50 +0100
[PATCH 1/2] cpufreq: schedutil: Fix per-CPU structure initialization in sugov_start() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-19 14:50 +0100
Re: [PATCH 1/2] cpufreq: schedutil: Fix per-CPU structure initialization in sugov_start() Viresh Kumar <viresh.kumar@linaro.org> - 2017-03-20 04:30 +0100
Re: [PATCH 1/2] cpufreq: schedutil: Fix per-CPU structure initialization in sugov_start() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 13:50 +0100
[RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-19 14:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-19 22:40 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-19 22:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-20 12:10 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 13:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Viresh Kumar <viresh.kumar@linaro.org> - 2017-03-20 05:00 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-20 09:30 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-20 13:40 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Joel Fernandes <joelaf@google.com> - 2017-03-23 01:00 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-23 23:10 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Joel Fernandes <joelaf@google.com> - 2017-03-25 04:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-27 09:10 +0200
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 14:10 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-20 14:40 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 14:30 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-20 12:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 13:50 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-20 14:00 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-20 14:10 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 14:40 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-20 15:20 +0100
Re: [RFC][PATCH 2/2] cpufreq: schedutil: Force max frequency on busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 14:20 +0100
[RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-20 23:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Viresh Kumar <viresh.kumar@linaro.org> - 2017-03-21 07:50 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rafael@kernel.org> - 2017-03-21 13:40 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-21 10:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 13:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 14:30 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-21 14:40 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-21 15:30 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-21 15:40 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 15:50 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-21 16:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 16:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-21 16:30 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 18:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-21 18:30 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 16:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 16:20 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 20:30 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-21 16:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 16:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 15:40 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 16:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Peter Zijlstra <peterz@infradead.org> - 2017-03-21 16:00 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-21 18:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-21 18:10 +0100
Re: [RFC][PATCH v2 2/2] cpufreq: schedutil: Avoid decreasing frequency of busy CPUs Patrick Bellasi <patrick.bellasi@arm.com> - 2017-03-21 13:00 +0100
[RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-22 00:20 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Peter Zijlstra <peterz@infradead.org> - 2017-03-22 10:30 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Viresh Kumar <viresh.kumar@linaro.org> - 2017-03-22 11:10 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Joel Fernandes <joelaf@google.com> - 2017-03-23 02:10 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Sai Gurrappadi <sgurrappadi@nvidia.com> - 2017-03-23 20:30 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Sai Gurrappadi <sgurrappadi@nvidia.com> - 2017-03-23 22:00 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely "Rafael J. Wysocki" <rafael@kernel.org> - 2017-03-24 02:40 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Sai Gurrappadi <sgurrappadi@nvidia.com> - 2017-03-24 20:20 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Sai Gurrappadi <sgurrappadi@nvidia.com> - 2017-03-25 02:20 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely "Rafael J. Wysocki" <rafael@kernel.org> - 2017-03-25 02:40 +0100
Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely Vincent Guittot <vincent.guittot@linaro.org> - 2017-03-27 09:20 +0200
csiph-web