Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1607072 > unrolled thread

[PATCH 0/5] cpufreq: intel_pstate: HW support changes, limits rework and documentation

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] cpufreq: intel_pstate: HW support changes, limits rework and documentation "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-23 00:50 +0100
    [PATCH 1/5] cpufreq: intel_pstate: Support HWP processors in all operation modes "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-23 00:50 +0100
    [PATCH 4/5] cpufreq: intel_pstate: Avoid transient updates of cpuinfo.max_freq "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-23 00:50 +0100
    RE: [PATCH 5/5] cpufreq: intel_pstate: Document the current behavior and user interface "Doug Smythies" <dsmythies@telus.net> - 2017-03-27 08:40 +0200
      Re: [PATCH 5/5] cpufreq: intel_pstate: Document the current behavior and user interface "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-03-30 02:30 +0200

#1607072 — [PATCH 0/5] cpufreq: intel_pstate: HW support changes, limits rework and documentation

Hi All,

This series of patches makes changes to intel_pstate that will modify its
behavior in visible ways and add documentation describing the driver's
interface and behavior after those changes.  It is targeted at 4.12.

The functional patches are based on the current linux-next branch of the
linux-pm tree:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=linux-next

and the documentation patch depends on this one:

https://patchwork.kernel.org/patch/9622259/

in the documentation tree now (AFAICS).

Please refer to patch changelogs for more information.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1607073 — [PATCH 1/5] cpufreq: intel_pstate: Support HWP processors in all operation modes

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Currently, some processors supporting HWP are only supported by
intel_pstate if HWP is actually going to be used and not supported
otherwise which is confusing.

Specifically, they are not supported if "intel_pstate=no_hwp" is
passed to the kernel in the command line or if the driver is started
in the passive mode ("intel_pstate=passive").

There is no real reason for that, because everything about those
processor is known anyway and the driver can work with them in all
modes, so make that happen, but use the load-based P-state selection
algorithm for the active mode "powersave" policy with them.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -2627,28 +2627,33 @@ static const struct x86_cpu_id hwp_suppo
 
 static int __init intel_pstate_init(void)
 {
-	const struct x86_cpu_id *id;
-	struct cpu_defaults *cpu_def;
-	int rc = 0;
+	int rc;
 
 	if (no_load)
 		return -ENODEV;
 
-	if (x86_match_cpu(hwp_support_ids) && !no_hwp) {
+	if (x86_match_cpu(hwp_support_ids)) {
 		copy_cpu_funcs(&core_params.funcs);
-		hwp_active++;
-		intel_pstate.attr = hwp_cpufreq_attrs;
-		goto hwp_cpu_matched;
-	}
-
-	id = x86_match_cpu(intel_pstate_cpu_ids);
-	if (!id)
-		return -ENODEV;
+		if (no_hwp) {
+			pstate_funcs.get_target_pstate = get_target_pstate_use_cpu_load;
+		} else {
+			hwp_active++;
+			intel_pstate.attr = hwp_cpufreq_attrs;
+			goto hwp_cpu_matched;
+		}
+	} else {
+		const struct x86_cpu_id *id;
+		struct cpu_defaults *cpu_def;
+
+		id = x86_match_cpu(intel_pstate_cpu_ids);
+		if (!id)
+			return -ENODEV;
 
-	cpu_def = (struct cpu_defaults *)id->driver_data;
+		cpu_def = (struct cpu_defaults *)id->driver_data;
 
-	copy_pid_params(&cpu_def->pid_policy);
-	copy_cpu_funcs(&cpu_def->funcs);
+		copy_pid_params(&cpu_def->pid_policy);
+		copy_cpu_funcs(&cpu_def->funcs);
+	}
 
 	if (intel_pstate_msrs_not_valid())
 		return -ENODEV;

[toc] | [prev] | [next] | [standalone]

#1607075 — [PATCH 4/5] cpufreq: intel_pstate: Avoid transient updates of cpuinfo.max_freq

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Both intel_pstate_verify_policy() and intel_cpufreq_verify_policy()
set policy->cpuinfo.max_freq depending on the turbo status, but the
updates made by them are discarded by the core, because the policy
object passed to them by the core is temporary and cpuinfo.max_freq
from that object is not copied to the final policy object in
cpufreq_set_policy().

However, cpufreq_set_policy() passes the temporary policy object
to the ->setpolicy callback of the driver, so intel_pstate_set_policy()
actually sees the policy->cpuinfo.max_freq value updated by
intel_pstate_verify_policy() and not the final one.  It also
updates policy->max sometimes which basically has no effect after
it returns, because the core discards that update.

To avoid confusion, eliminate policy->cpuinfo.max_freq updates from
intel_pstate_verify_policy() and intel_cpufreq_verify_policy()
entirely and check the maximum frequency explicitly in
intel_pstate_update_perf_limits() instead of relying on the
transiently updated policy->cpuinfo.max_freq value.

Moreover, move the max->policy adjustment carried out in
intel_pstate_set_policy() to a separate function and call that
function from the ->verify driver callbacks to ensure that it will
actually be effective.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/intel_pstate.c |   47 ++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 19 deletions(-)

Index: linux-pm/drivers/cpufreq/intel_pstate.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/intel_pstate.c
+++ linux-pm/drivers/cpufreq/intel_pstate.c
@@ -2021,19 +2021,25 @@ static void intel_pstate_clear_update_ut
 	synchronize_sched();
 }
 
