2015-11-18 18:34:09

by Shi, Yang

[permalink] [raw]
Subject: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

As what Pavel Machek reported [1], some userspace applications depend on
bogomips showed by /proc/cpuinfo.

Although there is much less legacy impact on aarch64 than arm, but it does
break libvirt.

Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
some tweak due to context change.

[1] https://lkml.org/lkml/2015/1/4/132

Signed-off-by: Yang Shi <[email protected]>
---
arch/arm64/kernel/cpuinfo.c | 5 +++++
arch/arm64/kernel/smp.c | 7 ++++++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 706679d..8d4ba77 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -30,6 +30,7 @@
#include <linux/seq_file.h>
#include <linux/sched.h>
#include <linux/smp.h>
+#include <linux/delay.h>

/*
* In case the boot CPU is hotpluggable, we record its initial state and
@@ -112,6 +113,10 @@ static int c_show(struct seq_file *m, void *v)
*/
seq_printf(m, "processor\t: %d\n", i);

+ seq_printf(m, "BogoMIPS\t: %lu.%02lu\n\n",
+ loops_per_jiffy / (500000UL/HZ),
+ loops_per_jiffy / (5000UL/HZ) % 100);
+
/*
* Dump out the common processor features in a single line.
* Userspace should read the hwcaps with getauxval(AT_HWCAP)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b1adc51..1bed772 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -326,7 +326,12 @@ static void __init hyp_mode_check(void)

void __init smp_cpus_done(unsigned int max_cpus)
{
- pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
+ unsigned long bogosum = loops_per_jiffy * num_online_cpus();
+
+ pr_info("SMP: Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
+ num_online_cpus(), bogosum / (500000/HZ),
+ (bogosum / (5000/HZ)) % 100);
+
setup_cpu_features();
hyp_mode_check();
apply_alternatives_all();
--
2.0.2


2015-11-18 18:47:40

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

Hello,

On Wed, Nov 18, 2015 at 10:15:05AM -0800, Yang Shi wrote:
> As what Pavel Machek reported [1], some userspace applications depend on
> bogomips showed by /proc/cpuinfo.
>
> Although there is much less legacy impact on aarch64 than arm, but it does
> break libvirt.
>
> Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> some tweak due to context change.
>
> [1] https://lkml.org/lkml/2015/1/4/132

I lost this argument last time around, so I won't re-tread that path this
time around. I do, however, have some comments on the patch.

>
> Signed-off-by: Yang Shi <[email protected]>
> ---
> arch/arm64/kernel/cpuinfo.c | 5 +++++
> arch/arm64/kernel/smp.c | 7 ++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 706679d..8d4ba77 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -30,6 +30,7 @@
> #include <linux/seq_file.h>
> #include <linux/sched.h>
> #include <linux/smp.h>
> +#include <linux/delay.h>
>
> /*
> * In case the boot CPU is hotpluggable, we record its initial state and
> @@ -112,6 +113,10 @@ static int c_show(struct seq_file *m, void *v)
> */
> seq_printf(m, "processor\t: %d\n", i);
>
> + seq_printf(m, "BogoMIPS\t: %lu.%02lu\n\n",

This double newline makes /proc/cpuinfo looks really odd. Can we just
have one, please?

> + loops_per_jiffy / (500000UL/HZ),
> + loops_per_jiffy / (5000UL/HZ) % 100);
> +
> /*
> * Dump out the common processor features in a single line.
> * Userspace should read the hwcaps with getauxval(AT_HWCAP)
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index b1adc51..1bed772 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -326,7 +326,12 @@ static void __init hyp_mode_check(void)
>
> void __init smp_cpus_done(unsigned int max_cpus)
> {
> - pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
> + unsigned long bogosum = loops_per_jiffy * num_online_cpus();
> +
> + pr_info("SMP: Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
> + num_online_cpus(), bogosum / (500000/HZ),
> + (bogosum / (5000/HZ)) % 100);

Can we drop this hunk? I don't see a pressing need to print this in
dmesg.

With those two changes:

Acked-by: Will Deacon <[email protected]>

I guess this needs Cc'ing to stable, too.

Thanks,

Will

2015-11-18 18:54:12

by Shi, Yang

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On 11/18/2015 10:47 AM, Will Deacon wrote:
> Hello,
>
> On Wed, Nov 18, 2015 at 10:15:05AM -0800, Yang Shi wrote:
>> As what Pavel Machek reported [1], some userspace applications depend on
>> bogomips showed by /proc/cpuinfo.
>>
>> Although there is much less legacy impact on aarch64 than arm, but it does
>> break libvirt.
>>
>> Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
>> ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
>> some tweak due to context change.
>>
>> [1] https://lkml.org/lkml/2015/1/4/132
>
> I lost this argument last time around, so I won't re-tread that path this
> time around. I do, however, have some comments on the patch.
>
>>
>> Signed-off-by: Yang Shi <[email protected]>
>> ---
>> arch/arm64/kernel/cpuinfo.c | 5 +++++
>> arch/arm64/kernel/smp.c | 7 ++++++-
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
>> index 706679d..8d4ba77 100644
>> --- a/arch/arm64/kernel/cpuinfo.c
>> +++ b/arch/arm64/kernel/cpuinfo.c
>> @@ -30,6 +30,7 @@
>> #include <linux/seq_file.h>
>> #include <linux/sched.h>
>> #include <linux/smp.h>
>> +#include <linux/delay.h>
>>
>> /*
>> * In case the boot CPU is hotpluggable, we record its initial state and
>> @@ -112,6 +113,10 @@ static int c_show(struct seq_file *m, void *v)
>> */
>> seq_printf(m, "processor\t: %d\n", i);
>>
>> + seq_printf(m, "BogoMIPS\t: %lu.%02lu\n\n",
>
> This double newline makes /proc/cpuinfo looks really odd. Can we just
> have one, please?
>
>> + loops_per_jiffy / (500000UL/HZ),
>> + loops_per_jiffy / (5000UL/HZ) % 100);
>> +
>> /*
>> * Dump out the common processor features in a single line.
>> * Userspace should read the hwcaps with getauxval(AT_HWCAP)
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index b1adc51..1bed772 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -326,7 +326,12 @@ static void __init hyp_mode_check(void)
>>
>> void __init smp_cpus_done(unsigned int max_cpus)
>> {
>> - pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
>> + unsigned long bogosum = loops_per_jiffy * num_online_cpus();
>> +
>> + pr_info("SMP: Total of %d processors activated (%lu.%02lu BogoMIPS).\n",
>> + num_online_cpus(), bogosum / (500000/HZ),
>> + (bogosum / (5000/HZ)) % 100);
>
> Can we drop this hunk? I don't see a pressing need to print this in
> dmesg.

Sure, will solve them in v2.

Thanks,
Yang

>
> With those two changes:
>
> Acked-by: Will Deacon <[email protected]>
>
> I guess this needs Cc'ing to stable, too.
>
> Thanks,
>
> Will
>

2015-11-18 21:55:15

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, 18 Nov 2015, Will Deacon wrote:

> Hello,
>
> On Wed, Nov 18, 2015 at 10:15:05AM -0800, Yang Shi wrote:
> > As what Pavel Machek reported [1], some userspace applications depend on
> > bogomips showed by /proc/cpuinfo.
> >
> > Although there is much less legacy impact on aarch64 than arm, but it does
> > break libvirt.
> >
> > Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> > ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> > some tweak due to context change.
> >
> > [1] https://lkml.org/lkml/2015/1/4/132
>
> I lost this argument last time around, so I won't re-tread that path this
> time around.

No kidding. ;-)

> I do, however, have some comments on the patch.
>
> >
> > Signed-off-by: Yang Shi <[email protected]>
> > ---
> > arch/arm64/kernel/cpuinfo.c | 5 +++++
> > arch/arm64/kernel/smp.c | 7 ++++++-
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > index 706679d..8d4ba77 100644
> > --- a/arch/arm64/kernel/cpuinfo.c
> > +++ b/arch/arm64/kernel/cpuinfo.c
> > @@ -30,6 +30,7 @@
> > #include <linux/seq_file.h>
> > #include <linux/sched.h>
> > #include <linux/smp.h>
> > +#include <linux/delay.h>
> >
> > /*
> > * In case the boot CPU is hotpluggable, we record its initial state and
> > @@ -112,6 +113,10 @@ static int c_show(struct seq_file *m, void *v)
> > */
> > seq_printf(m, "processor\t: %d\n", i);
> >
> > + seq_printf(m, "BogoMIPS\t: %lu.%02lu\n\n",
>
> This double newline makes /proc/cpuinfo looks really odd. Can we just
> have one, please?
>
> > + loops_per_jiffy / (500000UL/HZ),
> > + loops_per_jiffy / (5000UL/HZ) % 100);

