lkml.org 
[lkml]   [2016]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/3] intel_pstate: Clarify average performance computation
From
Date
On Sat, 2016-05-07 at 01:44 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@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.wysocki@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

\
 
 \ /
  Last update: 2016-05-10 03:41    [W:0.077 / U:3.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site