2005-12-02 18:14:17

by Pallipadi, Venkatesh

[permalink] [raw]
Subject: [PATCH] CPU frequency display in /proc/cpuinfo



What is the value shown in "cpu MHz" of /proc/cpuinfo when CPUs are capable of
changing frequency?

Today the answer is: It depends.
On i386:
SMP kernel - It is always the boot frequency
UP kernel - Scales with the frequency change and shows that was last set.

On x86_64:
There is one single variable cpu_khz that gets written by all the CPUs. So,
the frequency set by last CPU will be seen on /proc/cpuinfo of all the
CPUs in the system. What you see also depends on whether you have constant_tsc
capable CPU or not.

On ia64:
It is always boot time frequency of a particular CPU that gets displayed.


The patch below changes this to:
Show the last known frequency of the particular CPU, when cpufreq is present. If
cpu doesnot support changing of frequency through cpufreq, then boot frequency
will be shown. The patch affects i386, x86_64 and ia64 architectures.

Signed-off-by: Venkatesh Pallipadi<[email protected]>

Index: linux-2.6.12/arch/i386/kernel/cpu/proc.c
===================================================================
--- linux-2.6.12.orig/arch/i386/kernel/cpu/proc.c 2005-08-30 11:10:46.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/cpu/proc.c 2005-10-07 15:39:48.000000000 -0700
@@ -3,6 +3,7 @@
#include <linux/string.h>
#include <asm/semaphore.h>
#include <linux/seq_file.h>
+#include <linux/cpufreq.h>

/*
* Get CPU information for use by the procfs.
@@ -86,8 +87,11 @@
seq_printf(m, "stepping\t: unknown\n");

if ( cpu_has(c, X86_FEATURE_TSC) ) {
+ unsigned int freq = cpufreq_quick_get(n);
+ if (!freq)
+ freq = cpu_khz;
seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
- cpu_khz / 1000, (cpu_khz % 1000));
+ freq / 1000, (freq % 1000));
}

/* Cache size */
Index: linux-2.6.12/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6.12.orig/drivers/cpufreq/cpufreq.c 2005-09-26 14:59:23.000000000 -0700
+++ linux-2.6.12/drivers/cpufreq/cpufreq.c 2005-10-07 15:46:08.000000000 -0700
@@ -830,6 +830,30 @@


/**
+ * cpufreq_quick_get - get the CPU frequency (in kHz) frpm policy->cur
+ * @cpu: CPU number
+ *
+ * This is the last known freq, without actually getting it from the driver.
+ * Return value will be same as what is shown in scaling_cur_freq in sysfs.
+ */
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+ unsigned int ret = 0;
+
+ if (policy) {
+ down(&policy->lock);
+ ret = policy->cur;
+ up(&policy->lock);
+ cpufreq_cpu_put(policy);
+ }
+
+ return (ret);
+}
+EXPORT_SYMBOL(cpufreq_quick_get);
+
+
+/**
* cpufreq_get - get the current CPU frequency (in kHz)
* @cpu: CPU number
*
Index: linux-2.6.12/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/setup.c 2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/x86_64/kernel/setup.c 2005-10-07 15:40:24.000000000 -0700
@@ -42,6 +42,7 @@
#include <linux/mmzone.h>
#include <linux/kexec.h>
#include <linux/crash_dump.h>
+#include <linux/cpufreq.h>

#include <asm/mtrr.h>
#include <asm/uaccess.h>
@@ -1187,8 +1188,11 @@
seq_printf(m, "stepping\t: unknown\n");

if (cpu_has(c,X86_FEATURE_TSC)) {
+ unsigned int freq = cpufreq_quick_get((unsigned)(c-cpu_data));
+ if (!freq)
+ freq = cpu_khz;
seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
- cpu_khz / 1000, (cpu_khz % 1000));
+ freq / 1000, (freq % 1000));
}

/* Cache size */
Index: linux-2.6.12/arch/ia64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/ia64/kernel/setup.c 2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/ia64/kernel/setup.c 2005-10-07 15:41:38.000000000 -0700
@@ -43,6 +43,7 @@
#include <linux/initrd.h>
#include <linux/platform.h>
#include <linux/pm.h>
+#include <linux/cpufreq.h>