+static int intel_pstate_get_max_freq(struct cpudata *cpu)
+{
+	return global.turbo_disabled || global.no_turbo ?
+			cpu->pstate.max_freq : cpu->pstate.turbo_freq;
+}
+
 static void intel_pstate_update_perf_limits(struct cpufreq_policy *policy,
 					    struct cpudata *cpu)
 {
 	struct perf_limits *limits = &cpu->perf_limits;
+	int max_freq = intel_pstate_get_max_freq(cpu);
 	int32_t max_policy_perf, min_policy_perf;
 
-	max_policy_perf = div_ext_fp(policy->max, policy->cpuinfo.max_freq);
+	max_policy_perf = div_ext_fp(policy->max, max_freq);
 	max_policy_perf = clamp_t(int32_t, max_policy_perf, 0, int_ext_tofp(1));
 	if (policy->max == policy->min) {
 		min_policy_perf = max_policy_perf;
 	} else {
-		min_policy_perf = div_ext_fp(policy->min,
-					     policy->cpuinfo.max_freq);
+		min_policy_perf = div_ext_fp(policy->min, max_freq);
 		min_policy_perf = clamp_t(int32_t, min_policy_perf,
 					  0, max_policy_perf);
 	}
@@ -2048,7 +2054,7 @@ static void intel_pstate_update_perf_lim
 		/* Global limits are in percent of the maximum turbo P-state. */
 		global_max = percent_ext_fp(global.max_perf_pct);
 		global_min = percent_ext_fp(global.min_perf_pct);
-		if (policy->cpuinfo.max_freq != cpu->pstate.turbo_freq) {
+		if (max_freq != cpu->pstate.turbo_freq) {
 			int32_t turbo_factor;
 
 			turbo_factor = div_ext_fp(cpu->pstate.turbo_pstate,
@@ -2088,13 +2094,6 @@ static int intel_pstate_set_policy(struc
 	cpu = all_cpu_data[policy->cpu];
 	cpu->policy = policy->policy;
 
-	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) {
-		pr_debug("policy->max > max non turbo frequency\n");
-		policy->max = policy->cpuinfo.max_freq;
-	}
-
 	mutex_lock(&intel_pstate_limits_lock);
 
 	intel_pstate_update_perf_limits(policy, cpu);
@@ -2118,21 +2117,31 @@ static int intel_pstate_set_policy(struc
 	return 0;
 }
 
+static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
+					 struct cpudata *cpu)
+{
+	if (cpu->pstate.max_pstate_physical > cpu->pstate.max_pstate &&
+	    policy->max < policy->cpuinfo.max_freq &&
+	    policy->max > cpu->pstate.max_freq) {
+		pr_debug("policy->max > max non turbo frequency\n");
+		policy->max = policy->cpuinfo.max_freq;
+	}
+}
+
 static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 {
 	struct cpudata *cpu = all_cpu_data[policy->cpu];
 
 	update_turbo_state();
-	policy->cpuinfo.max_freq = global.turbo_disabled || global.no_turbo ?
-					cpu->pstate.max_freq :
-					cpu->pstate.turbo_freq;
-
-	cpufreq_verify_within_cpu_limits(policy);
+	cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
+				     intel_pstate_get_max_freq(cpu));
 
 	if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
 	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)
 		return -EINVAL;
 
+	intel_pstate_adjust_policy_max(policy, cpu);
+
 	return 0;
 }
 
@@ -2227,10 +2236,10 @@ static int intel_cpufreq_verify_policy(s
 	struct cpudata *cpu = all_cpu_data[policy->cpu];
 
 	update_turbo_state();
-	policy->cpuinfo.max_freq = global.no_turbo || global.turbo_disabled ?
-			cpu->pstate.max_freq : cpu->pstate.turbo_freq;
+	cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
+				     intel_pstate_get_max_freq(cpu));
 
-	cpufreq_verify_within_cpu_limits(policy);
+	intel_pstate_adjust_policy_max(policy, cpu);
 
 	intel_pstate_update_perf_limits(policy, cpu);
 

[toc] | [prev] | [next] | [standalone]

#1609506 — RE: [PATCH 5/5] cpufreq: intel_pstate: Document the current behavior and user interface

On 2017.03.22 16:32 Rafael J. Wysocki wrote:

I realize that there is tradeoff between a succinct and brief
document and having to write a full book, but I have a couple of
comments anyhow.

> Add a document describing the current behavior and user space
> interface of the intel_pstate driver in the RST format and
> drop the existing outdated intel_pstate.txt document.

... [cut]...

> +The second variant of the ``powersave`` P-state selection algorithm, used in all
> +of the other cases (generally, on processors from the Core line, so it is
> +referred to as the "Core" algorithm), is based on the values read from the APERF
> +and MPERF feedback registers alone

And target pstate over the last sample interval.

> and it does not really take CPU utilization
> +into account explicitly.  Still, it causes the CPU P-state to ramp up very
> +quickly in response to increased utilization which is generally desirable in
> +server environments.

It will only ramp up quickly if another CPU has already ramped up such that the
effective pstate is much higher than the target, giving a very very high "load"
(actually scaled_busy) see comments further down.

... [cut]...

> +Turbo P-states Support
> +======================
...
> +Some processors allow multiple cores to be in turbo P-states at the same time,
> +but the maximum P-state that can be set for them generally depends on the number
> +of cores running concurrently.  The maximum turbo P-state that can be set for 3
> +cores at the same time usually is lower than the analogous maximum P-state for
> +2 cores, which in turn usually is lower than the maximum turbo P-state that can
> +be set for 1 core.  The one-core maximum turbo P-state is thus the maximum
> +supported one overall.

The above segment was retained because it is relevant to footnote 1 below.

...[cut]...

> +For example, the default values of the PID controller parameters for the Sandy
> +Bridge generation of processors are
> +
> +| ``deadband`` = 0
> +| ``d_gain_pct`` = 0
> +| ``i_gain_pct`` = 0
> +| ``p_gain_pct`` = 20
> +| ``sample_rate_ms`` = 10
> +| ``setpoint`` = 97
> +
> +If the derivative and integral coefficients in the PID algorithm are both equal
> +to 0 (which is the case above), the next P-State value will be equal to:
> +
> +  ``current_pstate`` - ((``setpoint`` - ``current_load``) * ``p_gain_pct``)
> +
> +where ``current_pstate`` is the P-state currently set for the given CPU and
> +``current_load`` is the current load estimate for it based on the current values
> +of feedback registers.

While mentioned earlier, it should be emphasized again here that this
"current_load" might be, and very often is, very very different than
the actual load on the CPU. It can be as high as the ratio of the maximum
P state / minimum P state. I.E. for my older i7 processor it can be
38/16 *100% = 237.5%. For more recent processors, that maximum can be much
higher. This is how this control algorithm can achieve a very rapid ramp
of pstate on a CPU that was previously idle, with these settings, and when
other CPUs were already active and ramped up.

> +
> +If ``current_pstate`` is 8 (in the internal representation used by
> +``intel_pstate``) and ``current_load`` is 100 (in percent), the next P-state
> +value will be:
> +
> +	8 - ((97 - 100) * 0.2) = 8.6
> +
> +which will be rounded up to 9, so the P-state value goes up by 1 in this case.
> +If the load does not change during the next interval between invocations of the
> +driver's utilization update callback for the CPU in question, the P-state value
> +will go up by 1 again and so on, as long as the load exceeds the ``setpoint``
> +value (or until the maximum P-state is reached).

No, only if the "load" exceeds the setpoint by at least 0.5/p_gain+setpoint,
Or for these settings, 99.5. The point being that p_gain and setpoint effect
each other in terms of system response.

Suggest it would be worth a fast ramp up example here. Something like:
Minimum pstate = 16; Maximum pstate = 38.

Current pstate = 16,
Effective pstate over the last interval, due to another CPU = 38
"load" = 237.5%

16 - ((97-237.5) * 0.2) = 44.1, which would be clamped to 38.

Footnote 1: Readers might argue that, due to multiple cores being active
at one time, we would never actually get a "load" of 237.5 in the above example.
That is true, but it can get very very close. For simplicity of the example, the
suggestion is to ignore it.
A real trace data sample fast ramp up example:

mperf: 9806829 cycles
apref: 10936506 cycles
tsc: 99803828 cycles
freq: 3.7916 GHz ; effective pstate 37.9
old target pstate: 16
duration: 29.26 milliseconds
load (actual): 9.83%
"load" (scaled)busy): 236
New target pstate: 38
  
