acpi-cpufreq: Move average performance code and make it available to userspace and several cleanups in this area

acpi-cpufreq: Move average performance code and make it available to userspace and several cleanups in this area

[Thomas Renninger]

This patch series fixes following bugs:
     - if(!work_on_cpu..) must be if(work_on_cpu..), therefore always zero
       was returned for the average performance
     - use smp_call_function_single instead of work_on_cpu as suggested by
       Andrew Morton (will conflict slightly with his latest patch).
     - brackets forgotten when dividing
     - Handle overflow of MSR registers gracefully

Cleanups:
     - Make use of printk_once interface in acpi-cpufreq.c
     - Get rid of internal acpi-cpufreq max_freq per_cpu variable and use
       the already available policy->cpuinfo.max_freq instead
     - Do not read out cpuid again, but use the already available
       X86_FEATURE_IDA to detect whether MPERF/APERF registers can be used

New functionality:
     - Split up average performance code for re-use
     - Make cpufreq_stats show average performance in:
       /sys/devices/system/cpu/cpu*/cpufreq/stats/average_freq

This code patches against Len's latest acpi test tree.
The first patch is the Null pointer fix from Venkatesh posted on acpi-cpufreq
recently, subject:
[PATCH] x86, acpi_cpufreq: Fix the NULL pointer dereference in get_measured_perf

The patch from Andrew Morton which converts work_on_cpu to
smp_call_function_single at several places will not patch anymore. He sent
the patch on the acpi list recently, subject:
[patch for 2.6.30 1/2] arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c: avoid using work_on_cpu()
It would be nice when my patches can go in first and Andrew's gets adjusted
(I can do that then). It's easier this way round.

The algorithm to calculate the average frequency from the MSRs has not been
touched.
It would be great if this could still find it's way into 2.6.30.
If not, the fixes should probably be picked up.

Thanks,

     Thomas

[PATCH 1/8] Fix for a regression that was introduced by earlier commit

[Thomas Renninger]

From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

Regression resulted in the below error happened on systems with
software coordination where per_cpu acpi data will not be initiated for
secondary CPUs in a P-state domain.

BUG: unable to handle kernel NULL pointer dereference at
0000000000000020
IP: [<ffffffff8021af75>] get_measured_perf+0x4a/0xf9
PGD a7dd88067 PUD a7ccf5067 PMD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
CPU 0
Modules linked in: video output
Pid: 2091, comm: kondemand/0 Not tainted 2.6.30-rc2 #1 MP Server
RIP: 0010:[<ffffffff8021af75>]  [<ffffffff8021af75>]
get_measured_perf+0x4a/0xf9
RSP: 0018:ffff880a7d56de20  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 00000046241a42b6 RCX: ffff88004d219000
RDX: 000000000000b660 RSI: 0000000000000020 RDI: 0000000000000001
RBP: ffff880a7f052000 R08: 00000046241a42b6 R09: ffffffff807639f0
R10: 00000000ffffffea R11: ffffffff802207f4 R12: ffff880a7f052000
R13: ffff88004d20e460 R14: 0000000000ddd5a6 R15: 0000000000000001
FS:  0000000000000000(0000) GS:ffff88004d200000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000000000000020 CR3: 0000000a7f1bf000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process kondemand/0 (pid: 2091, threadinfo ffff880a7d56c000, task
ffff880a7d4d18c0)
Stack:
 ffff880a7f052078 ffffffff803efd54 00000046241a42b6 000000462ffa9e95
 0000000000000001 0000000000000001 00000000ffffffea ffffffff8064f41a
 0000000000000012 0000000000000012 ffff880a7f052000 ffffffff80650547
Call Trace:
 [<ffffffff803efd54>] ? kobject_get+0x12/0x17
 [<ffffffff8064f41a>] ? __cpufreq_driver_getavg+0x42/0x57
 [<ffffffff80650547>] ? do_dbs_timer+0x147/0x272
 [<ffffffff80650400>] ? do_dbs_timer+0x0/0x272
 [<ffffffff802474ca>] ? worker_thread+0x15b/0x1f5
 [<ffffffff8024a02c>] ? autoremove_wake_function+0x0/0x2e
 [<ffffffff8024736f>] ? worker_thread+0x0/0x1f5
 [<ffffffff80249f0d>] ? kthread+0x54/0x83
 [<ffffffff8020c87a>] ? child_rip+0xa/0x20
 [<ffffffff80249eb9>] ? kthread+0x0/0x83
 [<ffffffff8020c870>] ? child_rip+0x0/0x20
Code: 99 a6 03 00 31 c9 85 c0 0f 85 c3 00 00 00 89 df 4c 8b 44 24 10 48
c7 c2 60 b6 00 00 48 8b 0c fd e0 30 a5 80 4c 89 c3 48 8b 04 0a <48> 2b
58 20 48 8b 44 24 18 48 89 1c 24 48 8b 34 0a 48 2b 46 28
RIP  [<ffffffff8021af75>] get_measured_perf+0x4a/0xf9
 RSP <ffff880a7d56de20>
CR2: 0000000000000020

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 9d3af38..ed43734 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -68,11 +68,16 @@ struct acpi_cpufreq_data {
 	unsigned int max_freq;
 	unsigned int resume;
 	unsigned int cpu_feature;
-	u64 saved_aperf, saved_mperf;
 };
 
 static DEFINE_PER_CPU(struct acpi_cpufreq_data *, drv_data);
 
+struct acpi_msr_data {
+	u64 saved_aperf, saved_mperf;
+};
+
+static DEFINE_PER_CPU(struct acpi_msr_data, msr_data);
+
 DEFINE_TRACE(power_mark);
 
 /* acpi_perf_data is a pointer to percpu data. */
@@ -287,11 +292,11 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
 		return 0;
 
 	cur.aperf.whole = readin.aperf.whole -
-				per_cpu(drv_data, cpu)->saved_aperf;
+				per_cpu(msr_data, cpu).saved_aperf;
 	cur.mperf.whole = readin.mperf.whole -
-				per_cpu(drv_data, cpu)->saved_mperf;
-	per_cpu(drv_data, cpu)->saved_aperf = readin.aperf.whole;
-	per_cpu(drv_data, cpu)->saved_mperf = readin.mperf.whole;
+				per_cpu(msr_data, cpu).saved_mperf;
+	per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
+	per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
 
 #ifdef __i386__
 	/*
-- 
1.6.0.2

Re: [PATCH 1/8] Fix for a regression that was introduced by earlier commit

[Len Brown]

already applied

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 2/8] acpi-cpufreq: Cleanup: Use printk_once

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index ed43734..4475523 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -693,13 +693,9 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	/* Check for high latency (>20uS) from buggy BIOSes, like on T42 */
 	if (perf->control_register.space_id == ACPI_ADR_SPACE_FIXED_HARDWARE &&
 	    policy->cpuinfo.transition_latency > 20 * 1000) {
-		static int print_once;
 		policy->cpuinfo.transition_latency = 20 * 1000;
-		if (!print_once) {
-			print_once = 1;
-			printk(KERN_INFO "Capping off P-state tranision latency"
-				" at 20 uS\n");
-		}
+			printk_once(KERN_INFO "Capping off P-state tranision"
+				    " latency at 20 uS\n");
 	}
 
 	data->max_freq = perf->states[0].core_frequency * 1000;
-- 
1.6.0.2

Re: [PATCH 2/8] acpi-cpufreq: Cleanup: Use printk_once

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 3/8] acpi-cpufreq: First multiply then divide to avoid zero results

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 4475523..7948d88 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -340,7 +340,7 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
 
 #endif
 
-	retval = per_cpu(drv_data, policy->cpu)->max_freq * perf_percent / 100;
+	retval = (per_cpu(drv_data, policy->cpu)->max_freq * perf_percent) / 100;
 
 	return retval;
 }
-- 
1.6.0.2

Re: [PATCH 3/8] acpi-cpufreq: First multiply then divide to avoid zero results

[Pallipadi, Venkatesh]

Why is this needed? Normal C precedence rules should take care of this.
No?

Thanks,
Venki 

On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> Cc: <cpufreq@vger.kernel.org>
> ---
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 4475523..7948d88 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -340,7 +340,7 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
>  
>  #endif
>  
> -	retval = per_cpu(drv_data, policy->cpu)->max_freq * perf_percent / 100;
> +	retval = (per_cpu(drv_data, policy->cpu)->max_freq * perf_percent) / 100;
>  
>  	return retval;
>  }

