On Sat, 2016-05-07 at 01:44 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
>
> The core_pct_busy field of struct sample actually contains the
> average performace during the last sampling period (in percent)
> and not the utilization of the core as suggested by its name
> which is confusing.
>
> For this reason, change the name of that field to core_avg_perf
> and rename the function that computes its value accordingly.
>
Makes perfect sense.
> Also notice that it would be more useful if it was a "raw" fraction
> rather than percentage, so change its meaning too and update the
> code using it accordingly (it is better to change the name of
> the field along with its meaning in one go than to make those
> two changes separately, as that would likely lead to more
> confusion).
Due to the calculation the results from old and new method will be
similar but not same. For example in one scenario the
get_avg_frequency difference is 4.3KHz (printed side by side using both
old style using pct and new using fraction)
Frequency with old calc: 2996093 Hz
Frequency with old calc: 3000460 Hz
How much do you think the performance gain changing fraction vs pct?
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
> ---
> drivers/cpufreq/intel_pstate.c | 24 ++++++++++--------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -72,10 +72,10 @@ static inline int ceiling_fp(int32_t x)
>
> /**
> * struct sample - Store performance sample
> - * @core_pct_busy: Ratio of APERF/MPERF in percent, which is
> actual
> + * @core_avg_perf: Ratio of APERF/MPERF which is the actual
> average
> * performance during last sample period
> * @busy_scaled: Scaled busy value which is used to calculate
> next
> - * P state. This can be different than
> core_pct_busy
> + * P state. This can be different than
> core_avg_perf
> * to account for cpu idle period
> * @aperf: Difference of actual performance frequency
> clock count
> * read from APERF MSR between last and
> current sample
> @@ -90,7 +90,7 @@ static inline int ceiling_fp(int32_t x)
> * data for choosing next P State.
> */
> struct sample {
> - int32_t core_pct_busy;
> + int32_t core_avg_perf;
> int32_t busy_scaled;
> u64 aperf;
> u64 mperf;
> @@ -1147,15 +1147,11 @@ static void intel_pstate_get_cpu_pstates
> intel_pstate_set_min_pstate(cpu);
> }
>
> -static inline void intel_pstate_calc_busy(struct cpudata *cpu)
> +static inline void intel_pstate_calc_avg_perf(struct cpudata *cpu)
> {
> struct sample *sample = &cpu->sample;
> - int64_t core_pct;
>
> - core_pct = sample->aperf * int_tofp(100);
> - core_pct = div64_u64(core_pct, sample->mperf);
> -
> - sample->core_pct_busy = (int32_t)core_pct;
> + sample->core_avg_perf = div_fp(sample->aperf, sample-
> >mperf);
> }
>
> static inline bool intel_pstate_sample(struct cpudata *cpu, u64
> time)
> @@ -1198,9 +1194,9 @@ static inline bool intel_pstate_sample(s
>
> static inline int32_t get_avg_frequency(struct cpudata *cpu)
> {
> - return fp_toint(mul_fp(cpu->sample.core_pct_busy,
> + return fp_toint(mul_fp(cpu->sample.core_avg_perf,
> int_tofp(cpu-
> >pstate.max_pstate_physical *
> - cpu->pstate.scaling
> / 100)));
> + cpu-
> >pstate.scaling)));
> }
>
> static inline int32_t get_avg_pstate(struct cpudata *cpu)
> @@ -1260,7 +1256,7 @@ static inline int32_t get_target_pstate_
> * period. The result will be a percentage of busy at a
> * specified pstate.
> */
> - core_busy = cpu->sample.core_pct_busy;
> + core_busy = 100 * cpu->sample.core_avg_perf;
> max_pstate = cpu->pstate.max_pstate_physical;
> current_pstate = cpu->pstate.current_pstate;
> core_busy = mul_fp(core_busy, div_fp(max_pstate,
> current_pstate));
> @@ -1312,7 +1308,7 @@ static inline void intel_pstate_adjust_b
> intel_pstate_update_pstate(cpu, target_pstate);
>
> sample = &cpu->sample;
> - trace_pstate_sample(fp_toint(sample->core_pct_busy),
> + trace_pstate_sample(fp_toint(100 * sample->core_avg_perf),
> fp_toint(sample->busy_scaled),
> from,
> cpu->pstate.current_pstate,
> @@ -1332,7 +1328,7 @@ static void intel_pstate_update_util(str
> bool sample_taken = intel_pstate_sample(cpu, time);
>
> if (sample_taken) {
> - intel_pstate_calc_busy(cpu);
> + intel_pstate_calc_avg_perf(cpu);
> if (!hwp_active)
> intel_pstate_adjust_busy_pstate(cpu)
> ;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