Also, given nobody ever relied on any prior value here, can we at least
print something here with some semblance of a meaning i.e. something
related to the actual CPU speed and not some separate useless constant
timer clock please?


Nicolas

2015-11-19 12:20:34

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, Nov 18, 2015 at 04:55:10PM -0500, Nicolas Pitre wrote:
> On Wed, 18 Nov 2015, Will Deacon wrote:
> > On Wed, Nov 18, 2015 at 10:15:05AM -0800, Yang Shi wrote:
> > > + loops_per_jiffy / (500000UL/HZ),
> > > + loops_per_jiffy / (5000UL/HZ) % 100);
>
> Also, given nobody ever relied on any prior value here, can we at least
> print something here with some semblance of a meaning i.e. something
> related to the actual CPU speed and not some separate useless constant
> timer clock please?

There's also an argument that a 32-bit application should see the same
BogoMIPS value under an arm64 kernel as an arm kernel running on the
same machine. But I was never clear about what this value was intended
to mean anyway, despite all the shouting :)

Right now, I'm happy with anything that fixes libvirt and backports
easily.

Will

2015-11-25 05:45:24

by Jon Masters

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On 11/18/15, 1:15 PM, Yang Shi wrote:

> As what Pavel Machek reported [1], some userspace applications depend on
> bogomips showed by /proc/cpuinfo.
>
> Although there is much less legacy impact on aarch64 than arm, but it does
> break libvirt.
>
> Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> some tweak due to context change.

