[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

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!".

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index ff6760a..940779c 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -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: ");
 
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 5e6052e..86ef244 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Pavel Machek wrote:

> 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!".
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>

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.


> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index ff6760a..940779c 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -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: ");
>  
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 5e6052e..86ef244 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -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
> 
> 

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Sun 2015-01-04 15:03:02, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Pavel Machek wrote:
> 
> > 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!".
> > 
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> NAK.
> 
> No setups actually relying on this completely fony bogomips value 
> bearing no links to hardware reality coule have been qualified as 
> "working".

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.

> 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.

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

> 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.

That is not how kernel development should work. See

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Pavel Machek wrote:

> On Sun 2015-01-04 15:03:02, Nicolas Pitre wrote:
> > 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.
> 
> I don't know what 1.00 will cause, and neither do you, so what about
> simply reverting the bad patch?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Sun 2015-01-04 15:25:02, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Pavel Machek wrote:
> 
> > On Sun 2015-01-04 15:03:02, Nicolas Pitre wrote:
> > > 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.
> > 
> > I don't know what 1.00 will cause, and neither do you, so what about
> > simply reverting the bad patch?
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Pavel Machek wrote:

> On Sun 2015-01-04 15:25:02, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Pavel Machek wrote:
> > 
> > > On Sun 2015-01-04 15:03:02, Nicolas Pitre wrote:
> > > > 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.
> > > 
> > > I don't know what 1.00 will cause, and neither do you, so what about
> > > simply reverting the bad patch?
> > 
> > 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.
> 
> People complain on the list, so what? People complain about systemd,
> too. We ignore them.

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

> Alternatively, just don't touch the bogomips computation. It is not
> that much of maintainance burden.

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.

> You can probably also get away with
> replacing bogomips with actual cpu frequency.

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

> Replacing it with 1 is asking for trouble.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Linus Torvalds wrote:

> On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > 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.
> 
> Really. Shut up.
> 
> The whole "no regressions" thing is very much about the fact that we
> don't waste users time.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Sun, Jan 04, 2015 at 04:20:57PM -0500, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Linus Torvalds wrote:
> 
> > On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > >
> > > 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.
> > 
> > Really. Shut up.
> > 
> > The whole "no regressions" thing is very much about the fact that we
> > don't waste users time.
> 
> 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.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Sun 2015-01-04 21:26:59, Russell King - ARM Linux wrote:
> On Sun, Jan 04, 2015 at 04:20:57PM -0500, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Linus Torvalds wrote:
> > 
> > > On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > >
> > > > 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.
> > > 
> > > Really. Shut up.
> > > 
> > > The whole "no regressions" thing is very much about the fact that we
> > > don't waste users time.
> > 
> > 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.
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Aaro Koskinen]

Hi,

On Sun, Jan 04, 2015 at 10:40:49PM +0100, Pavel Machek wrote:
> On Sun 2015-01-04 21:26:59, Russell King - ARM Linux wrote:
> > On Sun, Jan 04, 2015 at 04:20:57PM -0500, Nicolas Pitre wrote:
> > > On Sun, 4 Jan 2015, Linus Torvalds wrote:
> > > 
> > > > On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > > >
> > > > > 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.
> > > > 
> > > > Really. Shut up.
> > > > 
> > > > The whole "no regressions" thing is very much about the fact that we
> > > > don't waste users time.
> > > 
> > > 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.
> > 
> > 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.
> 
> 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?

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

A.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Theodore Ts'o]

On Sun, Jan 04, 2015 at 09:26:59PM +0000, Russell King - ARM Linux wrote:
> 
> With the revert in place, we now have insanely small bogomips values
> reported via /proc/cpuinfo when hardware timers are used.  That needs
> fixing.

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

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Will Deacon]

Hi Ted,