Re: [PATCH 3/8] acpi-cpufreq: First multiply then divide to avoid zero results

[Len Brown]

> Why is this needed? Normal C precedence rules should take care of this.

Correct, * and / are the same precedence
and they associate (are evaluated) left to right.

so this change is a functional no-op.

But my brain parsed this line better w/ this style change,
so i applied it:-)

thanks,
-Len Brown, Intel Open Source Technology Center

> On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > Cc: <linux-acpi@vger.kernel.org>
> > Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> > Cc: <cpufreq@vger.kernel.org>
> > ---
> >  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > index 4475523..7948d88 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > @@ -340,7 +340,7 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
> >  
> >  #endif
> >  
> > -	retval = per_cpu(drv_data, policy->cpu)->max_freq * perf_percent / 100;
> > +	retval = (per_cpu(drv_data, policy->cpu)->max_freq * perf_percent) / 100;
> >  
> >  	return retval;
> >  }
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 4/8] acpi-cpufreq: Do not let get_measured perf depend on internal variable

[Thomas Renninger]

Take already available policy->cpuinfo.max_freq and get rid of acpi-cpufreq
specific max_freq variable.

This implies that P0 is always the highest frequency which should always
be true as ACPI spec says:
As a result, the zeroth entry describes the highest performance state

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 7948d88..7a9a161 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -65,7 +65,6 @@ enum {
 struct acpi_cpufreq_data {
 	struct acpi_processor_performance *acpi_data;
 	struct cpufreq_frequency_table *freq_table;
-	unsigned int max_freq;
 	unsigned int resume;
 	unsigned int cpu_feature;
 };
@@ -340,7 +339,7 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
 
 #endif
 
-	retval = (per_cpu(drv_data, policy->cpu)->max_freq * perf_percent) / 100;
+	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
 
 	return retval;
 }
@@ -698,7 +697,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 				    " latency at 20 uS\n");
 	}
 
-	data->max_freq = perf->states[0].core_frequency * 1000;
 	/* table init */
 	for (i = 0; i < perf->state_count; i++) {
 		if (i > 0 && perf->states[i].core_frequency >=
@@ -717,6 +715,9 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	if (result)
 		goto err_freqfree;
 
+	if (perf->states[0].core_frequency * 1000 != policy->cpuinfo.max_freq)
+		printk(KERN_WARNING FW_WARN "P-state 0 is not max freq\n");
+
 	switch (perf->control_register.space_id) {
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		/* Current speed is unknown and not detectable by IO port */
-- 
1.6.0.2

Re: [PATCH 4/8] acpi-cpufreq: Do not let get_measured perf depend on internal variable

[Pallipadi, Venkatesh]

Ack.

I am not sure below printk is adding much value though, considering
cpufreq_frequency_table_cpuinfo() that is initializing cpuinfo.max_freq
is getting called just one line before this code.

Thanks,
Venki

> +	if (perf->states[0].core_frequency * 1000 !=
policy->cpuinfo.max_freq)
> +		printk(KERN_WARNING FW_WARN "P-state 0 is not max freq\n");
> +


On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> Take already available policy->cpuinfo.max_freq and get rid of acpi-cpufreq
> specific max_freq variable.
> 
> This implies that P0 is always the highest frequency which should always
> be true as ACPI spec says:
> As a result, the zeroth entry describes the highest performance state
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> Cc: <cpufreq@vger.kernel.org>
> ---
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 7948d88..7a9a161 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -65,7 +65,6 @@ enum {
>  struct acpi_cpufreq_data {
>  	struct acpi_processor_performance *acpi_data;
>  	struct cpufreq_frequency_table *freq_table;
> -	unsigned int max_freq;
>  	unsigned int resume;
>  	unsigned int cpu_feature;
>  };
> @@ -340,7 +339,7 @@ static unsigned int get_measured_perf(struct cpufreq_policy *policy,
>  
>  #endif
>  
> -	retval = (per_cpu(drv_data, policy->cpu)->max_freq * perf_percent) / 100;
> +	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
>  
>  	return retval;
>  }
> @@ -698,7 +697,6 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  				    " latency at 20 uS\n");
>  	}
>  
> -	data->max_freq = perf->states[0].core_frequency * 1000;
>  	/* table init */
>  	for (i = 0; i < perf->state_count; i++) {
>  		if (i > 0 && perf->states[i].core_frequency >=
> @@ -717,6 +715,9 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	if (result)
>  		goto err_freqfree;
>  
> +	if (perf->states[0].core_frequency * 1000 != policy->cpuinfo.max_freq)
> +		printk(KERN_WARNING FW_WARN "P-state 0 is not max freq\n");
> +
>  	switch (perf->control_register.space_id) {
>  	case ACPI_ADR_SPACE_SYSTEM_IO:
>  		/* Current speed is unknown and not detectable by IO port */

Re: [PATCH 4/8] acpi-cpufreq: Do not let get_measured perf depend on internal variable

[Thomas Renninger]

On Friday 17 April 2009 07:24:37 pm Pallipadi, Venkatesh wrote:
> Ack.
>
> I am not sure below printk is adding much value though, considering
> cpufreq_frequency_table_cpuinfo() that is initializing cpuinfo.max_freq
> is getting called just one line before this code.
It handles the BIOS bug case that P0 is not max_freq.
It should never happen...
Better would be to check at _PSS call time that all frequencies
are in the right order and throw a FW_BUG then (if that is not done already). 

Len can you apply this with the last hunk thrown out then, please.

Thanks,

    Thomas

[PATCH 5/8] acpi-cpufreq: Move average performance funtions into separate file and KConfig

[Thomas Renninger]

This makes the code available for cpufreq_stats to monitor the average
frequency easily in userspace.
exports:
EXPORT_SYMBOL_GPL(get_average_perf)
unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
			      u64 *saved_aperf, u64 *saved_mperf)

Additional modification:
Use smp_call_function_single instead of work_on_cpu.
The latter was broken anyway and 0 was always returned as the called function
read_measured_perf_ctrs always returns zero which work_on_cpu's return value
was checked for:
if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
		return 0;

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/Kconfig             |   15 +++
 arch/x86/kernel/cpu/cpufreq/Makefile            |    1 +
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c      |   96 +--------------
 arch/x86/kernel/cpu/cpufreq/average_frequency.c |  146 +++++++++++++++++++++++
 include/linux/cpufreq.h                         |   13 ++
 5 files changed, 178 insertions(+), 93 deletions(-)
 create mode 100644 arch/x86/kernel/cpu/cpufreq/average_frequency.c

diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index 52c8398..3ca1602 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -26,6 +26,21 @@ config X86_ACPI_CPUFREQ
 
 	  If in doubt, say N.
 
+config X86_AVERAGE_FREQUENCY
+	bool "Calculate and consider average frequency over a time period"
+	depends on CPU_FREQ_TABLE
+	help
+	  Latest X86 Intel processors can overclock a single core
+	  behind the kernel's back (ida cpuinfo flag) if specific requirements
+	  are met.
+	  With this option, the kernel can evaluate the real frequency a core
+	  was running on over a time period and kernel parts, for example
+	  the cpufreq core and governor or later the scheduler can consider and
+	  optimize for the "boost" frequency on such processors.
+	  Currently the only driver which serves such processors is acpi-cpufreq.
+	  This option should be enabled for this driver at least
+	  on processors which show the "ida" flag in /proc/cpuinfo
+
 config ELAN_CPUFREQ
 	tristate "AMD Elan SC400 and SC410"
 	select CPU_FREQ_TABLE
diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
index 509296d..3eb5a64 100644
--- a/arch/x86/kernel/cpu/cpufreq/Makefile
+++ b/arch/x86/kernel/cpu/cpufreq/Makefile
@@ -2,6 +2,7 @@
 # K8 systems. ACPI is preferred to all other hardware-specific drivers.
 # speedstep-* is preferred over p4-clockmod.
 
+obj-$(CONFIG_X86_AVERAGE_FREQUENCY)     += average_frequency.o
 obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
 obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
 obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 7a9a161..148857a 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -246,102 +246,12 @@ static u32 get_cur_val(const struct cpumask *mask)
 	return cmd.val;
 }
 
-struct perf_pair {
-	union {
-		struct {
-			u32 lo;
-			u32 hi;
-		} split;
-		u64 whole;
-	} aperf, mperf;
-};
-
-
-static long read_measured_perf_ctrs(void *_cur)
-{
-	struct perf_pair *cur = _cur;
-
-	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
-	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
-
-	return 0;
-}
-
-/*
- * Return the measured active (C0) frequency on this CPU since last call
- * to this function.
- * Input: cpu number
- * Return: Average CPU frequency in terms of max frequency (zero on error)
- *
- * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
- * over a period of time, while CPU is in C0 state.
- * IA32_MPERF counts at the rate of max advertised frequency
- * IA32_APERF counts at the rate of actual CPU frequency
- * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
- * no meaning should be associated with absolute values of these MSRs.
- */
 static unsigned int get_measured_perf(struct cpufreq_policy *policy,
 				      unsigned int cpu)
 {
-	struct perf_pair readin, cur;
-	unsigned int perf_percent;
-	unsigned int retval;
-
-	if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
-		return 0;
-
-	cur.aperf.whole = readin.aperf.whole -
-				per_cpu(msr_data, cpu).saved_aperf;
-	cur.mperf.whole = readin.mperf.whole -
-				per_cpu(msr_data, cpu).saved_mperf;
-	per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
-	per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
-
-#ifdef __i386__
-	/*
-	 * We dont want to do 64 bit divide with 32 bit kernel
-	 * Get an approximate value. Return failure in case we cannot get
-	 * an approximate value.
-	 */
-	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
-		int shift_count;
-		u32 h;
-
-		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
-		shift_count = fls(h);
-
-		cur.aperf.whole >>= shift_count;
-		cur.mperf.whole >>= shift_count;
-	}
-
-	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
-		int shift_count = 7;
-		cur.aperf.split.lo >>= shift_count;
-		cur.mperf.split.lo >>= shift_count;
-	}
-
-	if (cur.aperf.split.lo && cur.mperf.split.lo)
-		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
-	else
-		perf_percent = 0;
-
-#else
-	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
-		int shift_count = 7;
-		cur.aperf.whole >>= shift_count;
-		cur.mperf.whole >>= shift_count;
-	}
-
-	if (cur.aperf.whole && cur.mperf.whole)
-		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
-	else
-		perf_percent = 0;
-
-#endif
-
-	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
-
-	return retval;
+	return get_average_perf(policy, cpu,
+				&per_cpu(msr_data, cpu).saved_aperf,
+				&per_cpu(msr_data, cpu).saved_mperf);
 }
 
 static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