On a total tangent, it would be ideal to (eventually) have something
reported in /proc/cpuinfo or dmesg during boot that does "accurately"
map back to the underlying core frequency (as opposed to the generic
timer frequency). I have seen almost countless silly situations in the
industry (external to my own organization) in which someone has taken a
$VENDOR_X reference system that they're not supposed to run benchmarks
on, and they've done it anyway. But usually on some silicon that's
clocked multiples under what production would be. Then silly rumors
about performance get around because nobody can do simple arithmetic and
notice that they ought to have at least divided by some factor.

Whether we do this through one of the platform tables or otherwise
(multiple vendor EFI firmwares are being modified to make this much more
glaringly obvious in the GUI view of system configuration so that when
they do things they shouldn't, it's at least in the output) we should
ultimately make sure that idiots at least have a fighting chance of
noticing that they're actually running at 1GHz, and not 2 or 3GHz.

Jon.

2015-11-25 11:52:35

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, Nov 25, 2015 at 12:45:13AM -0500, Jon Masters wrote:
> On 11/18/15, 1:15 PM, Yang Shi wrote:
>
> >As what Pavel Machek reported [1], some userspace applications depend on
> >bogomips showed by /proc/cpuinfo.
> >
> >Although there is much less legacy impact on aarch64 than arm, but it does
> >break libvirt.
> >
> >Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> >("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> >some tweak due to context change.
>
> On a total tangent, it would be ideal to (eventually) have something
> reported in /proc/cpuinfo or dmesg during boot that does "accurately" map
> back to the underlying core frequency (as opposed to the generic timer
> frequency).

I'm fine with this if someone proposes a (sane) patch. But it wouldn't
be for stable.

--
Catalin

2015-11-25 12:00:40

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, Nov 25, 2015 at 12:45:13AM -0500, Jon Masters wrote:
> On 11/18/15, 1:15 PM, Yang Shi wrote:
>
> >As what Pavel Machek reported [1], some userspace applications depend on
> >bogomips showed by /proc/cpuinfo.
> >
> >Although there is much less legacy impact on aarch64 than arm, but it does
> >break libvirt.
> >
> >Basically, this patch reverts commit 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> >("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> >some tweak due to context change.
>
> On a total tangent, it would be ideal to (eventually) have something
> reported in /proc/cpuinfo or dmesg during boot that does "accurately" map
> back to the underlying core frequency (as opposed to the generic timer
> frequency).

Absolutely not. You need to read the discussion that we had with Linus
when someone reported that ARMs /proc/cpuinfo no longer reported it.

Linus says that Bogomips reports the calibration value for udelay(),
period. Whether that's a software loop, or a hardware timer is neither
here nor there. It's nothing to do with the CPU clock rate.

So, if your timer runs slowly, but your CPU is at the top end, a report
of 6 Bogomips is (as far as Linus is concerned) absolutely correct, and
we have no business at all as kernel developers coming up with some fake
or real value to make it report as the CPU clock rate.

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

2015-11-25 12:36:18

by Riku Voipio

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On 25 November 2015 at 07:45, Jon Masters <[email protected]> wrote:
> On a total tangent, it would be ideal to (eventually) have something
> reported in /proc/cpuinfo or dmesg during boot that does "accurately" map
> back to the underlying core frequency (as opposed to the generic timer
> frequency).

Try:

grep . /sys/devices/system/cpu/cpu?/cpufreq/scaling_cur_freq

2015-11-25 15:16:07

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, 25 Nov 2015, Jon Masters wrote:

> On 11/18/15, 1:15 PM, Yang Shi wrote:
>
> > As what Pavel Machek reported [1], some userspace applications depend on
> > bogomips showed by /proc/cpuinfo.
> >
> > Although there is much less legacy impact on aarch64 than arm, but it does
> > break libvirt.
> >
> > Basically, this patch reverts commit
> > 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> > ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
> > some tweak due to context change.
>
> On a total tangent, it would be ideal to (eventually) have something reported
> in /proc/cpuinfo or dmesg during boot that does "accurately" map back to the
> underlying core frequency (as opposed to the generic timer frequency). I have
> seen almost countless silly situations in the industry (external to my own
> organization) in which someone has taken a $VENDOR_X reference system that
> they're not supposed to run benchmarks on, and they've done it anyway. But
> usually on some silicon that's clocked multiples under what production would
> be. Then silly rumors about performance get around because nobody can do
> simple arithmetic and notice that they ought to have at least divided by some
> factor.

Be my guest my friend.

According to the common wisdom, the bogomips reporting is completely
senseless at this point and no one should expect anything useful from
it. Therefore I attempted to rehabilitate some meaning into it given
that we just can't get rid of it either and it continues to cause
dammage. You certainly saw where that has led me.


Nicolas

2015-11-25 17:32:26

by Shi, Yang

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On 11/25/2015 7:16 AM, Nicolas Pitre wrote:
> On Wed, 25 Nov 2015, Jon Masters wrote:
>
>> On 11/18/15, 1:15 PM, Yang Shi wrote:
>>
>>> As what Pavel Machek reported [1], some userspace applications depend on
>>> bogomips showed by /proc/cpuinfo.
>>>
>>> Although there is much less legacy impact on aarch64 than arm, but it does
>>> break libvirt.
>>>
>>> Basically, this patch reverts commit
>>> 326b16db9f69fd0d279be873c6c00f88c0a4aad5
>>> ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but with
>>> some tweak due to context change.
>>
>> On a total tangent, it would be ideal to (eventually) have something reported
>> in /proc/cpuinfo or dmesg during boot that does "accurately" map back to the
>> underlying core frequency (as opposed to the generic timer frequency). I have
>> seen almost countless silly situations in the industry (external to my own
>> organization) in which someone has taken a $VENDOR_X reference system that
>> they're not supposed to run benchmarks on, and they've done it anyway. But
>> usually on some silicon that's clocked multiples under what production would
>> be. Then silly rumors about performance get around because nobody can do
>> simple arithmetic and notice that they ought to have at least divided by some
>> factor.
>
> Be my guest my friend.
>
> According to the common wisdom, the bogomips reporting is completely
> senseless at this point and no one should expect anything useful from
> it. Therefore I attempted to rehabilitate some meaning into it given
> that we just can't get rid of it either and it continues to cause
> dammage. You certainly saw where that has led me.

Or we may create a new one, i.e. "cpu MHz" like x86? Then we keep both
in cpuinfo so that the userspace could adopt it gradually?

Thanks,
Yang

>
>
> Nicolas
>

2015-11-25 18:09:20

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, 25 Nov 2015, Shi, Yang wrote:

> On 11/25/2015 7:16 AM, Nicolas Pitre wrote:
> > On Wed, 25 Nov 2015, Jon Masters wrote:
> >
> > > On 11/18/15, 1:15 PM, Yang Shi wrote:
> > >
> > > > As what Pavel Machek reported [1], some userspace applications depend on
> > > > bogomips showed by /proc/cpuinfo.
> > > >
> > > > Although there is much less legacy impact on aarch64 than arm, but it
> > > > does
> > > > break libvirt.
> > > >
> > > > Basically, this patch reverts commit
> > > > 326b16db9f69fd0d279be873c6c00f88c0a4aad5
> > > > ("arm64: delay: don't bother reporting bogomips in /proc/cpuinfo"), but
> > > > with
> > > > some tweak due to context change.
> > >
> > > On a total tangent, it would be ideal to (eventually) have something
> > > reported
> > > in /proc/cpuinfo or dmesg during boot that does "accurately" map back to
> > > the
> > > underlying core frequency (as opposed to the generic timer frequency). I
> > > have
> > > seen almost countless silly situations in the industry (external to my own
> > > organization) in which someone has taken a $VENDOR_X reference system that
> > > they're not supposed to run benchmarks on, and they've done it anyway. But
> > > usually on some silicon that's clocked multiples under what production
> > > would
> > > be. Then silly rumors about performance get around because nobody can do
> > > simple arithmetic and notice that they ought to have at least divided by
> > > some
> > > factor.
> >
> > Be my guest my friend.
> >
> > According to the common wisdom, the bogomips reporting is completely
> > senseless at this point and no one should expect anything useful from
> > it. Therefore I attempted to rehabilitate some meaning into it given
> > that we just can't get rid of it either and it continues to cause
> > dammage. You certainly saw where that has led me.
>
> Or we may create a new one, i.e. "cpu MHz" like x86? Then we keep both in
> cpuinfo so that the userspace could adopt it gradually?

The problem is that CPU MHz is not known/discoverable on all platforms.
The initial spirit behind bogomips was close to CPU clock with rough
precision that could be determined at run time.

But CPU MHz, when available, has the merit of not being open to
interpretation.


Nicolas

2015-11-26 10:23:14

by Jon Medhurst (Tixy)

[permalink] [raw]
Subject: Re: [PATCH] arm64: restore bogomips information in /proc/cpuinfo

On Wed, 2015-11-25 at 13:09 -0500, Nicolas Pitre wrote:
> But CPU MHz, when available, has the merit of not being open to
> interpretation.

Current MHz, maximum MHz or 'turbo' MHz? ;-)

--
Tixy