| Message ID | 1386924222-23169-1-git-send-email-vkale@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: > Print the cpu frequency field in /proc/cpuinfo. Why? ["x86 does this" isn't a valid answer :)]. > Signed-off-by: Vinayak Kale <vkale@apm.com> > --- > arch/arm64/kernel/setup.c | 15 +++++++++++++++ > 1 files changed, 15 insertions(+), 0 deletions(-) > > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index bd9bbd0..bca982b 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -41,6 +41,7 @@ > #include <linux/memblock.h> > #include <linux/of_fdt.h> > #include <linux/of_platform.h> > +#include <linux/cpufreq.h> > > #include <asm/cputype.h> > #include <asm/elf.h> > @@ -278,9 +279,19 @@ static const char *hwcap_str[] = { > NULL > }; > > +static unsigned int get_cpufreq_khz(unsigned int cpu) > +{ > +#ifdef CONFIG_CPU_FREQ > + return cpufreq_quick_get(cpu); > +#else > + return 0; > +#endif > +} That's going to be hilarious fun when cpufreq isn't compiled in. Just use the existing sysfs interfaces exposed by the cpufreq governers if you need to know this stuff. Will
On Fri, Dec 13, 2013 at 4:06 PM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: >> Print the cpu frequency field in /proc/cpuinfo. > > Why? ["x86 does this" isn't a valid answer :)]. Well, that's actually the main motive :-) Secondly, earlier in amr64 port, /proc/cpuinfo used to print BogoMIPS. So people used to get mislead with the values. Although in current tree, we don't have that print. So that's not much of a concern now. (You had removed it I guess for the same reason :)) > >> Signed-off-by: Vinayak Kale <vkale@apm.com> >> --- >> arch/arm64/kernel/setup.c | 15 +++++++++++++++ >> 1 files changed, 15 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c >> index bd9bbd0..bca982b 100644 >> --- a/arch/arm64/kernel/setup.c >> +++ b/arch/arm64/kernel/setup.c >> @@ -41,6 +41,7 @@ >> #include <linux/memblock.h> >> #include <linux/of_fdt.h> >> #include <linux/of_platform.h> >> +#include <linux/cpufreq.h> >> >> #include <asm/cputype.h> >> #include <asm/elf.h> >> @@ -278,9 +279,19 @@ static const char *hwcap_str[] = { >> NULL >> }; >> >> +static unsigned int get_cpufreq_khz(unsigned int cpu) >> +{ >> +#ifdef CONFIG_CPU_FREQ >> + return cpufreq_quick_get(cpu); >> +#else >> + return 0; >> +#endif >> +} > > That's going to be hilarious fun when cpufreq isn't compiled in. In that case, we are not printing the value since above function will return 0. > Just use the existing sysfs interfaces exposed by the cpufreq governers if > you need to know this stuff. > > Will
On Fri, Dec 13, 2013 at 4:36 AM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: >> Print the cpu frequency field in /proc/cpuinfo. > > Why? ["x86 does this" isn't a valid answer :)]. People want to know this stuff. How about making lscpu work and print useful things. Here is what you get on x66. You get a lot less on ARM IIRC. lscpu uses on cpuinfo and sysfs data. The cache info is based on sysfs cache data which I believe someone is working on for ARM. Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit Byte Order: Little Endian CPU(s): 4 On-line CPU(s) list: 0-3 Thread(s) per core: 2 Core(s) per socket: 2 Socket(s): 1 NUMA node(s): 1 Vendor ID: GenuineIntel CPU family: 6 Model: 37 Stepping: 5 CPU MHz: 2393.881 BogoMIPS: 4787.76 Virtualization: VT-x L1d cache: 32K L1i cache: 32K L2 cache: 256K L3 cache: 3072K NUMA node0 CPU(s): 0-3 >> Signed-off-by: Vinayak Kale <vkale@apm.com> >> --- >> arch/arm64/kernel/setup.c | 15 +++++++++++++++ >> 1 files changed, 15 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c >> index bd9bbd0..bca982b 100644 >> --- a/arch/arm64/kernel/setup.c >> +++ b/arch/arm64/kernel/setup.c >> @@ -41,6 +41,7 @@ >> #include <linux/memblock.h> >> #include <linux/of_fdt.h> >> #include <linux/of_platform.h> >> +#include <linux/cpufreq.h> >> >> #include <asm/cputype.h> >> #include <asm/elf.h> >> @@ -278,9 +279,19 @@ static const char *hwcap_str[] = { >> NULL >> }; >> >> +static unsigned int get_cpufreq_khz(unsigned int cpu) >> +{ >> +#ifdef CONFIG_CPU_FREQ >> + return cpufreq_quick_get(cpu); >> +#else >> + return 0; >> +#endif >> +} > > That's going to be hilarious fun when cpufreq isn't compiled in. > Just use the existing sysfs interfaces exposed by the cpufreq governers if > you need to know this stuff. We should be able to get the frequency independently from cpufreq being enabled. This should come from DT cpu clock-frequency property if cpufreq is not enabled. Rob
On Fri, Dec 13, 2013 at 08:16:16AM -0600, Rob Herring wrote: > On Fri, Dec 13, 2013 at 4:36 AM, Will Deacon <will.deacon@arm.com> wrote: > > On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: > >> Print the cpu frequency field in /proc/cpuinfo. > > > > Why? ["x86 does this" isn't a valid answer :)]. > > People want to know this stuff. People want to know this stuff so they can do stupid things in userspace, like make the assumption that if it gets reported that the CPU is running at N MHz, then that means to delay M us, they need Z instructions in a loop. We've also had people want the cache information in userspace, so that they can use that to make a decision on whether function X would be faster than Y, rather than measuring each implementation and basing it off measurement - we've shown in the past that the cache information doesn't let you make that kind of decision, because the result is affected not only by the cache but many other parameters as well (such as the implementation of the CPU itself.) I've been dead against exporting cache information from the kernel into userspace: it's not something that userspace should ever concern itself with, and it's not something that userspace should ever use to make decisions about what code should be run. So, "people want to know this stuff" is a poor reason. A good reason is "If I have access to X, then it allows me to do Y, and this is the right way to allow me to do Y". So far, I haven't seen any evidence that the export of cache information gives the right solution to any userspace problem. Hence, my position is still the same as above, and this information should *never* be exported to userspace.
On Fri, Dec 13, 2013 at 8:24 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Dec 13, 2013 at 08:16:16AM -0600, Rob Herring wrote: >> On Fri, Dec 13, 2013 at 4:36 AM, Will Deacon <will.deacon@arm.com> wrote: >> > On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: >> >> Print the cpu frequency field in /proc/cpuinfo. >> > >> > Why? ["x86 does this" isn't a valid answer :)]. >> >> People want to know this stuff. > > People want to know this stuff so they can do stupid things in userspace, > like make the assumption that if it gets reported that the CPU is running > at N MHz, then that means to delay M us, they need Z instructions in a > loop. > > We've also had people want the cache information in userspace, so that > they can use that to make a decision on whether function X would be > faster than Y, rather than measuring each implementation and basing it > off measurement - we've shown in the past that the cache information > doesn't let you make that kind of decision, because the result is > affected not only by the cache but many other parameters as well (such > as the implementation of the CPU itself.) > > I've been dead against exporting cache information from the kernel into > userspace: it's not something that userspace should ever concern itself > with, and it's not something that userspace should ever use to make > decisions about what code should be run. > > So, "people want to know this stuff" is a poor reason. A good reason is > "If I have access to X, then it allows me to do Y, and this is the right > way to allow me to do Y". So far, I haven't seen any evidence that the > export of cache information gives the right solution to any userspace > problem. Hence, my position is still the same as above, and this > information should *never* be exported to userspace. I agree using it for functional purposes in this way is stupid. It is more for informational purposes than functional reasons. The use case I hear is server OEMs want to run some tool(s) to dump out system information for support requests. You can argue all day long that there are other ways to do something, but they already have the infrastructure in place on x86. For example, they may want to know if the user changed the CPU or DIMMs. Frequency and cache sizes may be part of the SKU. One way or another, this information is going to get to userspace. The real question is should the kernel provide a standard cross-arch interface to various fields or just pass this information thru to userspace via DMI tables, /proc/device-tree, ACPI, etc. I'd rather see this presented in a standard way so userspace does not have to change depending on whether the data comes from DT, DMI, ACPI or somewhere else. Rob
On Fri, Dec 13, 2013 at 7:46 PM, Rob Herring <robherring2@gmail.com> wrote: > On Fri, Dec 13, 2013 at 4:36 AM, Will Deacon <will.deacon@arm.com> wrote: >> On Fri, Dec 13, 2013 at 08:43:42AM +0000, Vinayak Kale wrote: >>> Print the cpu frequency field in /proc/cpuinfo. >> >> Why? ["x86 does this" isn't a valid answer :)]. > > People want to know this stuff. How about making lscpu work and print > useful things. Here is what you get on x66. You get a lot less on ARM > IIRC. lscpu uses on cpuinfo and sysfs data. The cache info is based on > sysfs cache data which I believe someone is working on for ARM. > > Architecture: x86_64 > CPU op-mode(s): 32-bit, 64-bit > Byte Order: Little Endian > CPU(s): 4 > On-line CPU(s) list: 0-3 > Thread(s) per core: 2 > Core(s) per socket: 2 > Socket(s): 1 > NUMA node(s): 1 > Vendor ID: GenuineIntel > CPU family: 6 > Model: 37 > Stepping: 5 > CPU MHz: 2393.881 > BogoMIPS: 4787.76 > Virtualization: VT-x > L1d cache: 32K > L1i cache: 32K > L2 cache: 256K > L3 cache: 3072K > NUMA node0 CPU(s): 0-3 > > >>> Signed-off-by: Vinayak Kale <vkale@apm.com> >>> --- >>> arch/arm64/kernel/setup.c | 15 +++++++++++++++ >>> 1 files changed, 15 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c >>> index bd9bbd0..bca982b 100644 >>> --- a/arch/arm64/kernel/setup.c >>> +++ b/arch/arm64/kernel/setup.c >>> @@ -41,6 +41,7 @@ >>> #include <linux/memblock.h> >>> #include <linux/of_fdt.h> >>> #include <linux/of_platform.h> >>> +#include <linux/cpufreq.h> >>> >>> #include <asm/cputype.h> >>> #include <asm/elf.h> >>> @@ -278,9 +279,19 @@ static const char *hwcap_str[] = { >>> NULL >>> }; >>> >>> +static unsigned int get_cpufreq_khz(unsigned int cpu) >>> +{ >>> +#ifdef CONFIG_CPU_FREQ >>> + return cpufreq_quick_get(cpu); >>> +#else >>> + return 0; >>> +#endif >>> +} >> >> That's going to be hilarious fun when cpufreq isn't compiled in. >> Just use the existing sysfs interfaces exposed by the cpufreq governers if >> you need to know this stuff. > > We should be able to get the frequency independently from cpufreq > being enabled. This should come from DT cpu clock-frequency property > if cpufreq is not enabled. Reading cpu-freq from DT won't work in case of cpu frequency scaling. > > Rob
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index bd9bbd0..bca982b 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -41,6 +41,7 @@ #include <linux/memblock.h> #include <linux/of_fdt.h> #include <linux/of_platform.h> +#include <linux/cpufreq.h> #include <asm/cputype.h> #include <asm/elf.h> @@ -278,9 +279,19 @@ static const char *hwcap_str[] = { NULL }; +static unsigned int get_cpufreq_khz(unsigned int cpu) +{ +#ifdef CONFIG_CPU_FREQ + return cpufreq_quick_get(cpu); +#else + return 0; +#endif +} + static int c_show(struct seq_file *m, void *v) { int i; + unsigned int cpufreq_khz; seq_printf(m, "Processor\t: %s rev %d (%s)\n", cpu_name, read_cpuid_id() & 15, ELF_PLATFORM); @@ -294,6 +305,10 @@ static int c_show(struct seq_file *m, void *v) #ifdef CONFIG_SMP seq_printf(m, "processor\t: %d\n", i); #endif + cpufreq_khz = get_cpufreq_khz(i); + if (cpufreq_khz) + seq_printf(m, "cpu MHz\t\t: %u.%02u\n", + cpufreq_khz / 1000, (cpufreq_khz % 1000)); } /* dump out the processor features */
Print the cpu frequency field in /proc/cpuinfo. Signed-off-by: Vinayak Kale <vkale@apm.com> --- arch/arm64/kernel/setup.c | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)