[PATCH] Added multiplier for APERF and MPERF counters

[PATCH] Added multiplier for APERF and MPERF counters

[Lukasz Anaczkowski]

From: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>

Intel(R) Xeon Phi x200 (KNL) increments APERF/MPERF every
1024 clocks, which is different than other Intel architectures
(incremented every single clock).
It results in understated values for %Busy and Avg_MHz values,
thus multiplier for APERF/MPERF counters.

Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>
---
 tools/power/x86/turbostat/turbostat.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 9655cb4..1dda607 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -71,6 +71,7 @@ unsigned int extra_msr_offset32;
 unsigned int extra_msr_offset64;
 unsigned int extra_delta_offset32;
 unsigned int extra_delta_offset64;
+unsigned int aperf_mperf_multiplier = 1;
 int do_smi;
 double bclk;
 unsigned int show_pkg;
@@ -984,6 +985,8 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
 			return -3;
 		if (get_msr(cpu, MSR_IA32_MPERF, &t->mperf))
 			return -4;
+		t->aperf = t->aperf * aperf_mperf_multiplier;
+		t->mperf = t->mperf * aperf_mperf_multiplier;
 	}
 
 	if (do_smi) {
@@ -2543,6 +2546,13 @@ int is_knl(unsigned int family, unsigned int model)
 	return 0;
 }
 
+unsigned int get_aperf_mperf_multiplier(unsigned int family, unsigned int model)
+{
+	if (is_knl(family, model))
+		return 1024;
+	return 1;
+}
+
 #define SLM_BCLK_FREQS 5
 double slm_freq_table[SLM_BCLK_FREQS] = { 83.3, 100.0, 133.3, 116.7, 80.0};
 
@@ -2744,6 +2754,9 @@ void process_cpuid()
 		}
 	}
 
+	if (has_aperf)
+		aperf_mperf_multiplier = get_aperf_mperf_multiplier(family, model);
+
 	do_nhm_platform_info = do_nhm_cstates = do_smi = probe_nhm_msrs(family, model);
 	do_snb_cstates = has_snb_msrs(family, model);
 	do_pc2 = do_snb_cstates && (pkg_cstate_limit >= PCL__2);
-- 
1.8.3.1

Re: [PATCH] Added multiplier for APERF and MPERF counters

[Rafael J. Wysocki]

On Wednesday, September 16, 2015 02:52:18 PM Lukasz Anaczkowski wrote:
> From: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
> 
> Intel(R) Xeon Phi x200 (KNL) increments APERF/MPERF every
> 1024 clocks, which is different than other Intel architectures
> (incremented every single clock).
> It results in understated values for %Busy and Avg_MHz values,
> thus multiplier for APERF/MPERF counters.
> 
> Signed-off-by: Hubert Chrzaniuk <hubert.chrzaniuk@intel.com>
> Signed-off-by: Lukasz Anaczkowski <lukasz.anaczkowski@intel.com>

Please *always* send PM-related patches to linux-pm@vger.kernel.org (CCed now).

> ---
>  tools/power/x86/turbostat/turbostat.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 9655cb4..1dda607 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -71,6 +71,7 @@ unsigned int extra_msr_offset32;
>  unsigned int extra_msr_offset64;
>  unsigned int extra_delta_offset32;
>  unsigned int extra_delta_offset64;
> +unsigned int aperf_mperf_multiplier = 1;
>  int do_smi;
>  double bclk;
>  unsigned int show_pkg;
> @@ -984,6 +985,8 @@ int get_counters(struct thread_data *t, struct core_data *c, struct pkg_data *p)
>  			return -3;
>  		if (get_msr(cpu, MSR_IA32_MPERF, &t->mperf))
>  			return -4;
> +		t->aperf = t->aperf * aperf_mperf_multiplier;
> +		t->mperf = t->mperf * aperf_mperf_multiplier;
>  	}
>  
>  	if (do_smi) {
> @@ -2543,6 +2546,13 @@ int is_knl(unsigned int family, unsigned int model)
>  	return 0;
>  }
>  
> +unsigned int get_aperf_mperf_multiplier(unsigned int family, unsigned int model)
> +{
> +	if (is_knl(family, model))
> +		return 1024;
> +	return 1;
> +}
> +
>  #define SLM_BCLK_FREQS 5
>  double slm_freq_table[SLM_BCLK_FREQS] = { 83.3, 100.0, 133.3, 116.7, 80.0};
>  
> @@ -2744,6 +2754,9 @@ void process_cpuid()
>  		}
>  	}
>  
> +	if (has_aperf)
> +		aperf_mperf_multiplier = get_aperf_mperf_multiplier(family, model);
> +
>  	do_nhm_platform_info = do_nhm_cstates = do_smi = probe_nhm_msrs(family, model);
>  	do_snb_cstates = has_snb_msrs(family, model);
>  	do_pc2 = do_snb_cstates && (pkg_cstate_limit >= PCL__2);
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH] Added multiplier for APERF and MPERF counters

[Anaczkowski, Lukasz]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 628 bytes --]

From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] 
Sent: Thursday, September 17, 2015 7:05 PM

> Please *always* send PM-related patches to linux-pm@vger.kernel.org (CCed now).

Ok, thanks for noticing.
I've addressed my patch based on output of ./scripts/get_maintainer.pl
and gave me output like this:

Len Brown <len.brown@xxx>
Dasaratharaman Chandramouli <dasaratharaman.chandramouli@xxx>
linux-kernel@vger.kernel.org

Should it be fixed somehow?

Cheers,
Lukasz
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] Added multiplier for APERF and MPERF counters

[Rafael J. Wysocki]

On Friday, September 18, 2015 07:08:47 AM Anaczkowski, Lukasz wrote:
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] 
> Sent: Thursday, September 17, 2015 7:05 PM
> 
> > Please *always* send PM-related patches to linux-pm@vger.kernel.org (CCed now).
> 
> Ok, thanks for noticing.
> I've addressed my patch based on output of ./scripts/get_maintainer.pl
> and gave me output like this:
> 
> Len Brown <len.brown@xxx>
> Dasaratharaman Chandramouli <dasaratharaman.chandramouli@xxx>
> linux-kernel@vger.kernel.org
> 
> Should it be fixed somehow?

Well, so to speak, the script is not an intelligent entity and while it can
give you hints, it by no means is an ultimate source of information.

If you did "grep -r -I -l APERF *" in the kernel source tree, you'll
immediately see that PM uses it.

And while at it, we really should only be using the APERF/MPERF ratio so I'm
not sure why this patch actually matters?

Thanks,
Rafael