Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

9fc2105aeaaf56b0cf75296a84702d0f9e64437b breaks audio in python, and probably elsewhere, with message

FATAL: cannot locate cpu MHz in /proc/cpuinfo

I'm not the first one to hit it, see for example

https://theredblacktree.wordpress.com/2014/08/10/fatal-cannot-locate-cpu-mhz-in-proccpuinfo/
https://devtalk.nvidia.com/default/topic/765800/workaround-for-fatal-cannot-locate-cpu-mhz-in-proc-cpuinf/?offset=1

Reading original changelog, I have to say "Stop breaking working setups. You know who you are!".

index ff6760a..940779c 100644
@@ -1048,6 +1048,15 @@ 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);
+#else
+		seq_printf(m, "BogoMIPS\t: %lu.%02lu\n",
+			   loops_per_jiffy / (500000/HZ),
+			   (loops_per_jiffy / (5000/HZ)) % 100);
+#endif
 		/* dump out the processor features */
 		seq_puts(m, "Features\t: ");
 

index 5e6052e..86ef244 100644
@@ -387,6 +387,18 @@ asmlinkage void 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);
+
 	hyp_mode_check();
 }
 

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

NAK.

No setups actually relying on this completely fony bogomips value bearing no links to hardware reality coule have been qualified as "working".

The bogomips entry was removed from /proc/cpuinfo in 2013. We're now in 2015. You're apparently the first to suggest moving the kernel back to providing random values via /proc/cpuinfo. So this removal must not have inconvenienced that many people in the end.

Broken applications appear to have been fixed already as mentioned via those links you provided above. So if you want a working setup, you may stick with a kernel of the same vintage as your user space apps or update the later.

If that is still unacceptable to you for whatever reason, then the least wrong compromize should be:

	seq_printf(m, "BogoMIPS\t: 1.00\n");

That'D allow for those broken applications to run while making clear that the provided value is phony. I was about to suggest 0.00 but that could trigger a divide by zero error somewhere I suppose.

You broke python-pyaudio in Debian 7.7, which is pretty new version... I'm not saying something relies on the _exact_ value there, but the change is bad.

Take a look at links above. One broken system would be enough... people noticed but did not complain on lkml.

That is not how kernel development should work. See

https://lkml.org/lkml/2012/12/23/75

I don't know what 1.00 will cause, and neither do you, so what about simply reverting the bad patch?
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Because the patch wasn't "bad". It did solve a recurring support problem where people did actually complain on the list because the value was not what they would have liked. Removing this meaningless value did actually fix that support issue as no more complaints came through for the last 1.3 year, and is actually the only way for user space to be fixed too.

Nicolas

People complain on the list, so what? People complain about systemd, too. We ignore them.

Alternatively, just don't touch the bogomips computation. It is not that much of maintainance burden. You can probably also get away with replacing bogomips with actual cpu frequency.

Replacing it with 1 is asking for trouble.

See the links I quoted. Removing the value caused real problems. (And I still did not hear so much as "sorry".) Now you propose to put obviously wrong value in there, and claim it is not a problem... because it takes time before someone debugs breakage you want to cause.

https://www.google.cz/search?q=FATAL:+cannot+locate+cpu+MHz+in+/proc/cpuinfo&ie=utf-8&oe=utf-8&gws_rd=cr,ssl&ei=06OpVJWuIcPkUsOTgbAL

First 2 screens in google are full of poor folks debugging this breakage.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Are you kidding? By that measure, maybe we should ignore you, too?

It causes problems to those applications actually taking that value for granted when it is wrong. This wasn't removed for fun, but rather because it caused other kind of troubles by being there.

Where do you get that CPU frequency from? If it was that easy, that's what we would have done already.

What about 100 then? Or 1000? Choose your pick.

It wasted a lot of people's time before by simply being there and wrong before it was removed. It's only a matter of whose time you want to waste. Really.

Nicolas

Really. Shut up.

The whole "no regressions" thing is very much about the fact that we don't waste users time.

We want users to be able to upgrade their kernels, and feel safe in knowing that we as kernel developers did our best - including very much "spending our time" - to make sure that users don't have to waste time.

And if you aren't ok with "wasting time" on trying to give that kind of reassurances to users, then you shouldn't be working on the kernel.

I'm serious about this. You really *need* to understand that. Your job as a kernel developer is very much to support the users. Not try to make it easy for *you* at the cost of being nasty for *them*.

The kernel serves user space. That's what we do.

Linus

I was talking about users time all along.

Never mind. I'm sorry for the NAK and sorry for attempting to start a discussion to find a better replacement.

Nicolas

Nico,

I encourage you *not* to back down like this. Linus is right in so far as the regressions issue, but he is *totally* wrong to do the revert, which IMHO has been done out of nothing more than spite.

Either *with or without* the revert, the issue still remains, and needs to be addressed properly.

With the revert in place, we now have insanely small bogomips values reported via /proc/cpuinfo when hardware timers are used. That needs fixing.

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Too bad 9fc2105aeaaf56b0cf75296a84702d0f9e64437b's changelog did not mention that :-(.

I see reasonable values on Nokia N900. Do I need to change my config to see the problem, or should I try reproducing on socfpga board?

Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hi,

I believe Nokia boards fall into the "antique hardware" category, so for us using such HW the issue is not visible...

A.

Why does it need to be fixed?

It's clear that there are applications that are working OK with the existing value, and if you change it to fix it for some new applications, but it breaks for others, then have you considered defining a new interface (perhaps exported via sysfs) that exports a "sane" value and document that new applications shoud use the new interface.

Or if the answer is that no one should be using the bogomips field at all, then just document *that*, and then leave it be, so that existing applications don't break.

- Ted

Hi Ted,

I'm not sure it is that clear -- the reported regression was on a processor that doesn't use the timer-backed delay loop, so the bogomips value will essentially be restored by reverting the patch.

The issue comes on newer CPUs, where there will now be a very small bogomips value reported and (to my knowledge) nobody has yet tried running some affected applications there to see if they can cope.

It never hurts to document our assumptions or anticipated/preferred use-cases but in this case I think bogomips is difficult enough to use on any half-recent SoCs that most developers have either (a) found another way to do what they want (perf counters, clock_gettime) or (b) stopped bothering to guess the CPU frequency when it's not actually needed, so I don't *think* that new applications are such an issue.

Cheers,

Will

Here's my take on it. Taking a step back, it was stupid to mix bogomips with timer based delays.

----- >8
From: Nicolas Pitre <[email protected]>
Date: Sun, 4 Jan 2015 22:28:58 -0500
Subject: [PATCH] ARM: disentangle timer based delays and bogomips calibration

The bogomips value is a pseudo CPU speed value originally used to calibrate loop-based small delays. It is also exported to user space through the /proc filesystem and some user space apps started relying on it.

Modern hardware can vary their CPU clock at run time making the bogomips value less reliable for delay purposes. With the advent of high resolution timers, small delays migrated to timer polling loops instead. Strangely enough, the bogomips value calibration became timer based too, making it way more bogus than it already was as a CPU speed representation and people using it via /proc/cpuinfo started complaining.

Since it was wrong for user space to rely on a "bogus" mips value to start with, the initial responce from kernel people was to remove it. This broke user space even more as some applications then refused to run altogether. The bogomips export was therefore reinstated in commit 4bf9636c39 ("Revert 'ARM: 7830/1: delay: don't bother reporting bogomips in /proc/cpuinfo'").

Because the reported bogomips is orders of magnitude away from the traditionally expected value for a given CPU when timer based delays are in use, and because lumping bogomips and timer based delay loops is rather senseless anyway, let's calibrate bogomips using a CPU loop all the time even when timer based delays are available. Timer based delays don't need any calibration and /proc/cpuinfo will provide somewhat sensible values again.