... Doug

[toc] | [prev] | [next] | [standalone]

#1612459 — Re: [PATCH 5/5] cpufreq: intel_pstate: Document the current behavior and user interface

On Sunday, March 26, 2017 11:32:37 PM Doug Smythies wrote:
> On 2017.03.22 16:32 Rafael J. Wysocki wrote:
> 
> I realize that there is tradeoff between a succinct and brief
> document and having to write a full book, but I have a couple of
> comments anyhow.
> 
> > Add a document describing the current behavior and user space
> > interface of the intel_pstate driver in the RST format and
> > drop the existing outdated intel_pstate.txt document.
> 
> ... [cut]...
> 
> > +The second variant of the ``powersave`` P-state selection algorithm, used in all
> > +of the other cases (generally, on processors from the Core line, so it is
> > +referred to as the "Core" algorithm), is based on the values read from the APERF
> > +and MPERF feedback registers alone
> 
> And target pstate over the last sample interval.

Fair enough.

> > and it does not really take CPU utilization
> > +into account explicitly.  Still, it causes the CPU P-state to ramp up very
> > +quickly in response to increased utilization which is generally desirable in
> > +server environments.
> 
> It will only ramp up quickly if another CPU has already ramped up such that the
> effective pstate is much higher than the target, giving a very very high "load"
> (actually scaled_busy) see comments further down.