diff --git a/arch/x86/kernel/cpu/cpufreq/average_frequency.c b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
new file mode 100644
index 0000000..2b18a75
--- /dev/null
+++ b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
@@ -0,0 +1,146 @@
+/*
+ *  average_frequency.c
+ *
+ *  Copyright (C) 2009       Thomas Renninger <trenn@suse.de> (Novell)
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or (at
+ *  your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * Code taken from acpi-cpufreq which initially came from
+ * Mike Travis <travis@sgi.com> and
+ * Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/cpufreq.h>
+#include <linux/workqueue.h>
+
+#include <asm/msr.h>
+
+struct perf_pair {
+	union {
+		struct {
+			u32 lo;
+			u32 hi;
+		} split;
+		s64 whole;
+	} aperf, mperf;
+};
+
+static void read_measured_perf_ctrs(void *_cur)
+{
+	struct perf_pair *cur = _cur;
+
+	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
+	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
+}
+
+/*
+ * Return the measured active (C0) frequency on this CPU since last call
+ * to this function.
+ * Input: cpu number
+ *        cpufreq policy -> must at least have cpuinfo.max_freq be set
+ *        saved_mperf    -> register value of last call, will get updated
+ *        saved_aperf    -> register value of last call, will get updated
+ *
+ * Return: Average CPU frequency in terms of max frequency (zero on error)
+ *         since the function has been called the last time with saved
+ *         aperf/mperf values.
+ *
+ * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
+ * over a period of time, while CPU is in C0 state.
+ * IA32_MPERF counts at the rate of max advertised frequency
+ * IA32_APERF counts at the rate of actual CPU frequency
+ * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
+ * no meaning should be associated with absolute values of these MSRs.
+ *
+ * Callers must make sure that the X86_FEATURE_IDA bit is set.
+ */
+unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
+			      u64 *saved_aperf, u64 *saved_mperf)
+{
+	struct perf_pair readin, cur;
+	unsigned int perf_percent;
+	unsigned int retval;
+
+	smp_call_function_single(cpu, read_measured_perf_ctrs, &readin, 1);
+
+	/* Called the first time */
+	if ((*saved_aperf == 0) && (*saved_mperf == 0)) {
+		*saved_aperf = readin.aperf.whole;
+		*saved_mperf = readin.mperf.whole;
+		return 0;
+	}
+
+	cur.aperf.whole = readin.aperf.whole - *saved_aperf;
+	cur.mperf.whole = readin.mperf.whole - *saved_mperf;
+
+	/* Handle overflow gracefully */
+	if (unlikely(*saved_aperf > readin.aperf.whole))
+		cur.aperf.whole = 0ULL - readin.aperf.whole;
+	if (unlikely(*saved_mperf > readin.mperf.whole))
+		cur.mperf.whole = 0ULL - readin.mperf.whole;
+
+	*saved_aperf = readin.aperf.whole;
+	*saved_mperf = readin.mperf.whole;
+
+#ifdef __i386__
+	/*
+	 * We dont want to do 64 bit divide with 32 bit kernel
+	 * Get an approximate value. Return failure in case we cannot get
+	 * an approximate value.
+	 */
+	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
+		int shift_count;
+		u32 h;
+
+		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
+		shift_count = fls(h);
+
+		cur.aperf.whole >>= shift_count;
+		cur.mperf.whole >>= shift_count;
+	}
+
+	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
+		int shift_count = 7;
+		cur.aperf.split.lo >>= shift_count;
+		cur.mperf.split.lo >>= shift_count;
+	}
+
+	if (cur.aperf.split.lo && cur.mperf.split.lo)
+		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
+	else
+		perf_percent = 0;
+
+#else
+	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
+		int shift_count = 7;
+		cur.aperf.whole >>= shift_count;
+		cur.mperf.whole >>= shift_count;
+	}
+
+	if (cur.aperf.whole && cur.mperf.whole)
+		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
+	else
+		perf_percent = 0;
+
+#endif
+	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
+	return retval;
+}
+EXPORT_SYMBOL_GPL(get_average_perf);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 1610427..0ab8bf7 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -362,6 +362,19 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
 
 void cpufreq_frequency_table_put_attr(unsigned int cpu);
 
