[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2727017.UmaUvtBLeX@vostro.rjw.lan>
Date: Thu, 31 Mar 2016 17:43:39 +0200
From: "Rafael J. Wysocki" <rjw@...ysocki.net>
To: Jörg Otte <jrg.otte@...il.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux PM list <linux-pm@...r.kernel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Doug Smythies <dsmythies@...us.net>
Subject: Re: [intel-pstate driver regression] processor frequency very high even if in idle
On Thursday, March 31, 2016 05:25:18 PM Jörg Otte wrote:
> 2016-03-31 13:42 GMT+02:00 Rafael J. Wysocki <rjw@...ysocki.net>:
> > On Thursday, March 31, 2016 11:05:56 AM Jörg Otte wrote:
> >
> > [cut]
> >
> >> >
> >>
> >> Yes, works for me.
> >>
> >> CPUID(7): No-SGX
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 11 0.66 1682 2494
> >> 0 11 0.60 1856 2494
> >> 1 6 0.34 1898 2494
> >> 2 13 0.82 1628 2494
> >> 3 13 0.87 1528 2494
> >> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> >> - 6 0.58 963 2494
> >> 0 8 0.83 957 2494
> >> 1 1 0.08 984 2494
> >> 2 10 1.04 975 2494
> >> 3 3 0.35 934 2494
> >>
> >
[cut]
> >
>
> No, this patch doesn't help.
Well, more work to do then.
I've just noticed a bug in this patch, which is not relevant for the results,
but below is a new version.
> CPUID(7): No-SGX
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 8 0.32 2507 2495
> 0 13 0.53 2505 2495
> 1 3 0.11 2523 2495
> 2 1 0.06 2555 2495
> 3 15 0.59 2500 2495
> CPU Avg_MHz Busy% Bzy_MHz TSC_MHz
> - 8 0.33 2486 2495
> 0 12 0.50 2482 2495
> 1 5 0.22 2489 2495
> 2 1 0.04 2492 2495
> 3 15 0.59 2487 2495
Please apply the patch below and take a (1s or so) trace from the pstate_sample
tracepoint (under /sys/kernel/debug/tracing/events/power/ on my systems).
Then please apply the revert instead of it and take a trace from that tracepoint
again and send both of the traces to me.
---
From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Subject: [PATCH] intel_pstate: Do not set utilization update hook too early
The utilization update hook in the intel_pstate driver is set too
early, as it only should be set after the policy has been fully
initialized by the core. That may cause intel_pstate_update_util()
to use incorrect data and put the CPUs into incorrect P-states as
a result.
To prevent that from happening, make intel_pstate_set_policy() set
the utilization update hook instead of intel_pstate_init_cpu() so
intel_pstate_update_util() only runs when all things have been
initialized as appropriate.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
---
drivers/cpufreq/intel_pstate.c | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1103,7 +1103,6 @@ static int intel_pstate_init_cpu(unsigne
intel_pstate_sample(cpu, 0);
cpu->update_util.func = intel_pstate_update_util;
- cpufreq_set_update_util_data(cpunum, &cpu->update_util);
pr_debug("intel_pstate: controlling: cpu %d\n", cpunum);
@@ -1122,18 +1121,29 @@ static unsigned int intel_pstate_get(uns
return get_avg_frequency(cpu);
}
+static void intel_pstate_set_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, &all_cpu_data[cpu]->update_util);
+}
+
+static void intel_pstate_clear_update_util_hook(unsigned int cpu)
+{
+ cpufreq_set_update_util_data(cpu, NULL);
+ synchronize_sched();
+}
+
static int intel_pstate_set_policy(struct cpufreq_policy *policy)
{
if (!policy->cpuinfo.max_freq)
return -ENODEV;
+ intel_pstate_clear_update_util_hook(policy->cpu);
+
if (policy->policy == CPUFREQ_POLICY_PERFORMANCE &&
policy->max >= policy->cpuinfo.max_freq) {
pr_debug("intel_pstate: set performance\n");
limits = &performance_limits;
- if (hwp_active)
- intel_pstate_hwp_set(policy->cpus);
- return 0;
+ goto out;
}
pr_debug("intel_pstate: set powersave\n");
@@ -1163,6 +1173,9 @@ static int intel_pstate_set_policy(struc
limits->max_perf = div_fp(int_tofp(limits->max_perf_pct),
int_tofp(100));
+ out:
+ intel_pstate_set_update_util_hook(policy->cpu);
+
if (hwp_active)
intel_pstate_hwp_set(policy->cpus);
@@ -1187,8 +1200,7 @@ static void intel_pstate_stop_cpu(struct
pr_debug("intel_pstate: CPU %d exiting\n", cpu_num);
- cpufreq_set_update_util_data(cpu_num, NULL);
- synchronize_sched();
+ intel_pstate_clear_update_util_hook(cpu_num);
if (hwp_active)
return;
@@ -1455,8 +1467,7 @@ out:
get_online_cpus();
for_each_online_cpu(cpu) {
if (all_cpu_data[cpu]) {
- cpufreq_set_update_util_data(cpu, NULL);
- synchronize_sched();
+ intel_pstate_clear_update_util_hook(cpu);
kfree(all_cpu_data[cpu]);
}
}
Powered by blists - more mailing lists