#include <asm/ia32.h>
#include <asm/machvec.h>
@@ -474,6 +475,7 @@
char family[32], features[128], *cp, sep;
struct cpuinfo_ia64 *c = v;
unsigned long mask;
+ unsigned int proc_freq;
int i;

mask = c->features;
@@ -506,6 +508,10 @@
sprintf(cp, " 0x%lx", mask);
}

+ proc_freq = cpufreq_quick_get(cpunum);
+ if (!proc_freq)
+ proc_freq = c->proc_freq / 1000;
+
seq_printf(m,
"processor : %d\n"
"vendor : %s\n"
@@ -522,7 +528,7 @@
"BogoMIPS : %lu.%02lu\n",
cpunum, c->vendor, family, c->model, c->revision, c->archrev,
features, c->ppn, c->number,
- c->proc_freq / 1000000, c->proc_freq % 1000000,
+ proc_freq / 1000, proc_freq % 1000,
c->itc_freq / 1000000, c->itc_freq % 1000000,
lpj*HZ/500000, (lpj*HZ/5000) % 100);
#ifdef CONFIG_SMP
Index: linux-2.6.12/include/linux/cpufreq.h
===================================================================
--- linux-2.6.12.orig/include/linux/cpufreq.h 2005-09-26 14:59:25.000000000 -0700
+++ linux-2.6.12/include/linux/cpufreq.h 2005-10-07 14:19:05.000000000 -0700
@@ -259,6 +259,16 @@
/* query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it */
unsigned int cpufreq_get(unsigned int cpu);

+/* query the last known CPU freq (in kHz). If zero, cpufreq couldn't detect it */
+#ifdef CONFIG_CPU_FREQ
+unsigned int cpufreq_quick_get(unsigned int cpu);
+#else
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ return 0;
+}
+#endif
+

/*********************************************************************
* CPUFREQ DEFAULT GOVERNOR *


2005-12-02 18:19:31

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Fri, Dec 02, 2005 at 10:13:31AM -0800, Pallipadi, Venkatesh wrote:
> On x86_64:
> There is one single variable cpu_khz that gets written by all the CPUs. So,
> the frequency set by last CPU will be seen on /proc/cpuinfo of all the
> CPUs in the system. What you see also depends on whether you have constant_tsc
> capable CPU or not.

x86-64 part looks good. Thanks.

> /* query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it */
> unsigned int cpufreq_get(unsigned int cpu);
>
> +/* query the last known CPU freq (in kHz). If zero, cpufreq couldn't detect it */
> +#ifdef CONFIG_CPU_FREQ
> +unsigned int cpufreq_quick_get(unsigned int cpu);
> +#else
> +unsigned int cpufreq_quick_get(unsigned int cpu)
> +{
> + return 0;
> +}
> +#endif

Shouldn't this be a static inline?

-Andi

2005-12-02 18:43:48

by Pallipadi, Venkatesh

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Fri, Dec 02, 2005 at 10:19:27AM -0800, Andi Kleen wrote:
> Shouldn't this be a static inline?

Yes. Attached is the modified patch.

Thanks,
Venki

What is the value shown in "cpu MHz" of /proc/cpuinfo when CPUs are capable of
changing frequency?

Today the answer is: It depends.
On i386:
SMP kernel - It is always the boot frequency
UP kernel - Scales with the frequency change and shows that was last set.

On x86_64:
There is one single variable cpu_khz that gets written by all the CPUs. So,
the frequency set by last CPU will be seen on /proc/cpuinfo of all the
CPUs in the system. What you see also depends on whether you have constant_tsc
capable CPU or not.