+/*
+ * Get the average frequency since the last call of this function if the
+ * needed MSRs are supported by the CPU
+*/
+#ifdef CONFIG_X86_AVERAGE_FREQUENCY
+unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
+			      u64 *saved_aperf, u64 *saved_mperf);
+#else
+unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
+			      u64 *saved_aperf, u64 *saved_mperf)
+{ return 0; }
+#endif
+
 
 /*********************************************************************
  *                     UNIFIED DEBUG HELPERS                         *
-- 
1.6.0.2

Re: [PATCH 5/8] acpi-cpufreq: Move average performance funtions into separate file and KConfig

[Pallipadi, Venkatesh]

There has been changes in this code recently changing work_on_cpu to
smp_call_function_single. So, you will have to rebase this patch.

Also, I am not full certain whether cpufreq_stats is the right place to
do this. Why not have a userspace library (in cpufreq utils) that reads
the MSR and does the same calculation. Main issue I have with kernel API
is that I will never know what you are getting from this interface. I
mean there may be 10 other users other than me, calling this API and
from last call does not mean much in that situation. Userlevel lib that
can keep track of pid or some handle and track multiple callers seems a
better match for this.

Thanks,
Venki

On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> This makes the code available for cpufreq_stats to monitor the average
> frequency easily in userspace.
> exports:
> EXPORT_SYMBOL_GPL(get_average_perf)
> unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
> 			      u64 *saved_aperf, u64 *saved_mperf)
> 
> Additional modification:
> Use smp_call_function_single instead of work_on_cpu.
> The latter was broken anyway and 0 was always returned as the called function
> read_measured_perf_ctrs always returns zero which work_on_cpu's return value
> was checked for:
> if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
> 		return 0;
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> Cc: <cpufreq@vger.kernel.org>
> ---
>  arch/x86/kernel/cpu/cpufreq/Kconfig             |   15 +++
>  arch/x86/kernel/cpu/cpufreq/Makefile            |    1 +
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c      |   96 +--------------
>  arch/x86/kernel/cpu/cpufreq/average_frequency.c |  146 +++++++++++++++++++++++
>  include/linux/cpufreq.h                         |   13 ++
>  5 files changed, 178 insertions(+), 93 deletions(-)
>  create mode 100644 arch/x86/kernel/cpu/cpufreq/average_frequency.c
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
> index 52c8398..3ca1602 100644
> --- a/arch/x86/kernel/cpu/cpufreq/Kconfig
> +++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
> @@ -26,6 +26,21 @@ config X86_ACPI_CPUFREQ
>  
>  	  If in doubt, say N.
>  
> +config X86_AVERAGE_FREQUENCY
> +	bool "Calculate and consider average frequency over a time period"
> +	depends on CPU_FREQ_TABLE
> +	help
> +	  Latest X86 Intel processors can overclock a single core
> +	  behind the kernel's back (ida cpuinfo flag) if specific requirements
> +	  are met.
> +	  With this option, the kernel can evaluate the real frequency a core
> +	  was running on over a time period and kernel parts, for example
> +	  the cpufreq core and governor or later the scheduler can consider and
> +	  optimize for the "boost" frequency on such processors.
> +	  Currently the only driver which serves such processors is acpi-cpufreq.
> +	  This option should be enabled for this driver at least
> +	  on processors which show the "ida" flag in /proc/cpuinfo
> +
>  config ELAN_CPUFREQ
>  	tristate "AMD Elan SC400 and SC410"
>  	select CPU_FREQ_TABLE
> diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile b/arch/x86/kernel/cpu/cpufreq/Makefile
> index 509296d..3eb5a64 100644
> --- a/arch/x86/kernel/cpu/cpufreq/Makefile
> +++ b/arch/x86/kernel/cpu/cpufreq/Makefile
> @@ -2,6 +2,7 @@
>  # K8 systems. ACPI is preferred to all other hardware-specific drivers.
>  # speedstep-* is preferred over p4-clockmod.
>  
> +obj-$(CONFIG_X86_AVERAGE_FREQUENCY)     += average_frequency.o
>  obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
>  obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
>  obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 7a9a161..148857a 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -246,102 +246,12 @@ static u32 get_cur_val(const struct cpumask *mask)
>  	return cmd.val;
>  }
>  
> -struct perf_pair {
> -	union {
> -		struct {
> -			u32 lo;
> -			u32 hi;
> -		} split;
> -		u64 whole;
> -	} aperf, mperf;
> -};
> -
> -
> -static long read_measured_perf_ctrs(void *_cur)
> -{
> -	struct perf_pair *cur = _cur;
> -
> -	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
> -	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
> -
> -	return 0;
> -}
> -
> -/*
> - * Return the measured active (C0) frequency on this CPU since last call
> - * to this function.
> - * Input: cpu number
> - * Return: Average CPU frequency in terms of max frequency (zero on error)
> - *
> - * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
> - * over a period of time, while CPU is in C0 state.
> - * IA32_MPERF counts at the rate of max advertised frequency
> - * IA32_APERF counts at the rate of actual CPU frequency
> - * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
> - * no meaning should be associated with absolute values of these MSRs.
> - */
>  static unsigned int get_measured_perf(struct cpufreq_policy *policy,
>  				      unsigned int cpu)
>  {
> -	struct perf_pair readin, cur;
> -	unsigned int perf_percent;
> -	unsigned int retval;
> -
> -	if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
> -		return 0;
> -
> -	cur.aperf.whole = readin.aperf.whole -
> -				per_cpu(msr_data, cpu).saved_aperf;
> -	cur.mperf.whole = readin.mperf.whole -
> -				per_cpu(msr_data, cpu).saved_mperf;
> -	per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
> -	per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
> -
> -#ifdef __i386__
> -	/*
> -	 * We dont want to do 64 bit divide with 32 bit kernel
> -	 * Get an approximate value. Return failure in case we cannot get
> -	 * an approximate value.
> -	 */
> -	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
> -		int shift_count;
> -		u32 h;
> -
> -		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
> -		shift_count = fls(h);
> -
> -		cur.aperf.whole >>= shift_count;
> -		cur.mperf.whole >>= shift_count;
> -	}
> -
> -	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> -		int shift_count = 7;
> -		cur.aperf.split.lo >>= shift_count;
> -		cur.mperf.split.lo >>= shift_count;
> -	}
> -
> -	if (cur.aperf.split.lo && cur.mperf.split.lo)
> -		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
> -	else
> -		perf_percent = 0;
> -
> -#else
> -	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
> -		int shift_count = 7;
> -		cur.aperf.whole >>= shift_count;
> -		cur.mperf.whole >>= shift_count;
> -	}
> -
> -	if (cur.aperf.whole && cur.mperf.whole)
> -		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
> -	else
> -		perf_percent = 0;
> -
> -#endif
> -
> -	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
> -
> -	return retval;
> +	return get_average_perf(policy, cpu,
> +				&per_cpu(msr_data, cpu).saved_aperf,
> +				&per_cpu(msr_data, cpu).saved_mperf);
>  }
>  
>  static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
> diff --git a/arch/x86/kernel/cpu/cpufreq/average_frequency.c b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
> new file mode 100644
> index 0000000..2b18a75
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
> @@ -0,0 +1,146 @@
> +/*
> + *  average_frequency.c
> + *
> + *  Copyright (C) 2009       Thomas Renninger <trenn@suse.de> (Novell)
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or (at
> + *  your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * Code taken from acpi-cpufreq which initially came from
> + * Mike Travis <travis@sgi.com> and
> + * Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/cpufreq.h>
> +#include <linux/workqueue.h>
> +
> +#include <asm/msr.h>
> +
> +struct perf_pair {
> +	union {
> +		struct {
> +			u32 lo;
> +			u32 hi;
> +		} split;
> +		s64 whole;
> +	} aperf, mperf;
> +};
> +
> +static void read_measured_perf_ctrs(void *_cur)
> +{
> +	struct perf_pair *cur = _cur;
> +
> +	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
> +	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
> +}
> +
> +/*
> + * Return the measured active (C0) frequency on this CPU since last call
> + * to this function.
> + * Input: cpu number
> + *        cpufreq policy -> must at least have cpuinfo.max_freq be set
> + *        saved_mperf    -> register value of last call, will get updated
> + *        saved_aperf    -> register value of last call, will get updated
> + *
> + * Return: Average CPU frequency in terms of max frequency (zero on error)
> + *         since the function has been called the last time with saved
> + *         aperf/mperf values.
> + *
> + * We use IA32_MPERF and IA32_APERF MSRs to get the measured performance
> + * over a period of time, while CPU is in C0 state.
> + * IA32_MPERF counts at the rate of max advertised frequency
> + * IA32_APERF counts at the rate of actual CPU frequency
> + * Only IA32_APERF/IA32_MPERF ratio is architecturally defined and
> + * no meaning should be associated with absolute values of these MSRs.
> + *
> + * Callers must make sure that the X86_FEATURE_IDA bit is set.
> + */
> +unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
> +			      u64 *saved_aperf, u64 *saved_mperf)
> +{
> +	struct perf_pair readin, cur;
> +	unsigned int perf_percent;
> +	unsigned int retval;
> +
> +	smp_call_function_single(cpu, read_measured_perf_ctrs, &readin, 1);
> +
> +	/* Called the first time */
> +	if ((*saved_aperf == 0) && (*saved_mperf == 0)) {
> +		*saved_aperf = readin.aperf.whole;
> +		*saved_mperf = readin.mperf.whole;
> +		return 0;
> +	}
> +
> +	cur.aperf.whole = readin.aperf.whole - *saved_aperf;
> +	cur.mperf.whole = readin.mperf.whole - *saved_mperf;
> +
> +	/* Handle overflow gracefully */
> +	if (unlikely(*saved_aperf > readin.aperf.whole))
> +		cur.aperf.whole = 0ULL - readin.aperf.whole;
> +	if (unlikely(*saved_mperf > readin.mperf.whole))
> +		cur.mperf.whole = 0ULL - readin.mperf.whole;
> +
> +	*saved_aperf = readin.aperf.whole;
> +	*saved_mperf = readin.mperf.whole;
> +
> +#ifdef __i386__
> +	/*
> +	 * We dont want to do 64 bit divide with 32 bit kernel
> +	 * Get an approximate value. Return failure in case we cannot get
> +	 * an approximate value.
> +	 */
> +	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
> +		int shift_count;
> +		u32 h;
> +
> +		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
> +		shift_count = fls(h);
> +
> +		cur.aperf.whole >>= shift_count;
> +		cur.mperf.whole >>= shift_count;
> +	}
> +
> +	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> +		int shift_count = 7;
> +		cur.aperf.split.lo >>= shift_count;
> +		cur.mperf.split.lo >>= shift_count;
> +	}
> +
> +	if (cur.aperf.split.lo && cur.mperf.split.lo)
> +		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
> +	else
> +		perf_percent = 0;
> +
> +#else
> +	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
> +		int shift_count = 7;
> +		cur.aperf.whole >>= shift_count;
> +		cur.mperf.whole >>= shift_count;
> +	}
> +
> +	if (cur.aperf.whole && cur.mperf.whole)
> +		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
> +	else
> +		perf_percent = 0;
> +
> +#endif
> +	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
> +	return retval;
> +}
> +EXPORT_SYMBOL_GPL(get_average_perf);
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 1610427..0ab8bf7 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -362,6 +362,19 @@ void cpufreq_frequency_table_get_attr(struct cpufreq_frequency_table *table,
>  
>  void cpufreq_frequency_table_put_attr(unsigned int cpu);
>  
> +/*
> + * Get the average frequency since the last call of this function if the
> + * needed MSRs are supported by the CPU
> +*/
> +#ifdef CONFIG_X86_AVERAGE_FREQUENCY
> +unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
> +			      u64 *saved_aperf, u64 *saved_mperf);
> +#else
> +unsigned int get_average_perf(struct cpufreq_policy *policy, unsigned int cpu,
> +			      u64 *saved_aperf, u64 *saved_mperf)
> +{ return 0; }
> +#endif
> +
>  
>  /*********************************************************************
>   *                     UNIFIED DEBUG HELPERS                         *

Re: [PATCH 5/8] acpi-cpufreq: Move average performance funtions into separate file and KConfig

[Thomas Renninger]

On Friday 17 April 2009 19:32:09 Pallipadi, Venkatesh wrote:
> 
> There has been changes in this code recently changing work_on_cpu to
> smp_call_function_single. So, you will have to rebase this patch.
It can be adjusted easily with an on-top patch. This discussion should 
not hold off this patch series.
 
> Also, I am not full certain whether cpufreq_stats is the right place
> to do this. Why not have a userspace library (in cpufreq utils) that
> reads the MSR and does the same calculation.
I started with a userspace tool for that. In fact it's finish, I just 
wanted to add some parameters (-r reset MPERF/APERF registers, -i 
interval to (re-)read the MSRs, ...).
Then cpufreq_stats.c came into my mind, which is IMO much more useful.

> Main issue I have with kernel API
> is that I will never know what you are getting from this interface.
> I mean there may be 10 other users other than me, calling this API
> and from last call does not mean much in that situation.

Every caller has to remember the last read APERF/MPERF values.
Therefore every caller knows it gets the average performance since the 
last time *he* called the function.
Look at the cpufreq_stats changes, there you have:
static DEFINE_PER_CPU(u64, saved_aperf);
static DEFINE_PER_CPU(u64, saved_mperf);
which remembers the values of the registers for *cpufreq_stats*.
The same can be done for e.g. scheduler debug code in the kernel 
itself.
I could imagine sched_mc_power developers will find this call 
convenient as it should be the only way to find out about the real 
frequency a core is running on. And they probably want to get this 
info *in the kernel* at least for debugging/monitoring. There are 
probably other use-cases.

This patch series is for splitting out average frequency into an own
function and export it. I consider, getting the current running 
frequency of a processor, as an elementary function for upcoming 
machines where the cpufreq subsystem's values can be wrong (due to 
boost mode or HW restrictions).
It may be that other kernel parts are really needing this or it could 
be that it's only used for monitoring/debugging, but I am pretty sure 
this interface will be useful. Hmm, I should have added lkml or x86 
list so that others could comment whether getting the average freq 
will be useful for developing, I thought this is obvious.
Do you agree that the split and re-use of this code should happen?
Should I repost with lkml added for broader discussion?

> Userlevel lib that can keep track of pid or some handle and track
> multiple callers seems a better match for this.
You can track multiple callers.
I don't see the need of duplicating this in userspace, what exactly
do you expect from a userspace implementation?
Having this in cpufreq_stats should satisfy userspace monitoring 
needs. It avoids duplicated code and tests the in-kernel averg_perf 
implementation. It does only add 2 u64 per_cpu variables memory usage 
in a driver which is thought for debugging/monitoring anyway.

Will this (these) be accepted?

Thanks,

    Thomas

PS: A general thing about such close to HW userspace tools:
Could there be some cpuid library call in x86info propagated
which makes checking for CPU features much more easy in C-/C++ code
than parsing /proc/cpuinfo and /dev/cpu/X/{cpuid,msr}?
I started with read_msr() and write_msr() library calls. Every little 
tool implementing the access to /dev/cpu/*/msr and parsing 
/proc/cpuinfo for CPU capabilities looks wrong.