I really wouldn't like to go into too much detail here.

I'm about to write something along these lines:

"It does not really take CPU utilization into account explicitly, but as a rule it
causes the CPU P-state to ramp up [...]".

> ... [cut]...
> 
> > +Turbo P-states Support
> > +======================
> ...
> > +Some processors allow multiple cores to be in turbo P-states at the same time,
> > +but the maximum P-state that can be set for them generally depends on the number
> > +of cores running concurrently.  The maximum turbo P-state that can be set for 3
> > +cores at the same time usually is lower than the analogous maximum P-state for
> > +2 cores, which in turn usually is lower than the maximum turbo P-state that can
> > +be set for 1 core.  The one-core maximum turbo P-state is thus the maximum
> > +supported one overall.
> 
> The above segment was retained because it is relevant to footnote 1 below.
> 
> ...[cut]...
> 
> > +For example, the default values of the PID controller parameters for the Sandy
> > +Bridge generation of processors are
> > +
> > +| ``deadband`` = 0
> > +| ``d_gain_pct`` = 0
> > +| ``i_gain_pct`` = 0
> > +| ``p_gain_pct`` = 20
> > +| ``sample_rate_ms`` = 10
> > +| ``setpoint`` = 97
> > +
> > +If the derivative and integral coefficients in the PID algorithm are both equal
> > +to 0 (which is the case above), the next P-State value will be equal to:
> > +
> > +  ``current_pstate`` - ((``setpoint`` - ``current_load``) * ``p_gain_pct``)
> > +
> > +where ``current_pstate`` is the P-state currently set for the given CPU and
> > +``current_load`` is the current load estimate for it based on the current values
> > +of feedback registers.
> 
> While mentioned earlier, it should be emphasized again here that this
> "current_load" might be, and very often is, very very different than
> the actual load on the CPU. It can be as high as the ratio of the maximum
> P state / minimum P state. I.E. for my older i7 processor it can be
> 38/16 *100% = 237.5%. For more recent processors, that maximum can be much
> higher. This is how this control algorithm can achieve a very rapid ramp
> of pstate on a CPU that was previously idle, with these settings, and when
> other CPUs were already active and ramped up.

I actually copied this part from the existing intel_pstate.txt document and only
edited it somewhat.  Now I realize that it really was not too accurate at all
originally.

I think I'll simply skip the entire example part of this section, as the original
simply doesn't reflect the reality and I don't think it's particularly useful
to try to describe it more accurately here.

Thanks,
Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web