On ia64:
It is always boot time frequency of a particular CPU that gets displayed.


The patch below changes this to:
Show the last known frequency of the particular CPU, when cpufreq is present. If
cpu doesnot support changing of frequency through cpufreq, then boot frequency
will be shown. The patch affects i386, x86_64 and ia64 architectures.

Signed-off-by: Venkatesh Pallipadi<[email protected]>

Index: linux-2.6.12/arch/i386/kernel/cpu/proc.c
===================================================================
--- linux-2.6.12.orig/arch/i386/kernel/cpu/proc.c 2005-08-30 11:10:46.000000000 -0700
+++ linux-2.6.12/arch/i386/kernel/cpu/proc.c 2005-10-07 15:39:48.000000000 -0700
@@ -3,6 +3,7 @@
#include <linux/string.h>
#include <asm/semaphore.h>
#include <linux/seq_file.h>
+#include <linux/cpufreq.h>

/*
* Get CPU information for use by the procfs.
@@ -86,8 +87,11 @@
seq_printf(m, "stepping\t: unknown\n");

if ( cpu_has(c, X86_FEATURE_TSC) ) {
+ unsigned int freq = cpufreq_quick_get(n);
+ if (!freq)
+ freq = cpu_khz;
seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
- cpu_khz / 1000, (cpu_khz % 1000));
+ freq / 1000, (freq % 1000));
}

/* Cache size */
Index: linux-2.6.12/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6.12.orig/drivers/cpufreq/cpufreq.c 2005-09-26 14:59:23.000000000 -0700
+++ linux-2.6.12/drivers/cpufreq/cpufreq.c 2005-10-07 15:46:08.000000000 -0700
@@ -830,6 +830,30 @@


/**
+ * cpufreq_quick_get - get the CPU frequency (in kHz) frpm policy->cur
+ * @cpu: CPU number
+ *
+ * This is the last known freq, without actually getting it from the driver.
+ * Return value will be same as what is shown in scaling_cur_freq in sysfs.
+ */
+unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+ unsigned int ret = 0;
+
+ if (policy) {
+ down(&policy->lock);
+ ret = policy->cur;
+ up(&policy->lock);
+ cpufreq_cpu_put(policy);
+ }
+
+ return (ret);
+}
+EXPORT_SYMBOL(cpufreq_quick_get);
+
+
+/**
* cpufreq_get - get the current CPU frequency (in kHz)
* @cpu: CPU number
*
Index: linux-2.6.12/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/x86_64/kernel/setup.c 2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/x86_64/kernel/setup.c 2005-10-07 15:40:24.000000000 -0700
@@ -42,6 +42,7 @@
#include <linux/mmzone.h>
#include <linux/kexec.h>
#include <linux/crash_dump.h>
+#include <linux/cpufreq.h>

#include <asm/mtrr.h>
#include <asm/uaccess.h>
@@ -1187,8 +1188,11 @@
seq_printf(m, "stepping\t: unknown\n");

if (cpu_has(c,X86_FEATURE_TSC)) {
+ unsigned int freq = cpufreq_quick_get((unsigned)(c-cpu_data));
+ if (!freq)
+ freq = cpu_khz;
seq_printf(m, "cpu MHz\t\t: %u.%03u\n",
- cpu_khz / 1000, (cpu_khz % 1000));
+ freq / 1000, (freq % 1000));
}

/* Cache size */
Index: linux-2.6.12/arch/ia64/kernel/setup.c
===================================================================
--- linux-2.6.12.orig/arch/ia64/kernel/setup.c 2005-08-31 14:46:39.000000000 -0700
+++ linux-2.6.12/arch/ia64/kernel/setup.c 2005-10-07 15:41:38.000000000 -0700
@@ -43,6 +43,7 @@
#include <linux/initrd.h>
#include <linux/platform.h>
#include <linux/pm.h>
+#include <linux/cpufreq.h>

