* [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode
@ 2016-10-19 0:57 Rafael J. Wysocki
2016-10-20 18:22 ` Srinivas Pandruvada
2016-10-21 15:08 ` Srinivas Pandruvada
0 siblings, 2 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2016-10-19 0:57 UTC (permalink / raw)
To: Srinivas Pandruvada, Linux PM list; +Cc: Linux Kernel Mailing List
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
After commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with
utilization update callbacks) the cpufreq governor callbacks may not
be invoked on NOHZ_FULL CPUs and, in particular, switching to the
"performance" policy via sysfs may not have any effect on them. That
is a problem, because it usually is desirable to squeeze the last
bit of performance out of those CPUs, so work around it by setting
the maximum P-state (within the limits) in intel_pstate_set_policy()
upfront when the policy is CPUFREQ_POLICY_PERFORMANCE.
Fixes: a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with utilization update callbacks)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/intel_pstate.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -1137,10 +1137,8 @@ static void intel_pstate_get_min_max(str
*min = clamp_t(int, min_perf, cpu->pstate.min_pstate, max_perf);
}
-static void intel_pstate_set_min_pstate(struct cpudata *cpu)
+static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
{
- int pstate = cpu->pstate.min_pstate;
-
trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
cpu->pstate.current_pstate = pstate;
/*
@@ -1152,6 +1150,20 @@ static void intel_pstate_set_min_pstate(
pstate_funcs.get_val(cpu, pstate));
}
+static void intel_pstate_set_min_pstate(struct cpudata *cpu)
+{
+ intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
+}
+
+static void intel_pstate_max_within_limits(struct cpudata *cpu)
+{
+ int min_pstate, max_pstate;
+
+ update_turbo_state();
+ intel_pstate_get_min_max(cpu, &min_pstate, &max_pstate);
+ intel_pstate_set_pstate(cpu, max_pstate);
+}
+
static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
{
cpu->pstate.min_pstate = pstate_funcs.get_min();
@@ -1485,7 +1497,7 @@ static int intel_pstate_set_policy(struc
pr_debug("set_policy cpuinfo.max %u policy->max %u\n",
policy->cpuinfo.max_freq, policy->max);
- cpu = all_cpu_data[0];
+ cpu = all_cpu_data[policy->cpu];
if (cpu->pstate.max_pstate_physical > cpu->pstate.max_pstate &&
policy->max < policy->cpuinfo.max_freq &&
policy->max > cpu->pstate.max_pstate * cpu->pstate.scaling) {
@@ -1529,6 +1541,15 @@ static int intel_pstate_set_policy(struc
limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
out:
+ if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
+ /*
+ * NOHZ_FULL CPUs need this as the governor callback may not
+ * be invoked on them.
+ */
+ intel_pstate_clear_update_util_hook(policy->cpu);
+ intel_pstate_max_within_limits(cpu);
+ }
+
intel_pstate_set_update_util_hook(policy->cpu);
intel_pstate_hwp_set_policy(policy);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode
2016-10-19 0:57 [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode Rafael J. Wysocki
@ 2016-10-20 18:22 ` Srinivas Pandruvada
2016-10-20 21:19 ` Rafael J. Wysocki
2016-10-21 15:08 ` Srinivas Pandruvada
1 sibling, 1 reply; 4+ messages in thread
From: Srinivas Pandruvada @ 2016-10-20 18:22 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM list; +Cc: Linux Kernel Mailing List
On Wed, 2016-10-19 at 02:57 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
[...]
> + if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
> + /*
> + * NOHZ_FULL CPUs need this as the governor callback
> may not
> + * be invoked on them.
> + */
> + intel_pstate_clear_update_util_hook(policy->cpu);
> + intel_pstate_max_within_limits(cpu);
> + }
> +
Can we move this to intel_pstate_set_performance_limits
diff --git a/drivers/cpufreq/intel_pstate.c
b/drivers/cpufreq/intel_pstate.c
index a6ffd79..d0fd73e 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1543,6 +1543,13 @@ static void
intel_pstate_set_performance_limits(struct perf_limits *limits)
limits->max_sysfs_pct = 100;
limits->min_policy_pct = 0;
limits->min_sysfs_pct = 0;
+
+ /*
+ * NOHZ_FULL CPUs need this as the governor callback may not
+ * be invoked on them.
+ */
+ intel_pstate_clear_update_util_hook(policy->cpu);
+ intel_pstate_max_within_limits(cpu);
}
static int intel_pstate_set_policy(struct cpufreq_policy *policy)
@@ -1599,15 +1606,6 @@ static int intel_pstate_set_policy(struct
cpufreq_policy *policy)
limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
out:
- if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
- /*
- * NOHZ_FULL CPUs need this as the governor callback
may not
- * be invoked on them.
- */
- intel_pstate_clear_update_util_hook(policy->cpu);
- intel_pstate_max_within_limits(cpu);
- }
-
intel_pstate_set_update_util_hook(policy->cpu);
intel_pstate_hwp_set_policy(policy);
Thanks,
Srinivas
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode
2016-10-20 18:22 ` Srinivas Pandruvada
@ 2016-10-20 21:19 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2016-10-20 21:19 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: Rafael J. Wysocki, Linux PM list, Linux Kernel Mailing List
On Thu, Oct 20, 2016 at 8:22 PM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Wed, 2016-10-19 at 02:57 +0200, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>
> [...]
>
>> + if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
>> + /*
>> + * NOHZ_FULL CPUs need this as the governor callback
>> may not
>> + * be invoked on them.
>> + */
>> + intel_pstate_clear_update_util_hook(policy->cpu);
>> + intel_pstate_max_within_limits(cpu);
>> + }
>> +
>
> Can we move this to intel_pstate_set_performance_limits
>
> diff --git a/drivers/cpufreq/intel_pstate.c
> b/drivers/cpufreq/intel_pstate.c
> index a6ffd79..d0fd73e 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1543,6 +1543,13 @@ static void
> intel_pstate_set_performance_limits(struct perf_limits *limits)
> limits->max_sysfs_pct = 100;
> limits->min_policy_pct = 0;
> limits->min_sysfs_pct = 0;
> +
> + /*
> + * NOHZ_FULL CPUs need this as the governor callback may not
> + * be invoked on them.
> + */
> + intel_pstate_clear_update_util_hook(policy->cpu);
> + intel_pstate_max_within_limits(cpu);
> }
>
> static int intel_pstate_set_policy(struct cpufreq_policy *policy)
> @@ -1599,15 +1606,6 @@ static int intel_pstate_set_policy(struct
> cpufreq_policy *policy)
> limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
>
> out:
> - if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
> - /*
> - * NOHZ_FULL CPUs need this as the governor callback
> may not
> - * be invoked on them.
> - */
> - intel_pstate_clear_update_util_hook(policy->cpu);
> - intel_pstate_max_within_limits(cpu);
> - }
> -
> intel_pstate_set_update_util_hook(policy->cpu);
>
> intel_pstate_hwp_set_policy(policy);
Not really, because the policy->max < policy->cpuinfo.max_freq case
needs to be covered too.
At least I don't see why it shouldn't be covered.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode
2016-10-19 0:57 [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode Rafael J. Wysocki
2016-10-20 18:22 ` Srinivas Pandruvada
@ 2016-10-21 15:08 ` Srinivas Pandruvada
1 sibling, 0 replies; 4+ messages in thread
From: Srinivas Pandruvada @ 2016-10-21 15:08 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM list; +Cc: Linux Kernel Mailing List
On Wed, 2016-10-19 at 02:57 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> After commit a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with
> utilization update callbacks) the cpufreq governor callbacks may not
> be invoked on NOHZ_FULL CPUs and, in particular, switching to the
> "performance" policy via sysfs may not have any effect on them. That
> is a problem, because it usually is desirable to squeeze the last
> bit of performance out of those CPUs, so work around it by setting
> the maximum P-state (within the limits) in intel_pstate_set_policy()
> upfront when the policy is CPUFREQ_POLICY_PERFORMANCE.
>
> Fixes: a4675fbc4a7a (cpufreq: intel_pstate: Replace timers with
> utilization update callbacks)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1137,10 +1137,8 @@ static void intel_pstate_get_min_max(str
> *min = clamp_t(int, min_perf, cpu->pstate.min_pstate,
> max_perf);
> }
>
> -static void intel_pstate_set_min_pstate(struct cpudata *cpu)
> +static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
> {
> - int pstate = cpu->pstate.min_pstate;
> -
> trace_cpu_frequency(pstate * cpu->pstate.scaling, cpu->cpu);
> cpu->pstate.current_pstate = pstate;
> /*
> @@ -1152,6 +1150,20 @@ static void intel_pstate_set_min_pstate(
> pstate_funcs.get_val(cpu, pstate));
> }
>
> +static void intel_pstate_set_min_pstate(struct cpudata *cpu)
> +{
> + intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate);
> +}
> +
> +static void intel_pstate_max_within_limits(struct cpudata *cpu)
> +{
> + int min_pstate, max_pstate;
> +
> + update_turbo_state();
> + intel_pstate_get_min_max(cpu, &min_pstate, &max_pstate);
> + intel_pstate_set_pstate(cpu, max_pstate);
> +}
> +
> static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
> {
> cpu->pstate.min_pstate = pstate_funcs.get_min();
> @@ -1485,7 +1497,7 @@ static int intel_pstate_set_policy(struc
> pr_debug("set_policy cpuinfo.max %u policy->max %u\n",
> policy->cpuinfo.max_freq, policy->max);
>
> - cpu = all_cpu_data[0];
> + cpu = all_cpu_data[policy->cpu];
> if (cpu->pstate.max_pstate_physical > cpu->pstate.max_pstate
> &&
> policy->max < policy->cpuinfo.max_freq &&
> policy->max > cpu->pstate.max_pstate * cpu-
> >pstate.scaling) {
> @@ -1529,6 +1541,15 @@ static int intel_pstate_set_policy(struc
> limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
>
> out:
> + if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
> + /*
> + * NOHZ_FULL CPUs need this as the governor callback
> may not
> + * be invoked on them.
> + */
> + intel_pstate_clear_update_util_hook(policy->cpu);
> + intel_pstate_max_within_limits(cpu);
> + }
> +
> intel_pstate_set_update_util_hook(policy->cpu);
>
> intel_pstate_hwp_set_policy(policy);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-21 15:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-19 0:57 [PATCH] cpufreq: intel_pstate: Set P-state upfront in performance mode Rafael J. Wysocki
2016-10-20 18:22 ` Srinivas Pandruvada
2016-10-20 21:19 ` Rafael J. Wysocki
2016-10-21 15:08 ` Srinivas Pandruvada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).