diff mbox

[v2] cpufreq: schedutil: Trace frequency only if it has changed

Message ID 2563394.lUgF3b60iC@aspire.rjw.lan (mailing list archive)
State Mainlined
Delegated to: Rafael Wysocki
Headers show

Commit Message

Rafael J. Wysocki March 22, 2017, 5:32 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

sugov_update_commit() calls trace_cpu_frequency() to record the
current CPU frequency if it has not changed in the fast switch case
to prevent utilities from getting confused (they may report that the
CPU is idle if the frequency has not been recorded for too long, for
example).

However, that may cause the tracepoint to be triggered quite often
for no real reason (if the frequency doesn't change, we will not
modify the last update time stamp and governor computations may
run again shortly when that happens), so don't do that (arguably, it
is done to work around a utilities bug anyway).

That allows code duplication in sugov_update_commit() to be reduced
somewhat too.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

-> v2:
Drop the trace_cpu_frequency() call in the sg_policy->next_freq == next_freq case.

---
 kernel/sched/cpufreq_schedutil.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Viresh Kumar March 23, 2017, 4:32 a.m. UTC | #1
On 22-03-17, 18:32, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> sugov_update_commit() calls trace_cpu_frequency() to record the
> current CPU frequency if it has not changed in the fast switch case
> to prevent utilities from getting confused (they may report that the
> CPU is idle if the frequency has not been recorded for too long, for
> example).
> 
> However, that may cause the tracepoint to be triggered quite often
> for no real reason (if the frequency doesn't change, we will not
> modify the last update time stamp and governor computations may
> run again shortly when that happens), so don't do that (arguably, it
> is done to work around a utilities bug anyway).
> 
> That allows code duplication in sugov_update_commit() to be reduced
> somewhat too.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> -> v2:
> Drop the trace_cpu_frequency() call in the sg_policy->next_freq == next_freq case.
> 
> ---
>  kernel/sched/cpufreq_schedutil.c |   16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
diff mbox

Patch

Index: linux-pm/kernel/sched/cpufreq_schedutil.c
===================================================================
--- linux-pm.orig/kernel/sched/cpufreq_schedutil.c
+++ linux-pm/kernel/sched/cpufreq_schedutil.c
@@ -98,22 +98,20 @@  static void sugov_update_commit(struct s
 {
 	struct cpufreq_policy *policy = sg_policy->policy;
 
+	if (sg_policy->next_freq == next_freq)
+		return;
+
+	sg_policy->next_freq = next_freq;
+	sg_policy->last_freq_update_time = time;
+
 	if (policy->fast_switch_enabled) {
-		if (sg_policy->next_freq == next_freq) {
-			trace_cpu_frequency(policy->cur, smp_processor_id());
-			return;
-		}
-		sg_policy->next_freq = next_freq;
-		sg_policy->last_freq_update_time = time;
 		next_freq = cpufreq_driver_fast_switch(policy, next_freq);
 		if (next_freq == CPUFREQ_ENTRY_INVALID)
 			return;
 
 		policy->cur = next_freq;
 		trace_cpu_frequency(next_freq, smp_processor_id());
-	} else if (sg_policy->next_freq != next_freq) {
-		sg_policy->next_freq = next_freq;
-		sg_policy->last_freq_update_time = time;
+	} else {
 		sg_policy->work_in_progress = true;
 		irq_work_queue(&sg_policy->irq_work);
 	}