#include <asm/ia32.h>
#include <asm/machvec.h>
@@ -474,6 +475,7 @@
char family[32], features[128], *cp, sep;
struct cpuinfo_ia64 *c = v;
unsigned long mask;
+ unsigned int proc_freq;
int i;

mask = c->features;
@@ -506,6 +508,10 @@
sprintf(cp, " 0x%lx", mask);
}

+ proc_freq = cpufreq_quick_get(cpunum);
+ if (!proc_freq)
+ proc_freq = c->proc_freq / 1000;
+
seq_printf(m,
"processor : %d\n"
"vendor : %s\n"
@@ -522,7 +528,7 @@
"BogoMIPS : %lu.%02lu\n",
cpunum, c->vendor, family, c->model, c->revision, c->archrev,
features, c->ppn, c->number,
- c->proc_freq / 1000000, c->proc_freq % 1000000,
+ proc_freq / 1000, proc_freq % 1000,
c->itc_freq / 1000000, c->itc_freq % 1000000,
lpj*HZ/500000, (lpj*HZ/5000) % 100);
#ifdef CONFIG_SMP
Index: linux-2.6.12/include/linux/cpufreq.h
===================================================================
--- linux-2.6.12.orig/include/linux/cpufreq.h 2005-09-26 14:59:25.000000000 -0700
+++ linux-2.6.12/include/linux/cpufreq.h 2005-10-07 14:19:05.000000000 -0700
@@ -259,6 +259,16 @@
/* query the current CPU frequency (in kHz). If zero, cpufreq couldn't detect it */
unsigned int cpufreq_get(unsigned int cpu);

+/* query the last known CPU freq (in kHz). If zero, cpufreq couldn't detect it */
+#ifdef CONFIG_CPU_FREQ
+unsigned int cpufreq_quick_get(unsigned int cpu);
+#else
+static inline unsigned int cpufreq_quick_get(unsigned int cpu)
+{
+ return 0;
+}
+#endif
+