Then the mysterious CPU flags could be replaced (longterm) at some 
time with a userspace tool, explaining what exactly they are for.
Hmm, I better open an own thread for that, maybe Dave is even
reading this...

    Thomas

> Thanks,
> Venki
> 
> On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> > This makes the code available for cpufreq_stats to monitor the 
average
> > frequency easily in userspace.
> > exports:
> > EXPORT_SYMBOL_GPL(get_average_perf)
> > unsigned int get_average_perf(struct cpufreq_policy *policy, 
unsigned int cpu,
> > 			      u64 *saved_aperf, u64 *saved_mperf)
> > 
> > Additional modification:
> > Use smp_call_function_single instead of work_on_cpu.
> > The latter was broken anyway and 0 was always returned as the 
called function
> > read_measured_perf_ctrs always returns zero which work_on_cpu's 
return value
> > was checked for:
> > if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
> > 		return 0;
> > 
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > Cc: <linux-acpi@vger.kernel.org>
> > Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> > Cc: <cpufreq@vger.kernel.org>
> > ---
> >  arch/x86/kernel/cpu/cpufreq/Kconfig             |   15 +++
> >  arch/x86/kernel/cpu/cpufreq/Makefile            |    1 +
> >  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c      |   96 
+--------------
> >  arch/x86/kernel/cpu/cpufreq/average_frequency.c |  146 
+++++++++++++++++++++++
> >  include/linux/cpufreq.h                         |   13 ++
> >  5 files changed, 178 insertions(+), 93 deletions(-)
> >  create mode 100644 
arch/x86/kernel/cpu/cpufreq/average_frequency.c
> > 
> > diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig 
b/arch/x86/kernel/cpu/cpufreq/Kconfig
> > index 52c8398..3ca1602 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/Kconfig
> > +++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
> > @@ -26,6 +26,21 @@ config X86_ACPI_CPUFREQ
> >  
> >  	  If in doubt, say N.
> >  
> > +config X86_AVERAGE_FREQUENCY
> > +	bool "Calculate and consider average frequency over a time 
period"
> > +	depends on CPU_FREQ_TABLE
> > +	help
> > +	  Latest X86 Intel processors can overclock a single core
> > +	  behind the kernel's back (ida cpuinfo flag) if specific 
requirements
> > +	  are met.
> > +	  With this option, the kernel can evaluate the real frequency a 
core
> > +	  was running on over a time period and kernel parts, for 
example
> > +	  the cpufreq core and governor or later the scheduler can 
consider and
> > +	  optimize for the "boost" frequency on such processors.
> > +	  Currently the only driver which serves such processors is 
acpi-cpufreq.
> > +	  This option should be enabled for this driver at least
> > +	  on processors which show the "ida" flag in /proc/cpuinfo
> > +
> >  config ELAN_CPUFREQ
> >  	tristate "AMD Elan SC400 and SC410"
> >  	select CPU_FREQ_TABLE
> > diff --git a/arch/x86/kernel/cpu/cpufreq/Makefile 
b/arch/x86/kernel/cpu/cpufreq/Makefile
> > index 509296d..3eb5a64 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/Makefile
> > +++ b/arch/x86/kernel/cpu/cpufreq/Makefile
> > @@ -2,6 +2,7 @@
> >  # K8 systems. ACPI is preferred to all other hardware-specific 
drivers.
> >  # speedstep-* is preferred over p4-clockmod.
> >  
> > +obj-$(CONFIG_X86_AVERAGE_FREQUENCY)     += average_frequency.o
> >  obj-$(CONFIG_X86_POWERNOW_K8)		+= powernow-k8.o
> >  obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
> >  obj-$(CONFIG_X86_POWERNOW_K6)		+= powernow-k6.o
> > diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c 
b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > index 7a9a161..148857a 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> > @@ -246,102 +246,12 @@ static u32 get_cur_val(const struct cpumask 
*mask)
> >  	return cmd.val;
> >  }
> >  
> > -struct perf_pair {
> > -	union {
> > -		struct {
> > -			u32 lo;
> > -			u32 hi;
> > -		} split;
> > -		u64 whole;
> > -	} aperf, mperf;
> > -};
> > -
> > -
> > -static long read_measured_perf_ctrs(void *_cur)
> > -{
> > -	struct perf_pair *cur = _cur;
> > -
> > -	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
> > -	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
> > -
> > -	return 0;
> > -}
> > -
> > -/*
> > - * Return the measured active (C0) frequency on this CPU since 
last call
> > - * to this function.
> > - * Input: cpu number
> > - * Return: Average CPU frequency in terms of max frequency (zero 
on error)
> > - *
> > - * We use IA32_MPERF and IA32_APERF MSRs to get the measured 
performance
> > - * over a period of time, while CPU is in C0 state.
> > - * IA32_MPERF counts at the rate of max advertised frequency
> > - * IA32_APERF counts at the rate of actual CPU frequency
> > - * Only IA32_APERF/IA32_MPERF ratio is architecturally defined 
and
> > - * no meaning should be associated with absolute values of these 
MSRs.
> > - */
> >  static unsigned int get_measured_perf(struct cpufreq_policy 
*policy,
> >  				      unsigned int cpu)
> >  {
> > -	struct perf_pair readin, cur;
> > -	unsigned int perf_percent;
> > -	unsigned int retval;
> > -
> > -	if (!work_on_cpu(cpu, read_measured_perf_ctrs, &readin))
> > -		return 0;
> > -
> > -	cur.aperf.whole = readin.aperf.whole -
> > -				per_cpu(msr_data, cpu).saved_aperf;
> > -	cur.mperf.whole = readin.mperf.whole -
> > -				per_cpu(msr_data, cpu).saved_mperf;
> > -	per_cpu(msr_data, cpu).saved_aperf = readin.aperf.whole;
> > -	per_cpu(msr_data, cpu).saved_mperf = readin.mperf.whole;
> > -
> > -#ifdef __i386__
> > -	/*
> > -	 * We dont want to do 64 bit divide with 32 bit kernel
> > -	 * Get an approximate value. Return failure in case we cannot 
get
> > -	 * an approximate value.
> > -	 */
> > -	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
> > -		int shift_count;
> > -		u32 h;
> > -
> > -		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
> > -		shift_count = fls(h);
> > -
> > -		cur.aperf.whole >>= shift_count;
> > -		cur.mperf.whole >>= shift_count;
> > -	}
> > -
> > -	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > -		int shift_count = 7;
> > -		cur.aperf.split.lo >>= shift_count;
> > -		cur.mperf.split.lo >>= shift_count;
> > -	}
> > -
> > -	if (cur.aperf.split.lo && cur.mperf.split.lo)
> > -		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
> > -	else
> > -		perf_percent = 0;
> > -
> > -#else
> > -	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
> > -		int shift_count = 7;
> > -		cur.aperf.whole >>= shift_count;
> > -		cur.mperf.whole >>= shift_count;
> > -	}
> > -
> > -	if (cur.aperf.whole && cur.mperf.whole)
> > -		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
> > -	else
> > -		perf_percent = 0;
> > -
> > -#endif
> > -
> > -	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
> > -
> > -	return retval;
> > +	return get_average_perf(policy, cpu,
> > +				&per_cpu(msr_data, cpu).saved_aperf,
> > +				&per_cpu(msr_data, cpu).saved_mperf);
> >  }
> >  
> >  static unsigned int get_cur_freq_on_cpu(unsigned int cpu)
> > diff --git a/arch/x86/kernel/cpu/cpufreq/average_frequency.c 
b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
> > new file mode 100644
> > index 0000000..2b18a75
> > --- /dev/null
> > +++ b/arch/x86/kernel/cpu/cpufreq/average_frequency.c
> > @@ -0,0 +1,146 @@
> > +/*
> > + *  average_frequency.c
> > + *
> > + *  Copyright (C) 2009       Thomas Renninger <trenn@suse.de> 
(Novell)
> > + *
> > + * 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + *  This program is free software; you can redistribute it and/or 
modify
> > + *  it under the terms of the GNU General Public License as 
published by
> > + *  the Free Software Foundation; either version 2 of the 
License, or (at
> > + *  your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be 
useful, but
> > + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the 
GNU
> > + *  General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public 
License along
> > + *  with this program; if not, write to the Free Software 
Foundation, Inc.,
> > + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> > + *
> > + * 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + * Code taken from acpi-cpufreq which initially came from
> > + * Mike Travis <travis@sgi.com> and
> > + * Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/cpufreq.h>
> > +#include <linux/workqueue.h>
> > +
> > +#include <asm/msr.h>
> > +
> > +struct perf_pair {
> > +	union {
> > +		struct {
> > +			u32 lo;
> > +			u32 hi;
> > +		} split;
> > +		s64 whole;
> > +	} aperf, mperf;
> > +};
> > +
> > +static void read_measured_perf_ctrs(void *_cur)
> > +{
> > +	struct perf_pair *cur = _cur;
> > +
> > +	rdmsr(MSR_IA32_APERF, cur->aperf.split.lo, cur->aperf.split.hi);
> > +	rdmsr(MSR_IA32_MPERF, cur->mperf.split.lo, cur->mperf.split.hi);
> > +}
> > +
> > +/*
> > + * Return the measured active (C0) frequency on this CPU since 
last call
> > + * to this function.
> > + * Input: cpu number
> > + *        cpufreq policy -> must at least have cpuinfo.max_freq 
be set
> > + *        saved_mperf    -> register value of last call, will get 
updated
> > + *        saved_aperf    -> register value of last call, will get 
updated
> > + *
> > + * Return: Average CPU frequency in terms of max frequency (zero 
on error)
> > + *         since the function has been called the last time with 
saved
> > + *         aperf/mperf values.
> > + *
> > + * We use IA32_MPERF and IA32_APERF MSRs to get the measured 
performance
> > + * over a period of time, while CPU is in C0 state.
> > + * IA32_MPERF counts at the rate of max advertised frequency
> > + * IA32_APERF counts at the rate of actual CPU frequency
> > + * Only IA32_APERF/IA32_MPERF ratio is architecturally defined 
and
> > + * no meaning should be associated with absolute values of these 
MSRs.
> > + *
> > + * Callers must make sure that the X86_FEATURE_IDA bit is set.
> > + */
> > +unsigned int get_average_perf(struct cpufreq_policy *policy, 
unsigned int cpu,
> > +			      u64 *saved_aperf, u64 *saved_mperf)
> > +{
> > +	struct perf_pair readin, cur;
> > +	unsigned int perf_percent;
> > +	unsigned int retval;
> > +
> > +	smp_call_function_single(cpu, read_measured_perf_ctrs, &readin, 
1);
> > +
> > +	/* Called the first time */
> > +	if ((*saved_aperf == 0) && (*saved_mperf == 0)) {
> > +		*saved_aperf = readin.aperf.whole;
> > +		*saved_mperf = readin.mperf.whole;
> > +		return 0;
> > +	}
> > +
> > +	cur.aperf.whole = readin.aperf.whole - *saved_aperf;
> > +	cur.mperf.whole = readin.mperf.whole - *saved_mperf;
> > +
> > +	/* Handle overflow gracefully */
> > +	if (unlikely(*saved_aperf > readin.aperf.whole))
> > +		cur.aperf.whole = 0ULL - readin.aperf.whole;
> > +	if (unlikely(*saved_mperf > readin.mperf.whole))
> > +		cur.mperf.whole = 0ULL - readin.mperf.whole;
> > +
> > +	*saved_aperf = readin.aperf.whole;
> > +	*saved_mperf = readin.mperf.whole;
> > +
> > +#ifdef __i386__
> > +	/*
> > +	 * We dont want to do 64 bit divide with 32 bit kernel
> > +	 * Get an approximate value. Return failure in case we cannot 
get
> > +	 * an approximate value.
> > +	 */
> > +	if (unlikely(cur.aperf.split.hi || cur.mperf.split.hi)) {
> > +		int shift_count;
> > +		u32 h;
> > +
> > +		h = max_t(u32, cur.aperf.split.hi, cur.mperf.split.hi);
> > +		shift_count = fls(h);
> > +
> > +		cur.aperf.whole >>= shift_count;
> > +		cur.mperf.whole >>= shift_count;
> > +	}
> > +
> > +	if (((unsigned long)(-1) / 100) < cur.aperf.split.lo) {
> > +		int shift_count = 7;
> > +		cur.aperf.split.lo >>= shift_count;
> > +		cur.mperf.split.lo >>= shift_count;
> > +	}
> > +
> > +	if (cur.aperf.split.lo && cur.mperf.split.lo)
> > +		perf_percent = (cur.aperf.split.lo * 100) / cur.mperf.split.lo;
> > +	else
> > +		perf_percent = 0;
> > +
> > +#else
> > +	if (unlikely(((unsigned long)(-1) / 100) < cur.aperf.whole)) {
> > +		int shift_count = 7;
> > +		cur.aperf.whole >>= shift_count;
> > +		cur.mperf.whole >>= shift_count;
> > +	}
> > +
> > +	if (cur.aperf.whole && cur.mperf.whole)
> > +		perf_percent = (cur.aperf.whole * 100) / cur.mperf.whole;
> > +	else
> > +		perf_percent = 0;
> > +
> > +#endif
> > +	retval = (policy->cpuinfo.max_freq * perf_percent) / 100;
> > +	return retval;
> > +}
> > +EXPORT_SYMBOL_GPL(get_average_perf);
> > diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> > index 1610427..0ab8bf7 100644
> > --- a/include/linux/cpufreq.h
> > +++ b/include/linux/cpufreq.h
> > @@ -362,6 +362,19 @@ void cpufreq_frequency_table_get_attr(struct 
cpufreq_frequency_table *table,
> >  
> >  void cpufreq_frequency_table_put_attr(unsigned int cpu);
> >  
> > +/*
> > + * Get the average frequency since the last call of this function 
if the
> > + * needed MSRs are supported by the CPU
> > +*/
> > +#ifdef CONFIG_X86_AVERAGE_FREQUENCY
> > +unsigned int get_average_perf(struct cpufreq_policy *policy, 
unsigned int cpu,
> > +			      u64 *saved_aperf, u64 *saved_mperf);
> > +#else
> > +unsigned int get_average_perf(struct cpufreq_policy *policy, 
unsigned int cpu,
> > +			      u64 *saved_aperf, u64 *saved_mperf)
> > +{ return 0; }
> > +#endif
> > +
> >  
> >  
/*********************************************************************
> >   *                     UNIFIED DEBUG HELPERS                         
*
> 
> 

Re: [PATCH 5/8] acpi-cpufreq: Move average performance funtions into separate file and KConfig

[Pallipadi, Venkatesh]

On Mon, 2009-04-20 at 04:41 -0700, Thomas Renninger wrote:
> On Friday 17 April 2009 19:32:09 Pallipadi, Venkatesh wrote:
> >
> > There has been changes in this code recently changing work_on_cpu to
> > smp_call_function_single. So, you will have to rebase this patch.
> It can be adjusted easily with an on-top patch. This discussion should
> not hold off this patch series.
> 

Agree about not holding of this patch series. I just mentioned as I saw
conflicting changes, that this patch has to be rebased.

> > Also, I am not full certain whether cpufreq_stats is the right place
> > to do this. Why not have a userspace library (in cpufreq utils) that
> > reads the MSR and does the same calculation.
> I started with a userspace tool for that. In fact it's finish, I just
> wanted to add some parameters (-r reset MPERF/APERF registers, -i
> interval to (re-)read the MSRs, ...).
> Then cpufreq_stats.c came into my mind, which is IMO much more useful.
> 

I have thought about this in cpufreq_stats vs in userspace long and
hard. My feeling is this stuff belongs more in userspace than
cpufreq_stats. This has no direct relation to cpufreq. There is no
clear-cut way to export the information in cpufreq_stats. Average freq
since last read is not a good interface that multiple users can use.

> > Main issue I have with kernel API
> > is that I will never know what you are getting from this interface.
> > I mean there may be 10 other users other than me, calling this API
> > and from last call does not mean much in that situation.
> 
> Every caller has to remember the last read APERF/MPERF values.
> Therefore every caller knows it gets the average performance since the
> last time *he* called the function.
> Look at the cpufreq_stats changes, there you have:
> static DEFINE_PER_CPU(u64, saved_aperf);
> static DEFINE_PER_CPU(u64, saved_mperf);
> which remembers the values of the registers for *cpufreq_stats*.

One userspace tool (say for example top) reads this cpufreq_stats
average_freq and sleeps for 10 seconds.
After 9 seconds, another userspace tool (say for example vmstate) reads
this cpufreq_stats and sleeps for 10 seconds.
After 1 sec, top comes back to read the interface and gets average_freq
for 1 sec. But, it was expecting avg_freq for 10 secs and thats what it
will display as it have no way to know it got average freq for only 1
sec.

That is the biggest issue with having simplistic kernel interface that
apps will end up using wrongly and there is no way to control it.

This can be done in more clean way where multiple users can use this
MSRs and share it among themselves. That can be done using existing MSR
and CPUID drivers and no new kernel APIs.

> The same can be done for e.g. scheduler debug code in the kernel
> itself.
> I could imagine sched_mc_power developers will find this call
> convenient as it should be the only way to find out about the real
> frequency a core is running on. And they probably want to get this
> info *in the kernel* at least for debugging/monitoring. There are
> probably other use-cases.
> 
> This patch series is for splitting out average frequency into an own
> function and export it. I consider, getting the current running
> frequency of a processor, as an elementary function for upcoming
> machines where the cpufreq subsystem's values can be wrong (due to
> boost mode or HW restrictions).
> It may be that other kernel parts are really needing this or it could
> be that it's only used for monitoring/debugging, but I am pretty sure
> this interface will be useful. Hmm, I should have added lkml or x86
> list so that others could comment whether getting the average freq
> will be useful for developing, I thought this is obvious.
> Do you agree that the split and re-use of this code should happen?
> Should I repost with lkml added for broader discussion?
> 

I agree with splitting this code out and having some in kernel APIs for
scheduler, debugger, perfmon etc to use it. I don't like the new
kernel-user API. So, my comments are for your next patch

[PATCH 6/8] CPUFREQ: Add average frequency to sysfs exported files of
cpufreq_stats
  
> > Userlevel lib that can keep track of pid or some handle and track
> > multiple callers seems a better match for this.
> You can track multiple callers.
> I don't see the need of duplicating this in userspace, what exactly
> do you expect from a userspace implementation?
> Having this in cpufreq_stats should satisfy userspace monitoring
> needs. It avoids duplicated code and tests the in-kernel averg_perf
> implementation. It does only add 2 u64 per_cpu variables memory usage
> in a driver which is thought for debugging/monitoring anyway.
> 
> Will this (these) be accepted?

No, As I mentioned above having a userspace API not clearly defined is a
problem. And I dont like to add verhead of tracking each individual
caller (like ps or vmstat in above example) and returning appropriate
values for each caller from kernel. 

Thanks,
Venki


> PS: A general thing about such close to HW userspace tools:
> Could there be some cpuid library call in x86info propagated
> which makes checking for CPU features much more easy in C-/C++ code
> than parsing /proc/cpuinfo and /dev/cpu/X/{cpuid,msr}?
> I started with read_msr() and write_msr() library calls. Every little
> tool implementing the access to /dev/cpu/*/msr and parsing
> /proc/cpuinfo for CPU capabilities looks wrong.
> 
> Then the mysterious CPU flags could be replaced (longterm) at some
> time with a userspace tool, explaining what exactly they are for.
> Hmm, I better open an own thread for that, maybe Dave is even
> reading this...
> 

Agreed. Everytime I write some tiny app to look at cpuid and msr, I end
up reusing read_msr, write_msr kind of routines that I have in one of my
common lib file. They should be in some cpuid related library. May be it
is already there somewhere that we don't know about...

[PATCH 6/8] CPUFREQ: Add average frequency to sysfs exported files of cpufreq_stats

[Thomas Renninger]

Average frequency can now be monitored easily (e.g. every second) by:
watch -n1 cat /sys/devices/system/cpu/cpu*/cpufreq/stats/average_freq
if CONFIG_CPU_FREQ_STAT is compiled and the cpufreq_stats module got
loaded.

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 drivers/cpufreq/cpufreq_stats.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index 5a62d67..64542dd 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -20,6 +20,9 @@
 #include <linux/spinlock.h>
 #include <linux/notifier.h>
 #include <asm/cputime.h>
+#ifdef CONFIG_CPU_FREQ_STAT_DETAILS
+#include <asm/processor.h>
+#endif
 
 static spinlock_t cpufreq_stats_lock;
 
@@ -44,6 +47,10 @@ struct cpufreq_stats {
 };
 
 static DEFINE_PER_CPU(struct cpufreq_stats *, cpufreq_stats_table);
+#ifdef CONFIG_X86_AVERAGE_FREQUENCY
+static DEFINE_PER_CPU(u64, saved_aperf);
+static DEFINE_PER_CPU(u64, saved_mperf);
+#endif
 
 struct cpufreq_stats_attribute {
 	struct attribute attr;
@@ -139,6 +146,28 @@ static ssize_t show_trans_table(struct cpufreq_policy *policy, char *buf)
 CPUFREQ_STATDEVICE_ATTR(trans_table, 0444, show_trans_table);
 #endif
 
+#ifdef CONFIG_X86_AVERAGE_FREQUENCY
+static ssize_t show_average_freq(struct cpufreq_policy *policy, char *buf)
+{
+	ssize_t len;
+	unsigned int average_perf;
+
+	if (!cpu_has(&current_cpu_data, X86_FEATURE_IDA)) {
+		len = sprintf(buf, "not supported\n");
+		return len;
+	}
+
+	average_perf = get_average_perf(policy, policy->cpu,
+					&per_cpu(saved_aperf, policy->cpu),
+					&per_cpu(saved_mperf, policy->cpu));
+
+	len = sprintf(buf, "%d\n", average_perf);
+	return len;
+}
+CPUFREQ_STATDEVICE_ATTR(average_freq, 0444, show_average_freq);
+#endif
+
+
 CPUFREQ_STATDEVICE_ATTR(total_trans, 0444, show_total_trans);
 CPUFREQ_STATDEVICE_ATTR(time_in_state, 0444, show_time_in_state);
 
@@ -148,6 +177,9 @@ static struct attribute *default_attrs[] = {
 #ifdef CONFIG_CPU_FREQ_STAT_DETAILS
 	&_attr_trans_table.attr,
 #endif
+#ifdef CONFIG_X86_AVERAGE_FREQUENCY
+	&_attr_average_freq.attr,
+#endif
 	NULL
 };
 static struct attribute_group stats_attr_group = {
-- 
1.6.0.2

[PATCH 7/8] acpi-cpufreq: Use already defined IDA feature flag instead of checking cpuid

[Thomas Renninger]

It's already defined here:
arch/x86/kernel/cpu/addon_cpuid_features.c:
	{ X86_FEATURE_IDA, CR_EAX, 1, 0x00000006 },

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
index 148857a..aaf832f 100644
--- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
+++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
@@ -60,7 +60,6 @@ enum {
 };
 
 #define INTEL_MSR_RANGE		(0xffff)
-#define CPUID_6_ECX_APERFMPERF_CAPABILITY	(0x1)
 
 struct acpi_cpufreq_data {
 	struct acpi_processor_performance *acpi_data;
@@ -645,12 +644,9 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	acpi_processor_notify_smm(THIS_MODULE);
 
 	/* Check for APERF/MPERF support in hardware */
-	if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 6) {
-		unsigned int ecx;
-		ecx = cpuid_ecx(6);
-		if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY)
-			acpi_cpufreq_driver.getavg = get_measured_perf;
-	}
+	if (cpu_has(&current_cpu_data, X86_FEATURE_IDA))
+		acpi_cpufreq_driver.getavg = get_measured_perf;
+
 
 	dprintk("CPU%u - ACPI performance management activated.\n", cpu);
 	for (i = 0; i < perf->state_count; i++)
-- 
1.6.0.2

Re: [PATCH 7/8] acpi-cpufreq: Use already defined IDA feature flag instead of checking cpuid

[Pallipadi, Venkatesh]

Hmm.. This patch is mixing up two different flags here. APERF/MPERF
feature is independent of IDA, can exist on platforms without IDA as
well.

APERFMPERF is bit 0 of cpuid_ecx(6)
IDA is bit 1 of cpuid_eax(6)

Thanks,
Venki

On Fri, 2009-04-17 at 07:22 -0700, Thomas Renninger wrote:
> It's already defined here:
> arch/x86/kernel/cpu/addon_cpuid_features.c:
> 	{ X86_FEATURE_IDA, CR_EAX, 1, 0x00000006 },
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
> Cc: <cpufreq@vger.kernel.org>
> ---
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |   10 +++-------
>  1 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 148857a..aaf832f 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -60,7 +60,6 @@ enum {
>  };
>  
>  #define INTEL_MSR_RANGE		(0xffff)
> -#define CPUID_6_ECX_APERFMPERF_CAPABILITY	(0x1)
>  
>  struct acpi_cpufreq_data {
>  	struct acpi_processor_performance *acpi_data;
> @@ -645,12 +644,9 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	acpi_processor_notify_smm(THIS_MODULE);
>  
>  	/* Check for APERF/MPERF support in hardware */
> -	if (c->x86_vendor == X86_VENDOR_INTEL && c->cpuid_level >= 6) {
> -		unsigned int ecx;
> -		ecx = cpuid_ecx(6);
> -		if (ecx & CPUID_6_ECX_APERFMPERF_CAPABILITY)
> -			acpi_cpufreq_driver.getavg = get_measured_perf;
> -	}
> +	if (cpu_has(&current_cpu_data, X86_FEATURE_IDA))
> +		acpi_cpufreq_driver.getavg = get_measured_perf;
> +
>  
>  	dprintk("CPU%u - ACPI performance management activated.\n", cpu);
>  	for (i = 0; i < perf->state_count; i++)

[PATCH 8/8] CPUFREQ: Add documentation for new average_freq cpufreq_stats sysfs file

[Thomas Renninger]

Signed-off-by: Thomas Renninger <trenn@suse.de>
Cc: <linux-acpi@vger.kernel.org>
Cc: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: <cpufreq@vger.kernel.org>
---
 Documentation/cpu-freq/cpufreq-stats.txt |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/Documentation/cpu-freq/cpufreq-stats.txt b/Documentation/cpu-freq/cpufreq-stats.txt
index fc64749..aa05b65 100644
--- a/Documentation/cpu-freq/cpufreq-stats.txt
+++ b/Documentation/cpu-freq/cpufreq-stats.txt
@@ -44,11 +44,28 @@ the stats driver insertion.
 total 0
 drwxr-xr-x  2 root root    0 May 14 16:06 .
 drwxr-xr-x  3 root root    0 May 14 15:58 ..
+-r--r--r--  1 root root 4096 May 14 16:06 average_freq
 -r--r--r--  1 root root 4096 May 14 16:06 time_in_state
 -r--r--r--  1 root root 4096 May 14 16:06 total_trans
 -r--r--r--  1 root root 4096 May 14 16:06 trans_table
 --------------------------------------------------------------------------------
 
+-  average_freq
+Shows the average frequency the processor stayed in since the file was read
+the last time. Zero is shown if it is read the first time or "not supported".
+
+CONFIG_X86_AVERAGE_FREQUENCY=y config needs to be set and the processor must
+support overclocking of single cores (ida flag in /proc/cpuinfo).
+This is the only way to detect, monitor and optimze for the feature that the
+BIOS can overclock single CPU cores behind the kernel's back if thermal
+requirements are met (e.g. other cores of the socket run at low frequency or
+sleep).
+
+Also on multi-core processors the shown current CPU frequency the machine is
+running on in cpufreq/cpufreq_scaling_cur and /proc/cpuinfo can be wrong with
+current kernels if the frequency of several cores must be the same because of
+HW restritions. average_freq should show the real frequency.
+
 -  time_in_state
 This gives the amount of time spent in each of the frequencies supported by
 this CPU. The cat output will have "<frequency> <time>" pair in each line, which
-- 
1.6.0.2