In practice, calls to __delay() will now always use the CPU based loop. Things remain unchanged for udelay() and its derivatives.

index dff714d886..7feeb163f5 100644
@@ -21,13 +21,12 @@ struct delay_timer {
 };
 
 extern struct arm_delay_ops {
-	void (*delay)(unsigned long);
 	void (*const_udelay)(unsigned long);
 	void (*udelay)(unsigned long);
 	unsigned long ticks_per_jiffy;
 } arm_delay_ops;
 
-#define __delay(n)		arm_delay_ops.delay(n)
+#define __delay(n)		__loop_delay(n)
 
 /*
  * This function intentionally does not exist; if you see references to

@@ -63,7 +62,6 @@ extern void __loop_udelay(unsigned long usecs);
 extern void __loop_const_udelay(unsigned long);
 
 /* Delay-loop timer registration. */
-#define ARCH_HAS_READ_CURRENT_TIMER
 extern void register_current_timer_delay(const struct delay_timer *timer);
 
 #endif /* __ASSEMBLY__ */

index 312d43eb68..d958886874 100644
@@ -30,7 +30,6 @@
  * Default to the loop-based delay implementation.
  */
 struct arm_delay_ops arm_delay_ops = {
-	.delay		= __loop_delay,
 	.const_udelay	= __loop_const_udelay,
 	.udelay		= __loop_udelay,
 };

@@ -86,12 +85,8 @@ void __init register_current_timer_delay(const struct delay_timer *timer)
 	if (!delay_calibrated && (!delay_res || (res < delay_res))) {
 		pr_info("Switching to timer-based delay loop, resolution %lluns\n", res);
 		delay_timer			= timer;
-		lpj_fine			= timer->freq / HZ;
 		delay_res			= res;
-
-		/* cpufreq may scale loops_per_jiffy, so keep a private copy */
-		arm_delay_ops.ticks_per_jiffy	= lpj_fine;
-		arm_delay_ops.delay		= __timer_delay;
+		arm_delay_ops.ticks_per_jiffy	= timer->freq / HZ;
 		arm_delay_ops.const_udelay	= __timer_const_udelay;
 		arm_delay_ops.udelay		= __timer_udelay;
 	} else {

@@ -102,7 +97,9 @@ void __init register_current_timer_delay(const struct delay_timer *timer)
 unsigned long calibrate_delay_is_known(void)
 {
 	delay_calibrated = true;
-	return lpj_fine;
+
+	/* calibrate bogomips even when timer based delays are used */
+	return 0;
 }
 
 void calibration_delay_done(void)

Hi all,

Sorry for the late reply, it seems that neither myself or the arm-linux-kernel list were on CC for this thread.

Well, bogomips is directly related to loops_per_jiffy so I don't think the mechanism is "stupid"; the issue is that userspace makes assumptions (bogus or otherwise) about the relation of bogomips to cpu frequency which have historically been good enough. More below...

As you pointed out previously, these complaints were what prompted us to revisit the bogomips reporting. The class of application using the value was very much things like "How fast is my AwesomePhone-9000?":

https://play.google.com/store/apps/details?id=com.securiteinfo.android.bogomips&hl=en_GB
https://bugs.launchpad.net/ubuntu/+source/byobu/+bug/675442

so actually, having *these* applications either exit early with an "unable to calculate CPU frequency" message or print something like "CPU
freq: unknown" is arguably the right thing to do. What Pavel is now reporting is different; some useful piece of software has lost core functionality.

Now, even with the loop-based bogomips values we have the following (user-visible) problems:

(1) It's not portable between microarchitectures (for example, some CPUs can give you double the value if they predict the backwards branch in the calibration loop)

(2) It's not reliable in the face of frequency scaling

(3) It's not reliable in the face of heterogeneous systems (big.LITTLE)

(4) The lpj calculation is susceptible to overflow, limiting the maximum delay value depending on the CPU performance

Essentially, the value is only really useful on relatively low-clocked, in-order, uniprocessor systems (like the one where Pavel reported the bug).

Actually, our initial response was to report a dummy value iirc. I remember even making it selectable in kconfig, but it bordered on the absurd. It's worth noting that, with the current revert in place, the value reported is now basically selectable via the "clock-frequency" property on the arch_timer node for systems using the timer implementation.

Given that we have a hard limit of 3355 bogomips in our calibration code, could we not just report that instead? We already have all of the issues I highlighted above and the systems that are going to be hit by this are the more recent (more performant) cores that will be approaching this maximum value anyway.

We also need something we can port to the arm64 compat layer, so a constant would be easier there too, doesn't require the calibration delay at boot and doesn't break __delay.

One thing we're missing from all of this is what happens if Pavel's testcase is executed on a system using a timer-backed delay? If the program chokes on the next line anyway, then we could consider only advertising the bogomips line when the loop-based delay is in use.

Pavel: do you have something we can run to observe the problem?

Finally, the revert doesn't have a Cc stable tag which is probably needed if it's going to land in 3.19 final, irrespective of whether we agree that it's the right way forward.

Will

Sorry about that. I was pretty sure I cc-ed you, but apparently did not.

Debian 7.7: mpg123, anything pyaudio based, at least. (install python-pyaudio, try to run examples).

I hit it with:

#!/usr/bin/env python
# Written by Yu-Jie Lin
# Public Domain
#
# Deps: PyAudio, NumPy, and Matplotlib
# Blog:
# http://blog.yjl.im/2012/11/frequency-spectrum-of-sound-using.html

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.animation as animation
import pyaudio
import struct
import wave
...

When I decided to investigate.

Yes, pushing is to stable is good idea. Will you take care?

Thanks,
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

It is stupid to use loops_per_jiffy for timer based delay loops. The timer based loop simply polls the timer until the desired time has passed. Adding a loop count on top is completely artificial (may be justified to avoid timer wraparounds) but bares no relationship with loops_per_jiffy. Therefore determining loops_per_jiffy based on a timer frequency is wrong.

Absolutely. And that's what my patch is all about: restoring that "good enough" for user space (mis)purpose.

Don't you dare! Linus will shut you up. The sacred rule: "We don't break user space, period" irrespective of the nefarious application purpose for bogomips.

Who cares?

loops_per_jiffy is already scaled accordingly. Sure it is racy but that's what non timer based delay loop using platforms have to live with already. For /proc/cpuinfo purposes that ought to be more than good enough. The MHz on X86 that some applications use in place of the bogomips when available has the same issue.

Actually, it is. With my patch I do get different values in /proc/cpuinfo for the A15's and the A7's which is kind of expected.

That's an orthogonal issue that can be fixed separately.

Sure. Still on other systems it is some kind of ballpark figure that prevents applications from breaking.

Which is even more absurd, hence my patch.

I suggested 1.00 before in this thread. I also asked if 10, 100 or 1000 were any better. Apparently none of them were. The least controvertial value is certainly a runtime determined one. The hard limit is a rather weak excuse that can be fixed.

That's a weak excuse too.

Won't fix the current user space issue on timer-based-delay systems so this brings no good.

Nicolas

I already replied to the other issues in my previous email.

Now here's the bogomips hard limit gone:

----- >8
From: Nicolas Pitre <[email protected]>
Date: Mon, 5 Jan 2015 22:43:56 -0500
Subject: [PATCH] ARM: loop_udelay: remove bogomips value limitation

Now that we don't support ARMv3 anymore, the loop based delay code can convert microsecs into number of loops using a 64-bit multiplication. This allows us to lift the hard limit of 3355 on the bogomips value with loops_per_jiffy that can safely span the full 32-bit range.

index 7feeb163f5..287c96ad93 100644
@@ -10,8 +10,8 @@
 #include <asm/param.h>	/* HZ */
 
 #define MAX_UDELAY_MS	2
-#define UDELAY_MULT	((UL(2199023) * HZ) >> 11)
-#define UDELAY_SHIFT	30
+#define UDELAY_MULT	UL(2047 * HZ + 483648 * HZ / 1000000)
+#define UDELAY_SHIFT	31
 
 #ifndef __ASSEMBLY__
 

@@ -33,7 +33,7 @@ extern struct arm_delay_ops {
  * it, it means that you're calling udelay() with an out of range value.
  *
  * With currently imposed limits, this means that we support a max delay
- * of 2000us. Further limits: HZ<=1000 and bogomips<=3355
+ * of 2000us. Further limits: HZ<=1000
  */
 extern void __bad_udelay(void);
 

index 518bf6e93f..bcc23c5760 100644
@@ -17,7 +17,6 @@
 
 /*
  * r0  <= 2000
- * lpj <= 0x01ffffff (max. 3355 bogomips)
  * HZ  <= 1000
  */
 

@@ -25,16 +24,11 @@ ENTRY(__loop_udelay)
 		ldr	r2, .LC1
 		mul	r0, r2, r0
 ENTRY(__loop_const_udelay)			@ 0 <= r0 <= 0x7fffff06
-		mov	r1, #-1
 		ldr	r2, .LC0
-		ldr	r2, [r2]		@ max = 0x01ffffff
-		add	r0, r0, r1, lsr #32-14
-		mov	r0, r0, lsr #14		@ max = 0x0001ffff
-		add	r2, r2, r1, lsr #32-10
-		mov	r2, r2, lsr #10		@ max = 0x00007fff
-		mul	r0, r2, r0		@ max = 2^32-1
-		add	r0, r0, r1, lsr #32-6
-		movs	r0, r0, lsr #6
+		ldr	r2, [r2]
+		umull	r1, r0, r2, r0
+		adds	r1, r1, #0xffffffff
+		adcs	r0, r0, r0
 		reteq	lr
 
 /*

I think we still build RPC with gcc -march=armv3. Is that a problem with this patch?

Arnd

It is.

Tangential question: does anyone still own a working RPC?

For sure we no longer support the RPC unless it is fitted with a SA110. And IIRC the reason why -march=armv3 is used in that case has to do with the RPC memory bus not able to accommodate the SA110's LDRH/STRH instructions. However it should be able to execute UMULL regardless.

Now I could add ".arch armv7-a" in the file to make it compile for RPC. I can't just make it ".arch armv4" as this prevents Thumb2 compilation for that file.

Other ideas?

Nicolas

I settled on the following:

index 0573faab96..0eb8de1c6f 100644
@@ -40,7 +40,10 @@ else
   lib-y	+= io-readsw-armv4.o io-writesw-armv4.o
 endif
 
-lib-$(CONFIG_ARCH_RPC)		+= ecard.o io-acorn.o floppydma.o
+ifeq ($(CONFIG_ARCH_RPC),y)
+  lib-y				+= ecard.o io-acorn.o floppydma.o
+  AFLAGS_delay-loop.o		+= -march=armv4
+endif
 
 $(obj)/csumpartialcopy.o:	$(obj)/csumpartialcopygeneric.S
 $(obj)/csumpartialcopyuser.o:	$(obj)/csumpartialcopygeneric.S

That'll make things easier to sort out in the context of $tangential_question.

Nicolas

I'm pretty sure that Russell still has one. As far as I know there are quite a lot of RPC in working condition, but most of them are running RiscOS.

I think this makes sense since __delay() expects the number of loops as argument rather than a duration scaled by some factor (based on the generic timer frequency).

FWIW:

Minor comment below:

Catalin

Gaah. You guys make no sense at all.

No, __delay() does not expect "number of loops".

It doesn't do that on x86, and it doesn't even do it on arm.

It *literally* does exactly the reverse of what you say it does.

__delay() very much expects a "duration scaled by some factor". The factor being "loops_per_jiffy" (ok, so it's a "reverse factor", but you get the idea).

And this is very much exactly why bogomips is that "2 * loops_per_jiffy * HZ / 1000000".

Let's break it down:

- "loops_per_jiffy" is just the scale factor for __udelay(). Ignore
the "loops" part of the name, since it's purely historical. It comes from the original "decrement and jump" implementation, but that hasn't been true on x86 for over 15 years.

- the "2x" factor is also purely historical, and comes from the same
"decrement and jump" thing - it used to be two instructions. But again, it hasn't been true for a *looong* time, but it is part of the bogosity of bogomips.

- the "*HZ" is the "per jiffy" part.

- the "1000000" is the "MHz" part. The clock had better tick at a
megahertz or more, since the whole point of this is to do "udelay()".

Really. The original commit that removed bogomips from ARM was *wrong*. Because that

    per_cpu(cpu_data, i).loops_per_jiffy / (500000UL/HZ

calculation is in fact the very DEFINITION of bogomips (it's just written in a way to not overflow).

Seriously.

The reason I reverted that commit is because it was crap. When Pavel made the report, I looked at the code, and immediately reverted it. For good reason. Exactly because that original patch was WRONG.

The old bogomips computation in arch/arm/kernel/setup.c is exactly the right thing to do.

And every time somebody thinks it has to be about "loops" or about "cpu frequency", I can only say "STOP IT". It hasn't been about loops or CPU frequency for ages. I don't know when we switched it to "wait for the timer", because it predates my history that starts in early 2002. So it's at *least* 13 years old.

Linus

I think we're all saying more or less the same thing.

The bogomips *reporting* is back on ARM so user space won't break.

We'll make sure it is scaled properly so not to have orders of magnitude discrepancy whether the timer based or the CPU based loop is used for the purpose of making people feel good.

Any disagreement?

Nicolas

Why?

You'd basically be lying. And it might actually hide real problems. If the scaling hides the fact that the timer source cannot do a good job at microsecond resolution delays, then it's not just lying, it's lying in ways that hide real issues. So why should that kind of behavior be encouraged? The actual *real* unscaled resolution of the timer is valid and real information.

Random scaling like that would be *bad*, in other words.

This whole thread has wasted more time than the whole original argument for wasted time ever was. I still have no idea what the original argument was, and why you guys want some made-up and incorrect feel-good number rather than just document the fact that the bogomips is simple dependent on the clocksource you use for delays.

That kind of documentation wouldn't be lying, wouldn't be misleading, and wouldn't waste peoples time.

Linus

I think you are missing something fundamental in this thread.

On ARM, when the timer is used to provide small delays, it is typically the ARM architected timer which by definition must have a constant input clock in the MHz range. This timer clock has *nothing* to do with whatever CPU clock you might be using. On the system I have here, the CPU clock is 2GHz and the timer used for delays is 24MHz. If the CPU clock is scaled down to 180MHz the timer clock remains at 24MHz.

The implementation of udelay() in this case is basically doing:

void udelay(unsigned long usecs)

{
	unsigned long timer_ticks = usecs * (24000000 / 1000000);
	unsigned long start = read_timer_count();
	while (read_timer_count() - start < timer_ticks);
}

Some other systems might as well have a completely different timer clock based on what its hardware designers thought was best, or based on what they smoked the night before. There is no calibrating of the timer input clock: it is just set and the timer clock rate for a given hardware implementation is advertised by the firmware. No calibration is necessary. No calibration would even be possible if that's the only time source on the system.

Now tell me what this means for bogomips? Nothing. Absolutely nothing. Deriving a bogomips number from a 24MHz timer clock that bares no relationship with the CPU clock is completely useless. We might as well hardcode a constant 1.00 and be done with it. Or hash the machine name and add the RTC time and report that. At least the later would have some entertainment value.

What I'm suggesting is to leave the timer based udelay() alone as it doesn't need any loops_per_jiffy or whatnot to operate. Then, for the semi-entertaining value of having *something* displayed alongside "bogomips" in /proc/cpuinfo I'm suggesting to simply calibrate loops_per_jiffy the traditional way in all cases, whether or not a timer based udelay is in use.

Why you might have having a problem with that is beyond my understanding.

Nicolas

I think what Linus is trying to tell us is that:

1. Where the kernel uses a software loop for implementing delays, the kernel bogomips gives us a calibration of that loop.

2. Where the kernel uses a hardware timer for implementing delays, the kernel bogomips gives us a calibration of that hardware timer.

And it doesn't matter whether or not that timer has anything to do with the raw CPU speed.

In other words, bogomips is a statement about the accuracy of the internal kernel mechanism being used for delays, nothing more, nothing less.

Now, if I understand Linus correctly, what irks him is when someone upgrades a kernel on a platform, and some userland breaks. That's something which I've said multiple times I don't have a problem agreeing with, and I suspect no one in this thread would disagree that this is a serious failing, and one which needs fixing ASAP.

However, if running userland on platform A works, and but it doesn't work on platform B. The breakage may well be due to platform A reporting 300 bogomips because it's using the kernel software loop, and platform B reporting 6 bogomips because its using a hardware timer, but the CPU is actually faster. However, this is not a kernel problem, and it certainly is not a regression. It's a userspace bug which needs userspace to fix.

Does that make the difference clear?

FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

I'm not clear if that's actually what Linus is trying to tell us.

But that statement makes no sense at all. Why would user space care about kernel internal mechanism for small delays? This hardly has any influence on user space and I just can't imagine any user space code consuming the bogomips value for that purpose.

What user space did with bogomips, though, is to get some relative measure of the CPU speed for whatever calibration purposes, or simply for pretty printing.

I agree too. And that is fixed in mainline with commit 4bf9636c39.

There I disagree. In the spirit of "the kernel shall never break user space ever" I'd say that the kernel is simply doing a poor job at providing user space with a value that won't break user space expectations. And since it is not that hard to do (I made a patch already) I'd say we have less to lose by fixing it than keeping a totally senseless value around.

Nicolas

It's not that the kernel shall never break userspace. It must not cause userspace regressions.

If application A worked on box X and they upgrade the kernel and then application A no longer works. That's a regression, and must be fixed.

Now if I understand what Russell stated, if application A works on box X and you move to box Y using the same kernel, and application A no longer works, that's not a regression with the kernel (unless it use to work on box Y). If it never worked on box Y, it's a platform issue and application A is not robust enough to deal with it. AKA, not a kernel bug.

Now, it gets interesting if a fix was made that lets application A work on box Y, but that fix broked application B on box X. Reverting that fix will cause a regression for A or Y, but the fix itself caused a regression for B on X. In that case we need a new fix (which may be the case we are here).

-- Steve

Fine.

Fine as well.

Indeed.

And for whatever reason, some user space program thinks it can read something meaningful out of this number and use it in user space. But the consensus is that even if user space is badly implemented, we do *not* break it. I agree.

Agree. But I assume you refer to the fact that we removed the BogoMIPS reporting. It's fine to have it reverted.

We need to look back at the point we added timer-based delay about 2.5 years ago. Prior to commit d0a533b18235d362, platform A reported bogomips 300. After that commit, the *same* platform A (not B), started reported 6.

Is the above considered user breakage? That's what Nico is trying to solve. If we are fine with it, than we can close this thread, no further changes needed.

We can document it as Linus suggests and say that prior to whatever version we had 2.5 years ago, BogoMIPS was based on a busy loop. In more recent kernels, it is based on a timer delay. User space should make use of such information when interpreting BogoMIPS.

Catalin

Things change. The only thing that is considered "user breakage" is when something actually doesn't work any more.

That has always been the rule. It's not that the kernel ABI (with all the system calls, all the /proc files, all the ioctl's, etc) is set in stone and "sacred". Absolutely anything can be changed, wildly.

But if it turns out that applications (or hardware) that people use end up breaking noticeably, then *that* is a regression.

And the important part there are those weasel-words: "that people use" and "noticeably".

For example, a test-suite giving a different result is *not* a regression, although it should obviously be considered a big red flag. So if somebody tells you that some test-suite shows that some ABI changed, at the very least you should be very nervous about things.

But if that same test-suite result is then used in a production environment as part of some actual user flow, and it breaks that user model, then it suddenly becomes a regression. So the very definition of "regression" is not really about the API changing, but about breaking peoples existing setups. Of course, if you never change any API that is visible to user space, you can never create that kind of regression, so they are _related_, but some people confuse the two. They are still very different.

Similarly, theoretical arguments of "so-and-so wouldn't work after this change" are just that - theoretical arguments. It's something to worry about, but it's not an actual *regression* until it causes problems.

For an extreme example of this: we can remove support for whole platforms and architectures, and sometimes we do. It clearly completely breaks support for the hardware in question - but it only counts as a regression if anybody notices and cares. There may still be active users of that platform that provably cannot possibly work at all any more, but if they never upgrade the kernel, then it's still not a regression.

In this case, pretty much all of /proc/cpuinfo is mainly "informational". Maybe there are applications that show it, but more likely you have people who ssh in and just do

cat /proc/cpuinfo

to see what kind of system they are running on. That's the main point of much of /proc, and things like /proc/cpuinfo in particular.

Now *main* point doesn't necessarily mean "only point". There clearly are binaries parsing it. Some do it to figure out how many CPU's the system has, often simply because using /proc is simple from various scripting environments, for example. So while most of /proc/cpuinfo is clearly for human consumption, it's also understandable that some parts of it might matter for people.

And quite frankly, I personally think that any program that parses /proc/cpuinfo in order to find the bogomips value and use it for anything is just clearly insane. Why would you ever do that? It makes no sense. It's crazy. Apparently the crazy audio library didn't even do it in a meaningful way, and the use of that value seems to be pretty much random, and the actual value likely doesn't really even *matter*.

But the rule for "regression" has never been about sanity, or about whether the ABI changes. There are tons of horribly insane user programs. Parsing /proc to find bogomips may be insane and odd, but it's certainly not the worst kind of diseased code I've ever heard about. We have had major programs that literally depended on totally insane small details that were never intentional, and just happened to have some particular implementation detail. And then the implementation changed, and the interface ostensibly did exactly the same thing, but because it did it with some meaningless difference that couldn't *possibly* matter in any sane situation, it caused a regression.

So the kernel regression rules are very strict in that it's the absolute #1 rule in kernel development, but at the same time, they are about as lax as they can possibly be: an interface change is only a regression if somebody notices.

Changing the bogomips value - even radically - or removing it entirely isn't a regression in itself.

And in this case, I do suspect that the *actual* value really almost doesn't matter. It looks more like some internal badly done hint for some buffer size or other. It is possible that wild fluctuations could be noticeable, but it's fairly unlikely.

The other "good news" in this area is that I suspect that the random ARM platforms that actually changed 2.5 years ago are not very widely used any more. So not only does the actual real value probably not matter much to begin with, but the platforms where it really changed are probably not a major issue.

Linus

Would you mind if on ARM we used the bogomips line as an informative approximation for the CPU speed? After all, this is the meaning it had for nearly 20 years. And unlike on X86 we don't have the actual CPU clock in there. And that's still the meaning it has on most ARM systems out there.

In that case nobody would complain if on some systems we make it back to the traditional value. It took over a year before problem report about its disappearance reached us.

On other systems it will remain the same because its meaning still hasn't changed. But on the whole it'll have a coherent meaning.

I beg to differ. My primary test platform is one of them and it has the latest incarnation of the ARM 32-bit CPU architecture.

Rather, the good news is that not that many user space things rely on the bogomips value. Or maybe people don't upgrade their kernels that often e.g. the latest OpenWRT stable version from 3 months ago ships with a 3.10 kernel. Oh and all the Ubuntu releases for ARM in the last year shipped with kernels that don't advertise the bogomips and passed QA. So if we bring it back now, better make it usefully informative at least for humans.

Nicolas

Yes, I actually would mind, unless you have a damn good reason for it.

On x86, we have bogomips, but we also have this line:

cpu MHz : 2275.109

and I really don't see why you should lie in your /proc/cpuinfo.

Really.

Just give the real information. Don't lie.

Quite frankly, the *only* actual real reason I've heard from you for not having the real bogomips there is "waste of time".

And this whole thread has been *nothing* but waste of time. But it has been *you* wasting time, and that original commit. If you had just left it alone, and had just let the revert do its job, none of this waste of time would have happened.

So quite frankly, my patience for you arguing "wasting time" is pretty damn low. I think your arguments are crap, I still think your NAK was *way* out of line, and I think it's completely *insane* to lie about bogomips. It's disasteful, it's dishonest, and there's no reason for it.

If you want to give people a sense of the CPU frequency, then dammit, just *do* that. Add that "cpu MHz" line that we already have on x86.

Seriously, what kind of *insane* argument can you really marshal for lying to users?

And if you want users to know the CPU frequency, then why the f*ck would you call it "bogomips" and confuse them?

Christ, this whole thing is annoying. I really find it *offensive* how you want to basically lie to users.

Stop this idiocy. Really. There is no excuse.

Linus

Consistency.

On ARM we just can't find the CPU clock in a generic way. Yes, this is part of the ARM hardware environment where every implementer can do whatever they want with things that are not architected. That's not something we kernel developers can do anything about.

What we can do in a generic way, though, is counting the number of loops the CPU can perform during a jiffy.

You keep harping on with that statement. Could you just tell me _what_ we would be lying about and to _whom_? It's probably the third time I'm asking this with no rational answer so far.

What I'm telling you is that a kernel on a machine that used to show 800 bogomips and suddenly start showing 6 bogomips is lying. But for some contrived reasons that's fine with you.

Quite the reverse. In fact this is getting hilarious. Either you keep on understanding all I say only half way, or you purposely twist my words. What kind of game are you playing?

What I said is that:

1) The user space bogomips reporting on ARM, for the best part of the last 20 years, was based on the number of loops the CPU can perform during a jiffy. Given its history that's what I'd call the real bogomips.

2) Because of some relatively recent internal kernel changes, the bogomips reporting became totally useless for its consumers as it was orders of magnitude smaller than it used to be. Like moving from 800 to 6 on the same hardware. Those consumers started complaining.

3) We told those consumers: bogomips is evil, stop wasting everyone's time and don't use it, because a value of 8 is no longer the real bogomips. It was unreliable before, now it is meaningless. Go consume another metric instead.

4) Complaints still came by, so we decided to solve the issue by simply removing the meaningless bogomips reporting altogether. This worked wonderfully for more than a year, at least from our perspective.

5) The lack of a bogomips reporting broke some user space applications. This is an unacceptable regression and the meaningless bogomips was reinstated as in (3).

6) Now I'm claiming that if we're going to need the bogomips reporting not to break user space, we should at least go back to the real bogomips as it has been for the best part of the last 20 years i.e like in (1), not the meaningless one.

For (6) I have the patch, it is non intrusive, and doesn't touch generic code at all. The diffstat for my latest version is:

 2 files changed, 2 insertions(+), 15 deletions(-)

So, instead of telling me _why_ the above is not the sanest thing to do, you come up with diatribes like:

You are just full of b/s. I'm bringing rational arguments to this discussion, and when you can't debunk them you have nothing better than accusing me of wasting time with a stream of emotions. Sorry but I'm holding you up to better standards.

Sure my NAK on the revert was premature. I was envisioning a revert that would also include (6) above. But the issue is no longer about the revert. We can do (6) separately.

Nicolas

Fuck no.

"Completely made up number that you cannot explain" is not consistency.

The *current* situation is consistency. I can - and have in this very thread - explained what bogomips is in just a sentence or two.

You are just making shit up. Bad shit. Get off the drugs, because it's not the good kind.

Cry me a river.

So you want to make bogomips a totally random number, that has no meaning, no correlation to any clocksource, and no correlation to cpu frequency either?

And you argue for this though exactly WHAT? "Consistency".

Bullshit.

This whole thread is now marked as "muted" for me, because I can't take the BS any more. You make no sense.

You guys playing games with bogomips was what broke things in the first place, and now you want to play *more* games?

You're crazy. Go away. Or don't. I won't be seeing your emails anyway, so why would I care?

Linus

Again this statement. I'm not against it as this is a true statement.

I really wonder how you can describe my intent with that statement though. We must be living in different universes. This is so wrong as not to be funny anymore.

If you don't want to see my reply that's fine. It'll be there for the posterity at least. If That's because you're just too proud to concede I'm right then this is very sad.

All I'm trying to tell you is that 6 *is* a "Completely made up number that you cannot explain" for user space and that's what we have right now in mainline for some ARM machines.

What I'm advocating for is a number that is _not_ completely made up. I'm advocating for a bogomips which meaning is well known and dates back to early Linux releases: the number of loops performed by the CPU during a jiffy, scaled to a second worth of jiffies, divided by 2 because there are 2 instructions in that loop. Incidentally this very definition is the one you provided yourself in this thread. That's what I want, yet you qualify this as a "Completely made up number that you cannot explain".

Yeah right. I challenge you to quote anything I said in my previous email where I clearly explained everything in 6 points what I want. Anything you may quote to substantiate that statement of yours about what I want. Everyone following this thread knows you can't.

But you probably didn't even read it. It might hurt too much.

Sheesh.

Nicolas

Correct.

It's not a regression - yet. No one has shown that userspace has broken according to the definition of the first quote above, and that's the whole point.

We don't need to document it - we just need to point people at this URL:

http://en.wikipedia.org/wiki/BogoMips

which already describes it fully, including the existing ARM timer behaviour.

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

How much does one have to regress before it is a problem? I have two projects I've worked on that "broke" due to this issue.

They were minor breakages though.

The "linux_logo" userspace sysinfo tool broke to the extent that it was parsing for the bogomips string in /proc/cpuinfo and printed poorly formatted and/or corrupted text info to screen when it couldn't find it.

The "PAPI" library had some really ancient (and poorly thought-out) fallback code that would try to estimate MHz from bogomips if a MHz value was not available via the traditional methods. This failed after the change too, but not many people use PAPI on ARM so it wasn't that big an issue.

I noticed these problems early, even before the change hit mainline. But when I complained I was told in no uncertain terms that the ARM maintainers were tired of hearing about bogomips issues and nothing could be done to stop the change from getting in.

Vince

With the revert in place, now you get the bogomips value. Just don't assume anything about it.

We now have the bogomips string back, so this problem is solved.

There were many complaints, from marketing people to some Linux users who had a "feeling" that their platform just got much slower after the delay loop change. Since this patch didn't gain much traction:

https://lkml.org/lkml/2013/5/3/405

we decided to remove it completely so that we stop complaints that bogomips does not match the CPU frequency. Unfortunately, we broke the ABI.

Now the bogomips is back, but we are going to ignore anyone asking about its value as it can be either the speed of a busy delay loop or the generic timer frequency (completely independent; which one depends on kernel version and SoC).

I really think we should stop this thread. User ABI breakage fixed now.

Catalin

No. Vince's email shows a more serious problem.

The ABI breakage issue was reported before the patches were apparently merged, but that information seems to have been lost. We need to understand how that happened so similar instances don't happen in the future.

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

FYI, searching for "Vince Weaver" and "bogomips" found this:

https://lkml.org/lkml/2013/7/11/454

Catalin

Gah.

It looks like the report was made by hanging it into a totally different thread to the patches which caused the problem, and which went nowhere near the ARM kernel mailing lists.

At least that explains why people like me who really need to know this stuff were not aware of the issue.

One thing I still can't get my head around though is... Vince reported the breakage on 11 July 2013, but the patch is dated 30 August and I merged it 2 September. So, how did Vince know about it to report the ABI breakage? Had it been sitting somewhere else?

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Well, the patch may have been on the list, acked, before July but it only went to the patch system on the 30th of August. Vince probably concluded that it will go in once acked on the list.

Catalin

Yes, it looks like it went out on the linux-arm-kernel list on 3 May 2013.
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/166728.html

I don't follow that list though, so someone must have pointed me to the posting.

Though there must have been something else too, I definitely recall reading a long rant about how annoying it was that everyone was reporting bogomips value errors and this was the best way to handle things. All of the postings of the patch I can find are much more laid back.

Mostly why I didn't push the issue though was all the pushback I've gotten over the years for trying to get perf_event ABI breaks reverted (I've never successfully managed to get that to happen).

Vince

yes, though by that point I had thought it was a done deal and so was mostly complaining about it with regard to another issue.

I usually don't hang out on the ARM lists, but perf-event related ones, which was why I was primarily discussing the issue with Will Deacon in that case.

That's a good question. Someone definitely had told me that is was going to happen earlier, but I can't remember where now. Definitely someone with enough authority that I thought it wasn't worth the trouble complaining (I already seemingly have half the perf_event people annoyed with me on any given day, the last thing I wanted to do was get all of the ARM devs mad at me too).

Vince

This went at least to Will Deacon and Nicolas Pitre.

https://lkml.org/lkml/2013/5/15/217

So AFAICT yes, we knowingly broke at least
com.securiteinfo.android.bogomips application.

There was discussion on lkml at May 2013, clearly marked "ARM:". I don't see enough headers to know what else was copied.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hmm, "BogoMIPS is a tool used to create a web page about Android Benchmarks". And a stupid table here:
https://www.securiteinfo.com/divers/android_bogomips_benchmark.shtml.
Even though it's against the user ABI kernel policy, I must say I'm glad we broke this one ;).

pyaudio is a valid complaint though (and not about the bogomips value but about the presence of the "bogomips" string).

Catalin

If it's something which worked before and fails after, that's enough.

Sorry about that.

However, I'm having a hard time finding your report. Do you have a message id for the report?

For the record, I have no knowledge of this report having been made.

My linux-arm-kernel web archives bring up nothing either.

My mailbox record going back to the start of 2013 (which includes all linux-arm-kernel mailing list traffic and any traffic to _this_ address, and covers six months before the commit happened), I only have messages with these subject lines authored by you:

2013:
Subject: Re: [Ksummit-2013-discuss] [ARM ATTEND] catching up on exploit mitigations
Subject: Re: NSA310 + DT
2014:
Subject: Re: CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and bcm2835_defconfig
Subject: Re: CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and bcm2835_defconfig
Subject: Re: CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and bcm2835_defconfig
Subject: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: Re: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: Re: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: Re: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: Re: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: Re: [bisected] broken make-kpkg kernel build in 3.15-rc1
Subject: [tip:perf/core] perf/ARM: Use common PMU interrupt disabled code

For the thread where the offending patch was posted in 2013, my mailbox gives me:

4963     Jun 20 Will Deacon     (  36) [PATCH v2 0/2] ARM: Remove any correlatio
4964     Jun 20 Will Deacon     (  67) ├─>[PATCH v2 1/2] ARM: delay: don't bothe
4965     Jun 20 Will Deacon     (  50) ├─>[PATCH v2 2/2] init: calibrate: don't
4966     Jun 20 Nicolas Pitre   (  27) ├─>Re: [PATCH v2 0/2] ARM: Remove any cor
4967     Jun 21 Will Deacon     (  29) │ └─>
4968     Jun 21 Marc Zyngier    (  27) └─>

Did you copy linux-arm-kernel with your report?

FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Lying to whom? And for what purpose?

Come on. You're the one coming up with idiotic reasoning for the user space visible bogomips number. Give me at least one logical and rational reason why user space should get a timer clock value inconsequential to user space code through this interface and I'll shut up. Otherwise I'll interpret your heating up as your inability to provide such a reason.

Nicolas

What does it mean? That's the actual question. And I'm talking about the user visible value here.

Given the "kernel does not break user space" mantra, and given that user space has assumptions around the exported bogomips value that goes a long way back, I really question what the bogomips meaning is from a user space point of view.

According to the analysis Will Deacon performed on the user code that broke with the bogomips gone, whose breakage started this very thread, this user space instance appears to equate bogomips with CPU MHz.

You said yourself in this thread that one of the bogomips meanings is a value proportional to the number of loops the CPU can perform which is also in the same spirit as CPU MHz.

So far so good.

Since when does bogomips mean kernel low-level timer resolution? As Catalin pointed out, the bogomips value went from 800 down to 6 when timer based udelay was introduced on ARM for the same piece of hardware. Is that really what user space is expecting here?

That's where I don't understand anymore.

If the answer is: "user space should never care because bogomips is totally bogus" as some people are claiming then why all the fuss about "lying"?

I don't want stupid games here either, I want a sensible answer from a user space point of view because that's user space we don't want to lie to, right?

Nicolas

Well, effectively since always, even if it wasn't a primary thing. The primary thing was just the sillyness of it all.

The fact that it always meant low-level timer resolution comes through clearly in the actual change:

The very fact that bogomips changed is kind of indicative of what bogomips _is_, wouldn't you say.

But:

It doesn't matter what user space "expects". User space isn't conscious, and if it was, we still wouldn't care.

All that matters is whether it *works*.

And in this case, I suspect that pretty much any value actually happens to fall in that "works" category.

No the answer is "we have no real idea what crazy things user space might do, and we don't make value judgements on regressions".

User space is filled with insane code, some of it outright *buggy* code, but users don't care. They just care about "it used to work, and now it doesn't".

And, as a result, that's all we care about fixing too.

[ Ok, that's not really true.

Obviously we do care about maintainability and other intrinsic
goals, but to a first approximation the externally visible issues are much more important than our own intrinsic goals ]

And sometimes those kinds of regressions can end up being unsolvable.

Yes, the #1 rule for the kernel is "no regressions. Ever".

But while that is something I want people to consider a hard rule, there are some cases where it's just not possible. I mentioned security issues earlier - we've had cases where we simply could not support an interface because it turned out to be fundamentally flawed in a bad way.

Timing-related things can be similar. The whole "it used to work, now it doesn't" could be an actual race condition - in user space - that just was practically impossible to hit before on a particular machine, and then something changed, and now it's easy to hit. There's nothing we can really do about those kinds of things. It technically *is* still a regression, it's just not one we can fix.

So in theory, bogomips changing (on the same machine) could indeed be a regression. It wouldn't even be one of the "unsolvable" kinds, because while it's timing-related, we *could* fudge the user-visible scale factor etc. I wouldn't _want_ to, but it's not "fundamentally unsolvable"

I'm pretty sure that kind of fudging isn't actually necessary in this case, though.

But that "I'm pretty sure" is not because "people who would depend on bogomips are crazy and applications using it are fundamentally buggy, so we don't need to care". Buggy applications that do insane things still count as regressions.

No, the "pretty sure" is because we know that bogomips varies wildly between different machines, and any user application that actually uses the value and is actively bring used clearly *does* work with different values. It's likely fairly hard to make a real app that does anything actually useful, and that seriously depends on the exact bogomips (where "exact" is "within a few orders of magnitude" - not very exact at all ;).

Sure, it's not impossible, but you *almost* have to actively try to do something insane. And most user-space insanity is not "actively try to do something insane" as just due to plain incompetence.

"Never blame on malice that which can be sufficiently explained by stupidity".

Linus

Side note: this is not a theoretical argument. It has happened. And while it's "unfixable", we've actually had situations where it happened, and we explicitly tried to work around it.

The whole "sched_child_runs_first" sysctl isn't just a tunable (yes, it can also help on some loads), it's at least partly a result of a long-ago bash bug where bash had a race condition with its own fork()'ed children finishing and sending a SIGCHLD before bash had even had time to fill in the child pid information in its own data structures.

So even the "fundamentally impossible to fix" regressions where some scheduler detail changed, and it broke actively buggy user space programs, we've spent effort to maintain compatibility with those buggy applications.

Of course, at some point you do have to make a value judgment on how important the app in question is, and how fatal the breakage is.

Linus

One reason - consistency with kernels running on the *same* hardware prior to commit d0a533b18235d362 (ARM: 7452/1: delay: allow timer-based delay implementation to be selected). You may see BogoMIPS of 800 prior to that commit dropping to 6 after that commit on the *same* piece of hardware. If we don't break any user assumptions here, no further changes needed.

Catalin

Thanks.

The logic for delay_calibrated seemed to prevent changes to lpj in case a better timer source got registered after boot, however commit 6f3d90e5 made that irrelevant. So perhaps delay_calibrated can go now unless there are concerns about possible races if a better timer gets registered and called with arguments for the previous one or the like. In which case I think such a change would be best isolated in a separate patch.

Nicolas

(No, I did not test it, but going to "historical" value should not hurt, and it actually makes code shorter).

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Peversely, I think that '1.00' would be the correct value for your libjack case!

I had a quick look at the code, and it seems to be used to construct a microsecond-precision timeline for things like:

	ctl->signalled_at = jack_get_microseconds();

Now, jack_get_microseconds() is:

	static inline jack_time_t
	jack_get_microseconds (void)
	{
	        return _jack_get_microseconds ();
	}

_jack_get_microseconds is a pointer that points to this guy:

	jack_time_t
	jack_get_microseconds_from_cycles (void) {
	        return get_cycles() / __jack_cpu_mhz;
	}

where __jack_cpu_mhz is the bogomips read once from /proc/cpuinfo and get_cycles (on ARM) is this gem:

	static inline cycles_t get_cycles(void)
	{
	       struct timeval tv;
	       gettimeofday (&tv, NULL);

	       return ((cycles_t) tv.tv_sec * 1000000) + tv.tv_usec;
	}

The return value from jack_get_microseconds gets used interchangeably with real usecond values to do things like find the audio frame at Nus into a stream, so I don't really see how any of this can work. In this case, we just need *a* bogomips value to stop the initialisation from failing, then hope that pyaudio decides not to use jack after all (it falls back to some alsa thing on my machine)

Will

To quote the standard response for people who ignore regressions:

"SHUT THE FUCK UP"

you cannot NAK regression fixes. Seriously.

I don't understand how people can't get this simple thing.

You have two choices:

- acknowledge and fix regressions

- get the hell out of kernel development.

It really is that simple. I really don't get why people don't understand this simple first rule of kernel development. Is it really not widely enough documented?

No regressions. If user mode breaks, it is absolutely *never* acceptable to blame user mode.

Occasionally there may be major overriding reasons (security issue, whatever), but even then we bend over *backwards* trying to make sure breakage is minimal.

Christ people. Why does this even have to be discussed any more?

And I realize that ARM people may not be used to this whole "user mode matters" thing, what with all the crazy hardware incompatibilities. But you guys need to shape up. We don't break things. Not even on ARM.

Linus

How about we calm down and take some informed view instead of throwing mud? I'd never thought I'd have to be doing this but it seems necessary.

Yes, it's bad that we now have a userspace breakage from removing the bogomips, but a simple revert of the patch doesn't fix it properly. It _may_ seem to get things going again by providing a value, but that value may not be what was once provided back in the days that these programs were written.

The problem is that we now have hardware timers providing the kernel's delay implementation, which makes the published "bogomips" value no longer meaningful for the CPU speed.

So, merely reverting the patch doesn't fix the problem. So, Nico's NAK on the simple revert is quite right.

Instead of throwing insults, what we need to do is to approach this calmly, and work out what we can do to fix it properly.

I'm *not* going to be looking at this until at least mid-week because I'm still on my Christmas break, but also I'm unwell which has left me severely lacking sleep. If others want to do all the necessary research, then be my guest.

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

It is the value we had there in 2013, and it seems to be close enough to value userspace expects there. It is certainly safer/better/closer than "1.0" Nicolas proposes.

Looking at 9fc2105aeaaf56b0cf75296a84702d0f9e64437b's changelog, I'd say that insults are not out of place.
Pavel

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

OK, let's look at the issue more deeply then.

We're talking about exporting a bogomips entry via /proc/cpuinfo.

On ARM the calibration loop is no longer performed because udelay() is based on a hardware counter these days. Why? because the calibration was completely unreliable in the presence of CPU frequency scaling, and other modern hardware niceties.

Still, for a while, we did export the calibration loop value for /proc/cpuinfo's sake. Yet people complained because the exported value was "wrong". Of course it is wrong as it is just impossible to get "right", unless you have antique hardware that is.

I'm all for ensuring breakage is minimal. That'd imply settling on a good phony bogomips value that is no more broken than the real one was when it was exported.

Because we're discussing a choice between two evils. The actual regression happened when people upgraded their antique hardware and expected the bogomips to still work with new hardware. Even if the $subject commit is reverted, that won't solve the wrong bogomips problem and different people will start complaining again.

As I mentioned earlier, putting the entry back is not a problem if we may find the best phony number to go along with it.

Nicolas

.. and Pavel pointed you to several screenfuls of google hits for people complaining about this.

End of discussion. Seriously. Your whinging about "support costs" is just crying over the fact that you have users. Deal with it.

I've reverted the change for now, since that's clearly better than leaving things broken. And you should acknowledge that. In fact, you should *more* than acknowledge that, you need to internalize it and *understand* it, instead of arguing.

The fact is, we have bogomips on x86 too, and they've been around for a hell of a lot longer than ARM has been. The number may not be "meaningful" any more (since it's TSC cycles rather than anything else), but it's there in /proc anyway. We had it through the times when it fluctuated, and your made-up arguments about how it's a bad thing are just bogus. It wasn't a big deal, and having a reasonable value is not a problem.

And no, we don't set it to some "obviously bogus value". That just makes breakage even more subtle.

You could make it be something that is roughly on the order of the CPU frequency. It doesn't have to be exact, but from a QoI standpoint, try to do a reasonable job.

And dammit, I really never *ever* want to hear arguments against fixing regressions ever again. It really is the #1 rule for the kernel. There is *no* excuse for that NAK. There is only "sorry".

Linus

No, it is not the end of discussion, because what you've done is /really/ equally bad as what Nico was suggesting. In fact, it /could/ be worse.

Yes, x86 has the bogomips thing, and it switched to using a TSC. I'm willing to bet that on x86, the reported bogomips value is pretty similar whether it's using the TSC or whether it's using a software timing loop.

This is not the case on ARM. Here's an example where we use a hardware timer for the delay loop:

Calibrating delay loop (skipped), value calculated using timer frequency.. 6.00 BogoMIPS (lpj=12000)

which is nowhere near the precision of the CPU clock rate. So, when we have a hardware timer based delay implementation, the bogomips value which the kernel has access to (and thus the loops_per_jiffy value) is totally ... bogus.

If we want to take note of the "screenfulls of users reporting problems" then we need a much better solution than a simple revert, because right now, the simple revert makes machines with GHz clock frequencies look slower than the old 1990s 26-bit ARM technology.

So, Nico's NAK for a simple revert was the _right_ response, even if the remainder was not correct.

Now, Pavel is the /first/ to report this regression to the ARM kernel community. Yes, it may be that other users found a problem and failed to report it (which seems to be a /very/ common problem.) Thanks to Pavel, we're now aware of it, and now we need to fix it properly, not via some bastardized and broken revert by someone who doesn't know the implications of that.

FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Roughly yes. It's a completely dumb number anyway because any loop timing based number is bogus on any modern x86. Utterly bogus. However its armwavingly sufficient that software doesn't break (not that any x86 software I know of does anything but print the number for the user so they can copy it to tweet about how neat their new PC is).

PC people do keep filing bugs about x86 CPU clock numbers being wrong, we just close them all. Clock rate itself is a bit of a meaningless concept nowdays.

So multiply it by a fudge factor so it looks similar and in proportion.

That's a marketing problem not a technical one 8)

You actually have the reverse problem to consider. Who can quantify how many modern ARM apps actually check for Bogomips and then set policy according to its presence/absence. Reverting a 2013 change may actually break a load of software that has been fixed or deals with the real world properly. Pavel doing a random arbitrary long term revert without any planning is at least as stupid as ignoring it.

What I don't understand here is why it was removed for platforms where it was meaningful. Surely that's not needed and antiques running antique userspace would continue to be happy with numbers valid for that era ?

Alan

It's not that simple. The CPU frequency and the timer frequency are independent. One SoC may have 6MHz timer, another SoC with a similar CPU frequency may use a 24MHz timer (or maybe a much smaller one). So a fudge factor is not really any better than choosing a constant bogomips value at build time (say 2000).

If the always constant bogomips is not desirable, we are left with having to do a dummy calibration to guess some value to be reported to user as BogoMIPS. Even if we do this, on modern ARM processors it rarely matches the CPU frequency, only on older/simpler CPUs. With newer ones, the simple two instructions loop is never 2 cycles (it may even differ on the same CPU depending on the kernel build, e.g. code alignment affecting the branch prediction; I recall on ARM11MPCore the average was half-cycle multiple because the loop took different number of cycles on consecutive iterations).

(I'm not debating whether we should fix this problem or not, just trying to find the right solution; reverting the original patch doesn't entirely fix newer CPUs)

Catalin

Not at all.

Just use whatever counter frequency you use for the delay loop (and traditionally, you multiply by two, since the *original* bogomips loop was two instructions).

That's why the bogomips printout has that "500000" value. It's multiplying by two, and then dividing by a million to get MHz.

That's what bogomips *is*, for chrissake! It's a bogus measure of how many times you go through the delay loop.

And the delay loop does *not* have to be that "decrement and loop". It can be "read timer and loop". Since the delay loop is all about microseconds, your timer had betterr have a frequency in the MHz range anyway.

On some platforms, it could be the traditional sw-based loop counter because it's the only thing you have. On others, it could be a CPU cycle counter. On yet others, it could be a specialized timer counter. It doesn't matter, all is valid.

What is *not* valid is clearly:

- removing the bogomips line.

You can try again in a couple of years. Maybe nobody will notice.
But people *did* notice, and that commit got reverted. End of story, anybody who argues for removal is simply wrong.

- making shit up

Bogomips is a bogus benchmark, but it is *not* a constant number,
and it *does* have to do with the timing loop.

The "making shit up" case is less wrong (pretty much by definition, since "breaking user space applications" is the most wrong you can ever be), but you know what? bogomips has a long history of bogosity. It's integral. It goes back a long time. It has historical significance. It isn't 0.01 material, but it was certainly added before 1.0.

.. and it's still a better benchmark than dhrystone.

Linus

I think that's where the misunderstanding is. We don't have any idea how many times we go through the delay loop. We just go through the delay loop until the counter (driven by an independent frequency) changes X times. From arch/arm/lib/delay.c:

static void __timer_delay(unsigned long cycles)

{
        cycles_t start = get_cycles();

        while ((get_cycles() - start) < cycles)
                cpu_relax();
}

where the number of cycles is based on the pre-calculated lpj_fine value using the counter/timer frequency (read from DT or hardware).

I think Nico already pointed out that the way we use loops_per_jiffy when the udelay is timer-based is probably wrong. We actually need a ticks_per_jiffy which has nothing to do with loops_per_jiffy, the former is actually completely independent from the CPU speed.

That's true.

I think we all agree here (now).

Well, reporting timer ticks per jiffy no matter what CPU frequency doesn't look very far from "making shit up" to me.

With the current arm timer-based (and arm64) implementation, the reported BogoMIPS has nothing to do with the CPU benchmark. It just tells you that a X MHz counter needs X*1000000/HZ ticks per jiffy.

(that's another reason we removed it; to stop marketing people from complaining that Linux doesn't look fast on newer ARM CPUs)

So reverting the patch is just the first step, we need to make up some more real loops_per_jiffy now. It's not hard, something like counting how many get_cycle() loops we can get for a jiffy and multiplying it with the number of instructions in the loop.

Catalin

.. and that's exactly what we do on x86 too with the TSC. It's fine.

Yes. And that's a valid bogomips. We've done that for ages on x86.

Really, the "problem" that people wonder why some bogomips value is lower or higher isn't a problem. Just explain it to them. Or ignore them when they ask.

Bogomips is a delay value for the micro-second delay loop. Nothing less, nothing more.

Linus

I may be mistaken but isn't TSC somehow related to the CPU frequency on x86?

On ARM, the generic/architected timer is not. It's just a timer+counter (used as clocksource and event generator in Linux) clocked at a frequency completely independent from the CPU frequency, *no* relation between the two.

I think a better comparison would be to HPET rather than TSC. But on x86 we use HPET to calibrate the TSC and use the TSC for the delay loop. If on x86 the BogoMIPS was changed to report the HPET frequency, would you be ok with it?

See above, my understanding of TSC is that the x86 BogoMIPS is at least in some way closely related to the CPU frequency. That's more like a perf counter on ARM counting the CPU cycles but we don't use it for udelay() (as it may or may not be enabled in a production system).

Catalin

CPU frequency isn't a constant.

Seriously CPU frequency isn't a useful construct in this discussion. It too is bogus, so the TSC may be tied to some clock rate but the clock rate isn't necessarily a constant, and if its a constant its not tied to the rate your CPU will be issuing instructions at any given point.

The x86 bogomips has a relationship in some cases to the cpu base clock, but that relationship itself depends on the CPU model.

It's a random nunber. There is broken code out there which breaks if you don't give it some random number. If you make something up then it "works" (or is at least no more broken than it was).

I doubt any x86 code would break if we simply printed "1000000" all the time.

Alan

We already have the former.

The patch I posted rehabilitated thelater. That's the sanest we can do. Then we have both a timer based delay and a bogomips that is somewhat related to CPU speed again.

I also posted another patch to remove the 3355 bogomips limitation on ARM.

Nicolas