On Mon, Jan 05, 2015 at 01:34:36AM +0000, Theodore Ts'o wrote:
> On Sun, Jan 04, 2015 at 09:26:59PM +0000, Russell King - ARM Linux wrote:
> > 
> > With the revert in place, we now have insanely small bogomips values
> > reported via /proc/cpuinfo when hardware timers are used.  That needs
> > fixing.
> 
> Why does it need to be fixed?
> 
> It's clear that there are applications that are working OK with the
> existing value,

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Will Deacon]

Hi Ted,

On Mon, Jan 05, 2015 at 01:34:36AM +0000, Theodore Ts'o wrote:
> On Sun, Jan 04, 2015 at 09:26:59PM +0000, Russell King - ARM Linux wrote:
> > 
> > With the revert in place, we now have insanely small bogomips values
> > reported via /proc/cpuinfo when hardware timers are used.  That needs
> > fixing.
> 
> Why does it need to be fixed?
> 
> It's clear that there are applications that are working OK with the
> existing value,

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:

> On Sun, Jan 04, 2015 at 04:20:57PM -0500, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Linus Torvalds wrote:
> > 
> > > On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > >
> > > > 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.
> > > 
> > > Really. Shut up.
> > > 
> > > The whole "no regressions" thing is very much about the fact that we
> > > don't waste users time.
> > 
> > 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.
> 
> 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.

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

----- >8
From: Nicolas Pitre <nicolas.pitre@linaro.org>
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.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index dff714d886..7feeb163f5 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -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__ */
diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
index 312d43eb68..d958886874 100644
--- a/arch/arm/lib/delay.c
+++ b/arch/arm/lib/delay.c
@@ -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)

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Will Deacon]

Hi all,

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

On Mon, Jan 05, 2015 at 04:51:31AM +0000, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:
> > 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.
> 
> Here's my take on it.  Taking a step back, it was stupid to mix bogomips 
> with timer based delays.

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...

> ----- >8
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 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.

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).

> 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'").

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Will Deacon]

Hi all,

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

On Mon, Jan 05, 2015 at 04:51:31AM +0000, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:
> > 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.
> 
> Here's my take on it.  Taking a step back, it was stupid to mix bogomips 
> with timer based delays.

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...

> ----- >8
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 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.

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).

> 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'").

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.

> 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.

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

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Mon 2015-01-05 12:11:36, Will Deacon wrote:
> Hi all,
> 
> Sorry for the late reply, it seems that neither myself or the
> arm-linux-kernel list were on CC for this thread.

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

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

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Mon 2015-01-05 12:11:36, Will Deacon wrote:
> Hi all,
> 
> Sorry for the late reply, it seems that neither myself or the
> arm-linux-kernel list were on CC for this thread.

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

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

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.

> 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.

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

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Mon, 5 Jan 2015, Will Deacon wrote:

> On Mon, Jan 05, 2015 at 04:51:31AM +0000, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:
> > > 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.
> > 
> > Here's my take on it.  Taking a step back, it was stupid to mix bogomips 
> > with timer based delays.
> 
> Well, bogomips is directly related to loops_per_jiffy so I don't think the
> mechanism is "stupid";

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.

> 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...

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

> > ----- >8
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 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.
> 
> 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.

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.

> 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)

Who cares?

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

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.

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

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.

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

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

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

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

> > 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'").
> 
> 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.

Which is even more absurd, hence my patch.

> > 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.
> 
> 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.

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.

> 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.

That's a weak excuse too.

> 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.

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



Nicolas

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Mon, 5 Jan 2015, Will Deacon wrote:

> On Mon, Jan 05, 2015 at 04:51:31AM +0000, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:
> > > 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.
> > 
> > Here's my take on it.  Taking a step back, it was stupid to mix bogomips 
> > with timer based delays.
> 
> Well, bogomips is directly related to loops_per_jiffy so I don't think the
> mechanism is "stupid";

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.

> 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...

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

> > ----- >8
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 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.
> 
> 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.

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.

> 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)

Who cares?

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

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.

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

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.

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

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

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

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

> > 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'").
> 
> 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.

Which is even more absurd, hence my patch.

> > 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.
> 
> 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.

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.

> 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.

That's a weak excuse too.

> 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.

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