/*********************************************************************
* CPUFREQ DEFAULT GOVERNOR *

2005-12-04 16:43:38

by Dominik Brodowski

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> The patch below changes this to:
> Show the last known frequency of the particular CPU, when cpufreq is present. If
> cpu doesnot support changing of frequency through cpufreq, then boot frequency
> will be shown. The patch affects i386, x86_64 and ia64 architectures.

Looks good to me -- however, might this affect userspace cpufreq tools? I'd
vote for quite some time in -mm for this patch (i.e. only merge for 2.6.17)

Dominik

2005-12-04 18:32:47

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, Dec 04, 2005 at 05:43:35PM +0100, Dominik Brodowski wrote:
> On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> > The patch below changes this to:
> > Show the last known frequency of the particular CPU, when cpufreq is present. If
> > cpu doesnot support changing of frequency through cpufreq, then boot frequency
> > will be shown. The patch affects i386, x86_64 and ia64 architectures.
>
> Looks good to me -- however, might this affect userspace cpufreq tools? I'd

They normally use /sys anyways.

> vote for quite some time in -mm for this patch (i.e. only merge for 2.6.17)

Actually it just changes behaviour back to older kernels (~2.6.10 or earlier)
which always behaved like this. So it should be safe.

-Andi

2005-12-04 20:07:36

by Lee Revell

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, 2005-12-04 at 19:32 +0100, Andi Kleen wrote:
> On Sun, Dec 04, 2005 at 05:43:35PM +0100, Dominik Brodowski wrote:
> > On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> > > The patch below changes this to:
> > > Show the last known frequency of the particular CPU, when cpufreq is present. If
> > > cpu doesnot support changing of frequency through cpufreq, then boot frequency
> > > will be shown. The patch affects i386, x86_64 and ia64 architectures.
> >
> > Looks good to me -- however, might this affect userspace cpufreq tools? I'd
>
> They normally use /sys anyways.

Wrong, lots of userspace programs that need to know the CPU speed get it
from /proc/cpuinfo. It would be nice if there were a better API.

As long as you don't change the file format it should be OK.

Lee

2005-12-04 20:13:58

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, Dec 04, 2005 at 02:49:26PM -0500, Lee Revell wrote:
> On Sun, 2005-12-04 at 19:32 +0100, Andi Kleen wrote:
> > On Sun, Dec 04, 2005 at 05:43:35PM +0100, Dominik Brodowski wrote:
> > > On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> > > > The patch below changes this to:
> > > > Show the last known frequency of the particular CPU, when cpufreq is present. If
> > > > cpu doesnot support changing of frequency through cpufreq, then boot frequency
> > > > will be shown. The patch affects i386, x86_64 and ia64 architectures.
> > >
> > > Looks good to me -- however, might this affect userspace cpufreq tools? I'd
> >
> > They normally use /sys anyways.
>
> Wrong, lots of userspace programs that need to know the CPU speed get it
> from /proc/cpuinfo. It would be nice if there were a better API.

Talking about user space governours - I presume that is what
Dominik ment with "userspace cpufreq tools"

> As long as you don't change the file format it should be OK.

Great that we have your approval.

-Andi

2005-12-05 01:16:31

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, Dec 04, 2005 at 02:49:26PM -0500, Lee Revell wrote:
> On Sun, 2005-12-04 at 19:32 +0100, Andi Kleen wrote:
> > On Sun, Dec 04, 2005 at 05:43:35PM +0100, Dominik Brodowski wrote:
> > > On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> > > > The patch below changes this to:
> > > > Show the last known frequency of the particular CPU, when cpufreq is present. If
> > > > cpu doesnot support changing of frequency through cpufreq, then boot frequency
> > > > will be shown. The patch affects i386, x86_64 and ia64 architectures.
> > >
> > > Looks good to me -- however, might this affect userspace cpufreq tools? I'd
> >
> > They normally use /sys anyways.
>
> Wrong, lots of userspace programs that need to know the CPU speed get it
> from /proc/cpuinfo. It would be nice if there were a better API.

I can't think of a single valid reason why a program would want
to know the MHz rating of a CPU. Given that it's a) approximate,
b) subject to change due to power management, c) completely nonsensical
across CPU vendors, and d) only one of many variables regarding CPU
performance, any program that bases any decision on the values found
by parsing that field of /proc/cpuinfo is utterly broken beyond belief.

Adding any other interface to obtain this value is equally as broken.

Dave

2005-12-05 01:23:15

by Horst H. von Brand

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

Lee Revell <[email protected]> wrote:

[...]

> Wrong, lots of userspace programs that need to know the CPU speed get it
> from /proc/cpuinfo. It would be nice if there were a better API.

Why would a /user/ program need to know what the CPU speed is?

In any case, /proc is not the place for this, as it has nothing to do with
processes.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513

2005-12-05 13:02:29

by Erik Mouw

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, Dec 04, 2005 at 08:16:11PM -0500, Dave Jones wrote:
> I can't think of a single valid reason why a program would want
> to know the MHz rating of a CPU. Given that it's a) approximate,
> b) subject to change due to power management, c) completely nonsensical
> across CPU vendors, and d) only one of many variables regarding CPU
> performance, any program that bases any decision on the values found
> by parsing that field of /proc/cpuinfo is utterly broken beyond belief.

If you want a userspace governor to change the CPU speed, you need to
export the value to userland. There are several papers showing[1] that
such speed scheduling should be done by power-aware applications which
need to tell the OS what speed they require to run to meet their
processing needs.

I agree that /proc/cpuinfo shouldn't be used (though it is a nice
interface for humans to read about the CPU speed), but the current
sysfs interface should do.


Erik

[1] See for example
http://www.pds.twi.tudelft.nl/~pouwelse/energy_priority_scheduling.ps.gz
http://www.pds.twi.tudelft.nl/~pouwelse/power_aware_video_decoding.ps

--
+-- Erik Mouw -- http://www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

2005-12-05 15:32:04

by Lee Revell

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Sun, 2005-12-04 at 20:16 -0500, Dave Jones wrote:
> On Sun, Dec 04, 2005 at 02:49:26PM -0500, Lee Revell wrote:
> > On Sun, 2005-12-04 at 19:32 +0100, Andi Kleen wrote:
> > > On Sun, Dec 04, 2005 at 05:43:35PM +0100, Dominik Brodowski wrote:
> > > > On Fri, Dec 02, 2005 at 10:43:20AM -0800, Venkatesh Pallipadi wrote:
> > > > > The patch below changes this to:
> > > > > Show the last known frequency of the particular CPU, when cpufreq is present. If
> > > > > cpu doesnot support changing of frequency through cpufreq, then boot frequency
> > > > > will be shown. The patch affects i386, x86_64 and ia64 architectures.
> > > >
> > > > Looks good to me -- however, might this affect userspace cpufreq tools? I'd
> > >
> > > They normally use /sys anyways.
> >
> > Wrong, lots of userspace programs that need to know the CPU speed get it
> > from /proc/cpuinfo. It would be nice if there were a better API.
>
> I can't think of a single valid reason why a program would want
> to know the MHz rating of a CPU. Given that it's a) approximate,
> b) subject to change due to power management, c) completely nonsensical
> across CPU vendors, and d) only one of many variables regarding CPU
> performance, any program that bases any decision on the values found
> by parsing that field of /proc/cpuinfo is utterly broken beyond belief.

JACK needs to know the CPU speed in order to be able to use RDTSC for
timing. Yes that might be "broken" but gettimeofday() is simply not
fast enough for our use, we can't afford the overhead of thousands of
system calls per second. And until recently 99.999% of desktop machines
had a monotonic TSC so this worked very well.

I don't see how people can say that gettimeofday() is as fast as the
hardware allows, when gettimeofday() just uses RDTSC to interpolate
since the last timer tick but is ~40x slower than RDTSC on my system.

Lee

2005-12-05 15:59:05

by Mark Lord

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

>I can't think of a single valid reason why a program would want
>to know the MHz rating of a CPU.

Humans like to know what their machines are doing.

Simple as that: it's for the end-users, and the place they look
for it is always /proc/cpuinfo (since that works on every platform
and on kernels prior to the 2.[56].* series.

Not useful as an accurate number for any programming algorithms,
but it is used to satisfy human curiosity. A lot.

--ml

2005-12-05 16:29:06

by Avi Kivity

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

Dave Jones wrote:

>I can't think of a single valid reason why a program would want
>to know the MHz rating of a CPU. Given that it's a) approximate,
>b) subject to change due to power management, c) completely nonsensical
>across CPU vendors, and d) only one of many variables regarding CPU
>performance, any program that bases any decision on the values found
>by parsing that field of /proc/cpuinfo is utterly broken beyond belief.
>
>
Sometimes you need extremely low overhead time measurements, which need
not be too accurate. One way to do this is to dump rdtsc measurements
into some array, and later scale it using the cpu frequency.

I've done exactly this. The processes were pinned to their processors,
and there was no frequency scaling in effect. It worked very well.

2005-12-05 16:47:11

by linux-os (Dick Johnson)

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo


On Mon, 5 Dec 2005, Avi Kivity wrote:

> Dave Jones wrote:
>
>> I can't think of a single valid reason why a program would want
>> to know the MHz rating of a CPU. Given that it's a) approximate,
>> b) subject to change due to power management, c) completely nonsensical
>> across CPU vendors, and d) only one of many variables regarding CPU
>> performance, any program that bases any decision on the values found
>> by parsing that field of /proc/cpuinfo is utterly broken beyond belief.
>>
>>
> Sometimes you need extremely low overhead time measurements, which need
> not be too accurate. One way to do this is to dump rdtsc measurements
> into some array, and later scale it using the cpu frequency.
>
> I've done exactly this. The processes were pinned to their processors,
> and there was no frequency scaling in effect. It worked very well.

Also humans might need to know if the botherboard is set up properly,
or can even be set up properly to be able to run the CPU at its
expected speed. FYI, I tested an expensive "blade" server that
was not configurable to anything near its advertised specifications.
The contents of /proc/cpuinfo was one of the nails I used for
the vendor's coffin.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.13.4 on an i686 machine (5589.44 BogoMips).
Warning : 98.36% of all statistics are fiction.
.

****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [email protected] - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

2005-12-05 17:25:31

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Mon, Dec 05, 2005 at 02:02:24PM +0100, Erik Mouw wrote:
> On Sun, Dec 04, 2005 at 08:16:11PM -0500, Dave Jones wrote:
> > I can't think of a single valid reason why a program would want
> > to know the MHz rating of a CPU. Given that it's a) approximate,
> > b) subject to change due to power management, c) completely nonsensical
> > across CPU vendors, and d) only one of many variables regarding CPU
> > performance, any program that bases any decision on the values found
> > by parsing that field of /proc/cpuinfo is utterly broken beyond belief.
>
> If you want a userspace governor to change the CPU speed, you need to
> export the value to userland.

We have sysfs files for that.

Dave

2005-12-05 17:26:59

by Lee Revell

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Mon, 2005-12-05 at 12:25 -0500, Dave Jones wrote:
> On Mon, Dec 05, 2005 at 02:02:24PM +0100, Erik Mouw wrote:
> > On Sun, Dec 04, 2005 at 08:16:11PM -0500, Dave Jones wrote:
> > > I can't think of a single valid reason why a program would want
> > > to know the MHz rating of a CPU. Given that it's a) approximate,
> > > b) subject to change due to power management, c) completely nonsensical
> > > across CPU vendors, and d) only one of many variables regarding CPU
> > > performance, any program that bases any decision on the values found
> > > by parsing that field of /proc/cpuinfo is utterly broken beyond belief.
> >
> > If you want a userspace governor to change the CPU speed, you need to
> > export the value to userland.
>
> We have sysfs files for that.
>

OK thanks. Last time I asked on LKML about it (~ 6 months ago) I was
told there's no sysfs interface yet.

Lee

2005-12-05 17:27:01

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Mon, Dec 05, 2005 at 10:59:00AM -0500, Mark Lord wrote:
> >I can't think of a single valid reason why a program would want
^^^^^^^
> >to know the MHz rating of a CPU.
>
> Humans like to know what their machines are doing.

humans != programs

> Simple as that: it's for the end-users, and the place they look
> for it is always /proc/cpuinfo (since that works on every platform
> and on kernels prior to the 2.[56].* series.

Sure, 'cat' is the only reason its there at all.

> Not useful as an accurate number for any programming algorithms,
> but it is used to satisfy human curiosity. A lot.

I think we're in agreement, aren't we? :)

Dave

2005-12-05 17:28:13

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Mon, Dec 05, 2005 at 06:29:02PM +0200, Avi Kivity wrote:
> Dave Jones wrote:
>
> >I can't think of a single valid reason why a program would want
> >to know the MHz rating of a CPU. Given that it's a) approximate,
> >b) subject to change due to power management, c) completely nonsensical
> >across CPU vendors, and d) only one of many variables regarding CPU
> >performance, any program that bases any decision on the values found
> >by parsing that field of /proc/cpuinfo is utterly broken beyond belief.
> >
> >
> Sometimes you need extremely low overhead time measurements, which need
> not be too accurate. One way to do this is to dump rdtsc measurements
> into some array, and later scale it using the cpu frequency.
>
> I've done exactly this. The processes were pinned to their processors,
> and there was no frequency scaling in effect. It worked very well.

That code will break as soon as it's run on a CPU that uses P-states.
You can't "just scale" the value, there are other factors at work.

Dave

2005-12-05 18:37:00

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

> JACK needs to know the CPU speed in order to be able to use RDTSC for
> timing. Yes that might be "broken" but gettimeofday() is simply not

It is broken then. Was always broken, will be broken etc.

> fast enough for our use, we can't afford the overhead of thousands of
> system calls per second. And until recently 99.999% of desktop machines
> had a monotonic TSC so this worked very well.

You're wrong. First if you say "monotonic" you don't understand the problem.
Monotonicity is only a small part of it. The bigger one is just
getting the current frequency and figuring out of if the information
is safe to use.

Chips where it doesn't work:

- Intel Prescott and derivatives (newer Pentium 4, newer Xeon, on
Celeron you are still lucky because they normally don't have speedstep):
TSC runs at a different frequency than the current P state and P state often
varies with speedstep [These are the most common desktop chips in the world,
although not all have speedstep enabled. Many newer ones have though.]
- Intel P-M and earlier P4,P3 mobile chips, Athlon 64, Athlon XP-M,
Opteron etc:
TSC varies with P state and user space has no way to get timely
updates on P-state changes.
- VIA C3: I believe TSC stops in idle (at least it used to on
older versions, don't know if they fixed it)

-Andi

2005-12-06 11:13:51

by Erik Mouw

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Mon, Dec 05, 2005 at 12:25:13PM -0500, Dave Jones wrote:
> On Mon, Dec 05, 2005 at 02:02:24PM +0100, Erik Mouw wrote:
> > If you want a userspace governor to change the CPU speed, you need to
> > export the value to userland.
>
> We have sysfs files for that.

Earlier in this thread you said (I should have quoted that, my fault):

Adding any other interface to obtain this value is equally as broken.

So I'm confused, sysfs one of the "any other interfaces"...


Erik

--
+-- Erik Mouw -- http://www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

2005-12-06 16:56:41

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Tue, Dec 06, 2005 at 12:13:49PM +0100, Erik Mouw wrote:
> On Mon, Dec 05, 2005 at 12:25:13PM -0500, Dave Jones wrote:
> > On Mon, Dec 05, 2005 at 02:02:24PM +0100, Erik Mouw wrote:
> > > If you want a userspace governor to change the CPU speed, you need to
> > > export the value to userland.
> >
> > We have sysfs files for that.
>
> Earlier in this thread you said (I should have quoted that, my fault):
>
> Adding any other interface to obtain this value is equally as broken.
>
> So I'm confused, sysfs one of the "any other interfaces"...

userspace governors need to know the available frequencies to scale to,
which they obtain from sysfs. In addition, we maintain an index as to
which of those is currently chosen. However, programs should not rely
on this to be a "how fast is my CPU" status, as it's totally meaningless.
It's there purely for humans to see "Yes, X < Y, so I'm going at the
lower of the frequencies my CPU can do", not for programs to calculate
delays loops and such.

Dave


2005-12-06 17:35:37

by Erik Mouw

[permalink] [raw]
Subject: Re: [PATCH] CPU frequency display in /proc/cpuinfo

On Tue, Dec 06, 2005 at 11:56:07AM -0500, Dave Jones wrote:
> On Tue, Dec 06, 2005 at 12:13:49PM +0100, Erik Mouw wrote:
> > Earlier in this thread you said (I should have quoted that, my fault):
> >
> > Adding any other interface to obtain this value is equally as broken.
> >
> > So I'm confused, sysfs one of the "any other interfaces"...
>
> userspace governors need to know the available frequencies to scale to,
> which they obtain from sysfs. In addition, we maintain an index as to
> which of those is currently chosen. However, programs should not rely
> on this to be a "how fast is my CPU" status, as it's totally meaningless.
> It's there purely for humans to see "Yes, X < Y, so I'm going at the
> lower of the frequencies my CPU can do", not for programs to calculate
> delays loops and such.

Agreed, sysfs is the way to go.


Erik

--
+-- Erik Mouw -- http://www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands