| Message ID | 1367602547-19322-2-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi! > Now that we support a timer-backed delay loop, I'm quickly getting sick > and tired of people complaining that their beloved bogomips value has > decreased. You know who you are! > Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it > will likely break fragile userspace code which is parsing that stuff, so > instead replace it with a dummy value that can be chosen in the > Kconfig. So, instead of removing it you silently report invalid value? Note that there's no cpumhz display in procinfo... at least on OLPC I looked. And whether it is 30MHz or 3GHz cpu might be relevant information for userspace, and yes, bogomips used to be good enough for that. Actually it was mhz, mhz/2 or mhz*2 on pretty much all reasonable systems. > +choice > + prompt "BogoMIPs setting" > + default BOGOMIPS_MEDIUM > + help > + The BogoMIPs value reported by Linux is exactly what it sounds > + like: totally bogus. It is used to calibrate the delay loop, > + which may be backed by a timer clocked completely independently > + of the CPU. This is too ugly to live, sorry. > + config BOGOMIPS_SLOW > + bool "Slow (older machines)" > + help > + If you're comparing a faster machine with a slower machine, > + then you might want this option selected on one of them. > + > + config BOGOMIPS_MEDIUM > + bool "Medium (default)" > + help > + A BogoMIPS value for the masses. > + > + config BOGOMIPS_FAST > + bool "Fast (marketing)" > + help > + Some people believe that software runs faster with this > + setting so, if you're one of them, say Y here. April 1st came late this year? Pavel
On 07/05/13 00:16, Pavel Machek wrote: Hi Pawel, >> Now that we support a timer-backed delay loop, I'm quickly getting sick >> and tired of people complaining that their beloved bogomips value has >> decreased. You know who you are! > >> Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it >> will likely break fragile userspace code which is parsing that stuff, so >> instead replace it with a dummy value that can be chosen in the >> Kconfig. > > So, instead of removing it you silently report invalid value? Note Removing it would be an ABI breakage. Unfortunately. > that there's no cpumhz display in procinfo... at least on OLPC I > looked. And? What does the CPU frequency bring you when any modern CPU has some kind of cpufreq support that renders this information information pretty much meaningless? > And whether it is 30MHz or 3GHz cpu might be relevant information for > userspace, and yes, bogomips used to be good enough for that. Name one userspace application that extracts meaningful information out of the BogoMIPS field. Just one. > Actually it was mhz, mhz/2 or mhz*2 on pretty much all reasonable systems. So any number within a ratio from one to four is good enough for you? Wow. That pretty much proves the usefulness of this patch. Additionally, having switched arch_timer capable CPUs to a timer based delay instead of a loop-based delay means your precious pseudo-frequency is now a thing of the past. And guess what? Nothing broke so far. Because using BogoMIPS to deduct anything is *bogus*. Timer-based delay calibration has been around for about ages. I still remember the day (some 10 or 12 years ago) when DaveM switched the UltraSPARC port to use the timer, and my BogoMIPS went from ~1000 to 12. People moaned, DaveM held his position, and nothing happened. *Nothing*. End of story. M.
[ Yes, I'm late to the party ] On Tue, 7 May 2013, Marc Zyngier wrote: > On 07/05/13 00:16, Pavel Machek wrote: > > Hi Pawel, > > >> Now that we support a timer-backed delay loop, I'm quickly getting sick > >> and tired of people complaining that their beloved bogomips value has > >> decreased. You know who you are! > > > >> Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it > >> will likely break fragile userspace code which is parsing that stuff, so > >> instead replace it with a dummy value that can be chosen in the > >> Kconfig. > > > > So, instead of removing it you silently report invalid value? Note > > Removing it would be an ABI breakage. Unfortunately. Did you really write this with a straight face? > Name one userspace application that extracts meaningful information out > of the BogoMIPS field. Just one. Waitaminute... Didn't you just claim this would be an ABI break? So if no application can be found, where is the ABI breakage? This thread must be a joke. Please just kill the damn thing out of /proc/cpuinfo and see if any application notices. And if some do then they are already broken *today* and should be fixed. Nicolas
On Tue, May 14, 2013 at 2:54 PM, Nicolas Pitre <nico@fluxnic.net> wrote: > [ Yes, I'm late to the party ] > > On Tue, 7 May 2013, Marc Zyngier wrote: > >> On 07/05/13 00:16, Pavel Machek wrote: >> >> Hi Pawel, >> >> >> Now that we support a timer-backed delay loop, I'm quickly getting sick >> >> and tired of people complaining that their beloved bogomips value has >> >> decreased. You know who you are! >> > >> >> Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it >> >> will likely break fragile userspace code which is parsing that stuff, so >> >> instead replace it with a dummy value that can be chosen in the >> >> Kconfig. >> > >> > So, instead of removing it you silently report invalid value? Note >> >> Removing it would be an ABI breakage. Unfortunately. > > Did you really write this with a straight face? > >> Name one userspace application that extracts meaningful information out >> of the BogoMIPS field. Just one. > > Waitaminute... Didn't you just claim this would be an ABI break? > > So if no application can be found, where is the ABI breakage? lscpu But that is already "broken" on ARM because x86 uses "bogomips" and ARM uses "BogoMIPS". I agree reporting BogoMIPS is silly, but there are more differences between x86 and ARM. Somewhat meaningful fields like "cpu MHz" are missing on ARM for example. These are mainly useful for h/w inventory use which server folks tend to care about. > This thread must be a joke. Please just kill the damn thing out of > /proc/cpuinfo and see if any application notices. And if some do then > they are already broken *today* and should be fixed. And please do so for all arches then. Rob
On Tue, 14 May 2013, Rob Herring wrote: > On Tue, May 14, 2013 at 2:54 PM, Nicolas Pitre <nico@fluxnic.net> wrote: > > [ Yes, I'm late to the party ] > > > > On Tue, 7 May 2013, Marc Zyngier wrote: > > > >> On 07/05/13 00:16, Pavel Machek wrote: > >> > >> Hi Pawel, > >> > >> >> Now that we support a timer-backed delay loop, I'm quickly getting sick > >> >> and tired of people complaining that their beloved bogomips value has > >> >> decreased. You know who you are! > >> > > >> >> Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it > >> >> will likely break fragile userspace code which is parsing that stuff, so > >> >> instead replace it with a dummy value that can be chosen in the > >> >> Kconfig. > >> > > >> > So, instead of removing it you silently report invalid value? Note > >> > >> Removing it would be an ABI breakage. Unfortunately. > > > > Did you really write this with a straight face? > > > >> Name one userspace application that extracts meaningful information out > >> of the BogoMIPS field. Just one. > > > > Waitaminute... Didn't you just claim this would be an ABI break? > > > > So if no application can be found, where is the ABI breakage? > > lscpu > > But that is already "broken" on ARM because x86 uses "bogomips" and > ARM uses "BogoMIPS". So basically it won't be any more broken if the field disappears entirely, right? > I agree reporting BogoMIPS is silly, but there > are more differences between x86 and ARM. Somewhat meaningful fields > like "cpu MHz" are missing on ARM for example. These are mainly useful > for h/w inventory use which server folks tend to care about. So... the tool must be coping well with missing fields already? If /proc/cpuinfo is missing "important" fields on ARM then this is a different issue. > > This thread must be a joke. Please just kill the damn thing out of > > /proc/cpuinfo and see if any application notices. And if some do then > > they are already broken *today* and should be fixed. > > And please do so for all arches then. Why? It happens that the bogomips reporting on ARM is even more "broken" than on other architectures and therefore it deserves to be removed even more than on other architectures. Who knows, maybe it still has some semblance of a meaning on those other architectures and that should be up to their respective maintainers to decide. Nicolas
Hi Nico, On Wed, May 15, 2013 at 12:56:51AM +0100, Nicolas Pitre wrote: > On Tue, 14 May 2013, Rob Herring wrote: > > On Tue, May 14, 2013 at 2:54 PM, Nicolas Pitre <nico@fluxnic.net> wrote: > > > Waitaminute... Didn't you just claim this would be an ABI break? > > > > > > So if no application can be found, where is the ABI breakage? > > > > lscpu > > > > But that is already "broken" on ARM because x86 uses "bogomips" and > > ARM uses "BogoMIPS". > > So basically it won't be any more broken if the field disappears > entirely, right? It's not these sort of tools I was worried about. I was thinking about the possibility of badly written parsers which want some other field from cpuinfo, but use the bogomips line for context. Do these applications exist? No idea. My second (less stupid) version of the patch prints "not reported". I'd be happy to remove the line altogether if people get behind the decision though. In fact, I just tried it and at least my linaro filesystem seems happy enough. Will
On Wed 2013-05-15 10:01:03, Will Deacon wrote: > Hi Nico, > > On Wed, May 15, 2013 at 12:56:51AM +0100, Nicolas Pitre wrote: > > On Tue, 14 May 2013, Rob Herring wrote: > > > On Tue, May 14, 2013 at 2:54 PM, Nicolas Pitre <nico@fluxnic.net> wrote: > > > > Waitaminute... Didn't you just claim this would be an ABI break? > > > > > > > > So if no application can be found, where is the ABI breakage? > > > > > > lscpu > > > > > > But that is already "broken" on ARM because x86 uses "bogomips" and > > > ARM uses "BogoMIPS". > > > > So basically it won't be any more broken if the field disappears > > entirely, right? > > It's not these sort of tools I was worried about. I was thinking about the > possibility of badly written parsers which want some other field from > cpuinfo, but use the bogomips line for context. Do these applications exist? > No idea. Yes, they do. bogomips is not nearly as bogus as you imply. See http://www.clifton.nl/index.html?bogomips.html https://play.google.com/store/apps/details?id=com.securiteinfo.android.bogomips&hl=en > My second (less stupid) version of the patch prints "not reported". I'd be > happy to remove the line altogether if people get behind the decision though. > In fact, I just tried it and at least my linaro filesystem seems happy > enough. Would it be too much work to simply put the field back with the right value? We don't have to get it as precise as we used to, that should make slowdown minimum. Pavel
On Wed, 15 May 2013, Pavel Machek wrote: > On Wed 2013-05-15 10:01:03, Will Deacon wrote: > > My second (less stupid) version of the patch prints "not reported". Even then, you should tell the truth and say "not available" or "not applicable". > > I'd be happy to remove the line altogether if people get behind the > > decision though. You certainly have my ACK. > > In fact, I just tried it and at least my linaro filesystem seems happy > > enough. > > Would it be too much work to simply put the field back with the right > value? We don't have to get it as precise as we used to, that should > make slowdown minimum. There is simply no right value. And whenever there is a value, people read too much into it. Nicolas
Hi! > > > In fact, I just tried it and at least my linaro filesystem seems happy > > > enough. > > > > Would it be too much work to simply put the field back with the right > > value? We don't have to get it as precise as we used to, that should > > make slowdown minimum. > > There is simply no right value. And whenever there is a value, people > read too much into it. You are right that there is no right value. But there are many _wrong_ values. Take a look at CPU temperature. There's no right value there, either, as CPU has different temperatures at different places. Displaying heatsink temp +5C is probably ok. Displaying case temperature or compile time constant is not. Now, bogoMIPS is supposed to be instructions per second. Measure instructions per second, and you have acceptable value to display. Put back code from previous version, and you have acceptable value to display. "0", "1" or value from Kconfig is _not_ acceptable value. Maybe bogomips should not have been exposed in /proc in the first place, but it is too late now. "No regressions". Pavel
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 9b31f43..5a0fce1 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -633,4 +633,52 @@ config PID_IN_CONTEXTIDR additional instructions during context switch. Say Y here only if you are planning to use hardware trace tools with this kernel. +choice + prompt "BogoMIPs setting" + default BOGOMIPS_MEDIUM + help + The BogoMIPs value reported by Linux is exactly what it sounds + like: totally bogus. It is used to calibrate the delay loop, + which may be backed by a timer clocked completely independently + of the CPU. + + Unfortunately, that doesn't stop marketing types (and even people + who should know better) from using the number to compare machines + and then screaming if it's less than some fictitious, expected + value. + + So, this option can be used to avoid the inevitable amount of + pain and suffering you will endure when the chaps described above + start parsing /proc/cpuinfo. + + config BOGOMIPS_SLOW + bool "Slow (older machines)" + help + If you're comparing a faster machine with a slower machine, + then you might want this option selected on one of them. + + config BOGOMIPS_MEDIUM + bool "Medium (default)" + help + A BogoMIPS value for the masses. + + config BOGOMIPS_FAST + bool "Fast (marketing)" + help + Some people believe that software runs faster with this + setting so, if you're one of them, say Y here. + + config BOGOMIPS_RANDOM + bool "Random (increased Bogosity)" + help + Putting the Bogo back into BogoMIPs. +endchoice + +config BOGOMIPS + int + default 1 if BOGOMIPS_SLOW + default 10000 if BOGOMIPS_MEDIUM + default 99999 if BOGOMIPS_FAST + default 0 + endmenu diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c index ec04c16..7996a12 100644 --- a/arch/arm/kernel/setup.c +++ b/arch/arm/kernel/setup.c @@ -30,6 +30,7 @@ #include <linux/compiler.h> #include <linux/sort.h> #include <linux/debugfs.h> +#include <linux/random.h> #include <asm/unified.h> #include <asm/cp15.h> @@ -878,7 +879,7 @@ static const char *hwcap_str[] = { static int c_show(struct seq_file *m, void *v) { int i, j; - u32 cpuid; + u32 cpuid, bogomips; for_each_online_cpu(i) { /* @@ -891,15 +892,14 @@ static int c_show(struct seq_file *m, void *v) seq_printf(m, "model name\t: %s rev %d (%s)\n", cpu_name, cpuid & 15, elf_platform); -#if defined(CONFIG_SMP) - seq_printf(m, "BogoMIPS\t: %lu.%02lu\n", - per_cpu(cpu_data, i).loops_per_jiffy / (500000UL/HZ), - (per_cpu(cpu_data, i).loops_per_jiffy / (5000UL/HZ)) % 100); +#ifdef CONFIG_BOGOMIPS_RANDOM + get_random_bytes(&bogomips, sizeof(bogomips)); + bogomips &= USHRT_MAX; #else - seq_printf(m, "BogoMIPS\t: %lu.%02lu\n", - loops_per_jiffy / (500000/HZ), - (loops_per_jiffy / (5000/HZ)) % 100); + bogomips = CONFIG_BOGOMIPS; #endif + seq_printf(m, "BogoMIPS\t: %u.00\n", bogomips); + /* dump out the processor features */ seq_puts(m, "Features\t: "); diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 1f2cccc..8acff93 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -341,17 +341,8 @@ asmlinkage void __cpuinit secondary_start_kernel(void) void __init smp_cpus_done(unsigned int max_cpus) { - int cpu; - unsigned long bogosum = 0; - - for_each_online_cpu(cpu) - bogosum += per_cpu(cpu_data, cpu).loops_per_jiffy; - - printk(KERN_INFO "SMP: Total of %d processors activated " - "(%lu.%02lu BogoMIPS).\n", - num_online_cpus(), - bogosum / (500000/HZ), - (bogosum / (5000/HZ)) % 100); + printk(KERN_INFO "SMP: Total of %d processors activated.\n", + num_online_cpus()); hyp_mode_check(); }
Now that we support a timer-backed delay loop, I'm quickly getting sick and tired of people complaining that their beloved bogomips value has decreased. You know who you are! Unfortunately, we can't just remove the entry from /proc/cpuinfo, as it will likely break fragile userspace code which is parsing that stuff, so instead replace it with a dummy value that can be chosen in the Kconfig. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm/Kconfig.debug | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ arch/arm/kernel/setup.c | 16 ++++++++-------- arch/arm/kernel/smp.c | 13 ++----------- 3 files changed, 58 insertions(+), 19 deletions(-)