Nicolas

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Mon, 5 Jan 2015, Will Deacon wrote:

> On Mon, Jan 05, 2015 at 04:51:31AM +0000, Nicolas Pitre wrote:
> > 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.
> 
> 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.

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

Now here's the bogomips hard limit gone:

----- >8
From: Nicolas Pitre <nicolas.pitre@linaro.org>
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.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
index 7feeb163f5..287c96ad93 100644
--- a/arch/arm/include/asm/delay.h
+++ b/arch/arm/include/asm/delay.h
@@ -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);
 
diff --git a/arch/arm/lib/delay-loop.S b/arch/arm/lib/delay-loop.S
index 518bf6e93f..bcc23c5760 100644
--- a/arch/arm/lib/delay-loop.S
+++ b/arch/arm/lib/delay-loop.S
@@ -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
 
 /*

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Arnd Bergmann]

On Monday 05 January 2015 23:09:00 Nicolas Pitre wrote:
> 
> Now here's the bogomips hard limit gone:
> 
> ----- >8
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 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.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

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

	Arnd

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Tue, 6 Jan 2015, Arnd Bergmann wrote:

> On Monday 05 January 2015 23:09:00 Nicolas Pitre wrote:
> > 
> > Now here's the bogomips hard limit gone:
> > 
> > ----- >8
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 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.
> > 
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> 
> I think we still build RPC with gcc -march=armv3. Is that a problem
> with this patch?

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

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Tue, 6 Jan 2015, Nicolas Pitre wrote:

> On Tue, 6 Jan 2015, Arnd Bergmann wrote:
> 
> > I think we still build RPC with gcc -march=armv3. Is that a problem
> > with this patch?
> 
> 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?

I settled on the following:

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 0573faab96..0eb8de1c6f 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -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

[PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Arnd Bergmann]

On Tuesday 06 January 2015 16:27:05 Nicolas Pitre wrote:
> Tangential question: does anyone still own a working RPC?

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.

> -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

Looks good to me.

	Arnd

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On 5 January 2015 at 04:51, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 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.

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:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Minor comment below:

>  unsigned long calibrate_delay_is_known(void)
>  {
>         delay_calibrated = true;
> -       return lpj_fine;
> +
> +       /* calibrate bogomips even when timer based delays are used */
> +       return 0;
>  }

Do we need to remove delay_calibrated = true as well? We do it further
down again in calibration_delay_done() .

-- 
Catalin

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 10:11 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> 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).

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Wed, Jan 7, 2015 at 10:11 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> >
> > 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).
> 
> 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".

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 11:00 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Wed, Jan 7, 2015 at 11:00 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > 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.
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Wed, Jan 07, 2015 at 03:34:42PM -0500, Nicolas Pitre wrote:
> On Wed, 7 Jan 2015, Linus Torvalds wrote:
> 
> > On Wed, Jan 7, 2015 at 11:00 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > >
> > > 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.
> > 
> > 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.
> 
> I think you are missing something fundamental in this thread.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Russell King - ARM Linux wrote:

> On Wed, Jan 07, 2015 at 03:34:42PM -0500, Nicolas Pitre wrote:
> > On Wed, 7 Jan 2015, Linus Torvalds wrote:
> > 
> > > On Wed, Jan 7, 2015 at 11:00 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > >
> > > > 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.
> > > 
> > > 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.
> > 
> > I think you are missing something fundamental in this thread.
> 
> 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.

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.

> 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.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Steven Rostedt]

On Wed, Jan 07, 2015 at 04:15:00PM -0500, Nicolas Pitre wrote:
> 
> > 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.
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On 7 January 2015 at 20:53, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> 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.

Fine.

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

Fine as well.

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

Indeed.

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

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.

> 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.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 2:14 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> 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?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> 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.

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.

> 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.

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 4:45 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Wed, Jan 7, 2015 at 4:45 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > 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.
> 
> Yes, I actually would mind, unless you have a damn good reason for it.

Consistency.

> On x86, we have bogomips, but we also have this line:
> 
>    cpu MHz : 2275.109

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.

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

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 frankly, the *only* actual real reason I've heard from you for
> not having the real bogomips there is "waste of time".

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:

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 8:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>>
>> Yes, I actually would mind, unless you have a damn good reason for it.
>
> Consistency.

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.

> On ARM we just can't find the CPU clock in a generic way.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Wed, Jan 7, 2015 at 8:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >>
> >> Yes, I actually would mind, unless you have a damn good reason for it.
> >
> > Consistency.
> 
> Fuck no.
> 
> "Completely made up number that you cannot explain" is not consistency.

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".

> 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?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Wed, Jan 07, 2015 at 10:14:07PM +0000, Catalin Marinas wrote:
> On 7 January 2015 at 20:53, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > 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.
> 
> Agree. But I assume you refer to the fact that we removed the BogoMIPS
> reporting. It's fine to have it reverted.
> 
> > 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.
> 
> 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.

Correct.

> 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.

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 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.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Vince Weaver]

On Thu, 8 Jan 2015, Russell King - ARM Linux wrote:

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On Thu, Jan 08, 2015 at 03:44:53PM +0000, Vince Weaver wrote:
> On Thu, 8 Jan 2015, Russell King - ARM Linux wrote:
> 
> > 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.
> 
> 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.  

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

> 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.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> I really think we should stop this thread. User ABI breakage fixed now.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > I really think we should stop this thread. User ABI breakage fixed now.
> 
> 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.

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

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

-- 
Catalin

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > I really think we should stop this thread. User ABI breakage fixed now.
> > 
> > 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.
> 
> FYI, searching for "Vince Weaver" and "bogomips" found this:
> 
> https://lkml.org/lkml/2013/7/11/454

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On Thu, Jan 08, 2015 at 04:57:05PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> > On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > > I really think we should stop this thread. User ABI breakage fixed now.
> > > 
> > > 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.
> > 
> > FYI, searching for "Vince Weaver" and "bogomips" found this:
> > 
> > https://lkml.org/lkml/2013/7/11/454
> 
> 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?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Vince Weaver]

On Thu, 8 Jan 2015, Catalin Marinas wrote:

> On Thu, Jan 08, 2015 at 04:57:05PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> > > On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > > > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > > > I really think we should stop this thread. User ABI breakage fixed now.
> > > > 
> > > > 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.
> > > 
> > > FYI, searching for "Vince Weaver" and "bogomips" found this:
> > > 
> > > https://lkml.org/lkml/2013/7/11/454
> > 
> > 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?
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Vince Weaver]

On Thu, 8 Jan 2015, Russell King - ARM Linux wrote:

> On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> > On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > > I really think we should stop this thread. User ABI breakage fixed now.
> > > 
> > > 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.
> > 
> > FYI, searching for "Vince Weaver" and "bogomips" found this:
> > 
> > https://lkml.org/lkml/2013/7/11/454
> 
> 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.

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.

> 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?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Thu 2015-01-08 16:57:05, Russell King - ARM Linux wrote:
> On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> > On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > > I really think we should stop this thread. User ABI breakage fixed now.
> > > 
> > > 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.
> > 
> > FYI, searching for "Vince Weaver" and "bogomips" found this:
> > 
> > https://lkml.org/lkml/2013/7/11/454
> 
> 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.

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.

> 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?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On Thu, Jan 08, 2015 at 10:46:14PM +0000, Pavel Machek wrote:
> On Thu 2015-01-08 16:57:05, Russell King - ARM Linux wrote:
> > On Thu, Jan 08, 2015 at 04:41:12PM +0000, Catalin Marinas wrote:
> > > On Thu, Jan 08, 2015 at 04:34:50PM +0000, Russell King - ARM Linux wrote:
> > > > On Thu, Jan 08, 2015 at 04:19:08PM +0000, Catalin Marinas wrote:
> > > > > I really think we should stop this thread. User ABI breakage fixed now.
> > > > 
> > > > 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.
> > > 
> > > FYI, searching for "Vince Weaver" and "bogomips" found this:
> > > 
> > > https://lkml.org/lkml/2013/7/11/454
> > 
> > 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.
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Thu, Jan 08, 2015 at 10:44:53AM -0500, Vince Weaver wrote:
> On Thu, 8 Jan 2015, Russell King - ARM Linux wrote:
> 
> > 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.
> 
> 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.  

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

> 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.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Jan 7, 2015 12:34 PM, "Nicolas Pitre" <nicolas.pitre@linaro.org> wrote:
> >
> > I think you are missing something fundamental in this thread.
> 
> No I'm not.
> 
> Dammit, I understand. You guys are the ones who are confused. You have a
> clock, you use it, and you report *that* clock for bogomips.
> 
> It doesn't matter that it's just 10MHz rather than the CPU clock.
> 
> Lying about it is *not* fine, for all the reasons I've already wasted too
> much time trying to explain.

Lying to whom?  And for what purpose?

> Christ, stop with the idiocy already.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Linus Torvalds wrote:

> On Jan 7, 2015 1:29 PM, "Nicolas Pitre" <nicolas.pitre@linaro.org> wrote:
> > >
> > > Lying about it is *not* fine, for all the reasons I've already wasted
> too
> > > much time trying to explain.
> >
> > Lying to whom?  And for what purpose?
> 
> You guys are the ones suggesting various "scaling" things to make the delay
> loop look different from what it is. That's just dishonesty. It actively
> hides the resolution of the delay time source, and it is a bad bad idea.
> 
> Just educate people, don't lie to them. Tell them what bogomips means,
> instead of trying to make shit up.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 2:42 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> Since when does bogomips mean kernel low-level timer resolution?

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:

> 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.

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

But:

> Is that really what user space is expecting here?

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.

> 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"?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Wed, Jan 7, 2015 at 4:25 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On 7 January 2015 at 19:36, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Wed, 7 Jan 2015, Catalin Marinas wrote:

> On 5 January 2015 at 04:51, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > From: Nicolas Pitre <nicolas.pitre@linaro.org>
> > 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.
> 
> 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:
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.

> Minor comment below:
> 
> >  unsigned long calibrate_delay_is_known(void)
> >  {
> >         delay_calibrated = true;
> > -       return lpj_fine;
> > +
> > +       /* calibrate bogomips even when timer based delays are used */
> > +       return 0;
> >  }
> 
> Do we need to remove delay_calibrated = true as well? We do it further
> down again in calibration_delay_done() .

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Sun 2015-01-04 23:51:31, Nicolas Pitre wrote:
> On Sun, 4 Jan 2015, Russell King - ARM Linux wrote:
> 
> > On Sun, Jan 04, 2015 at 04:20:57PM -0500, Nicolas Pitre wrote:
> > > On Sun, 4 Jan 2015, Linus Torvalds wrote:
> > > 
> > > > On Sun, Jan 4, 2015 at 12:56 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > > >
> > > > > 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.
> > > > 
> > > > Really. Shut up.
> > > > 
> > > > The whole "no regressions" thing is very much about the fact that we
> > > > don't waste users time.
> > > 
> > > 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.
> > 
> > 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.
> 
> Here's my take on it.  Taking a step back, it was stupid to mix bogomips 
> with timer based delays.
> 
> ----- >8
> From: Nicolas Pitre <nicolas.pitre@linaro.org>
> 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.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

Acked-by: Pavel Machek <pavel@ucw.cz>

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

> diff --git a/arch/arm/include/asm/delay.h b/arch/arm/include/asm/delay.h
> index dff714d886..7feeb163f5 100644
> --- a/arch/arm/include/asm/delay.h
> +++ b/arch/arm/include/asm/delay.h
> @@ -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__ */
> diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
> index 312d43eb68..d958886874 100644
> --- a/arch/arm/lib/delay.c
> +++ b/arch/arm/lib/delay.c
> @@ -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)

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Will Deacon]

On Sun, Jan 04, 2015 at 08:37:24PM +0000, Pavel Machek wrote:
> On Sun 2015-01-04 15:25:02, Nicolas Pitre wrote:
> > On Sun, 4 Jan 2015, Pavel Machek wrote:
> > 
> > > On Sun 2015-01-04 15:03:02, Nicolas Pitre wrote:
> > > > 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.
> > > 
> > > I don't know what 1.00 will cause, and neither do you, so what about
> > > simply reverting the bad patch?
> > 
> > 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.
> 
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Sun, Jan 4, 2015 at 12:03 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> NAK.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Sun, Jan 04, 2015 at 12:22:36PM -0800, Linus Torvalds wrote:
> On Sun, Jan 4, 2015 at 12:03 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > NAK.
> 
> To quote the standard response for people who ignore regressions:
> 
>   "SHUT THE FUCK UP"
> 
> you cannot NAK regression fixes. Seriously.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Pavel Machek]

On Sun 2015-01-04 20:43:23, Russell King - ARM Linux wrote:
> On Sun, Jan 04, 2015 at 12:22:36PM -0800, Linus Torvalds wrote:
> > On Sun, Jan 4, 2015 at 12:03 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > >
> > > NAK.
> > 
> > To quote the standard response for people who ignore regressions:
> > 
> >   "SHUT THE FUCK UP"
> > 
> > you cannot NAK regression fixes. Seriously.
> 
> 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.

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.

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Sun, 4 Jan 2015, Linus Torvalds wrote:

> On Sun, Jan 4, 2015 at 12:03 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > NAK.
> 
> 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.

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.

> 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.

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.

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

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Sun, Jan 4, 2015 at 12:45 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
> Because we're discussing a choice between two evils.

.. 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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Russell King - ARM Linux]

On Sun, Jan 04, 2015 at 12:57:09PM -0800, Linus Torvalds wrote:
> On Sun, Jan 4, 2015 at 12:45 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> > Because we're discussing a choice between two evils.
> 
> .. 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.

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.

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[One Thousand Gnomes]

> 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.

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.

> 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.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On 5 January 2015 at 14:22, One Thousand Gnomes
<gnomes@lxorguk.ukuu.org.uk> wrote:
>> 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.
>
> So multiply it by a fudge factor so it looks similar and in proportion.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Mon, Jan 5, 2015 at 10:22 AM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On 6 January 2015 at 22:33, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jan 5, 2015 at 10:22 AM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
>>
>> 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.
>
> 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.

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.

> 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.

That's true.

> 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.

I think we all agree here (now).

>    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

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

>    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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Linus Torvalds]

On Tue, Jan 6, 2015 at 3:42 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>>
>> That's what bogomips *is*, for chrissake! It's a bogus measure of how
>> many times you go through the delay loop.
>
> 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.

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

> 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.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Catalin Marinas]

On Wed, Jan 07, 2015 at 01:20:06AM +0000, Linus Torvalds wrote:
> On Tue, Jan 6, 2015 at 3:42 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> >>
> >> That's what bogomips *is*, for chrissake! It's a bogus measure of how
> >> many times you go through the delay loop.
> >
> > 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.
> 
> .. and that's exactly what we do on x86 too with the TSC. It's fine.

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?

> > 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.
> 
> Yes. And that's a valid bogomips. We've done that for ages on x86.

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[One Thousand Gnomes]

On Wed, 7 Jan 2015 15:01:36 +0000
Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Wed, Jan 07, 2015 at 01:20:06AM +0000, Linus Torvalds wrote:
> > On Tue, Jan 6, 2015 at 3:42 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > >>
> > >> That's what bogomips *is*, for chrissake! It's a bogus measure of how
> > >> many times you go through the delay loop.
> > >
> > > 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.
> > 
> > .. and that's exactly what we do on x86 too with the TSC. It's fine.
> 
> I may be mistaken but isn't TSC somehow related to the CPU frequency on
> x86?

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

Re: [PATCH] Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

[Nicolas Pitre]

On Tue, 6 Jan 2015, Catalin Marinas wrote:

> 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.

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