diff mbox

Revert 9fc2105aeaaf56b0cf75296a84702d0f9e64437b to fix pyaudio (and probably more)

Message ID alpine.LFD.2.11.1501042343240.18844@knanqh.ubzr
State New
Headers show

Commit Message

Nicolas Pitre Jan. 5, 2015, 4:51 a.m. UTC
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>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Nicolas Pitre Jan. 5, 2015, 4:24 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 6:50 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 7 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 8:34 p.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 9:15 p.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 9:29 p.m. UTC | #6
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 7, 2015, 10:42 p.m. UTC | #7
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 8, 2015, 12:45 a.m. UTC | #8
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 8, 2015, 4:56 a.m. UTC | #9
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre Jan. 8, 2015, 5:54 a.m. UTC | #10
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

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)