Re: [RFC] Improving udelay/ndelay on platforms where that is possible

[Doug Anderson]

Hi,

On Thu, Nov 16, 2017 at 3:22 PM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxx> wrote:
> On Thu, Nov 16, 2017 at 02:15:02PM -0800, Doug Anderson wrote:
>> Hi,
>>
>> On Thu, Nov 16, 2017 at 1:05 PM, Marc Gonzalez
>> <marc_gonzalez@xxxxxxxxxxxxxxxx> wrote:
>> > On 16/11/2017 18:05, Russell King - ARM Linux wrote:
>> >
>> >> On Thu, Nov 16, 2017 at 05:42:36PM +0100, Marc Gonzalez wrote:
>> >>
>> >>> Requesting 100 ç and spinning only 25 ç is still a problem,
>> >>> don't you agree?
>> >>
>> >> Which is why, as I've said *many* times already, that drivers are written
>> >> with leaway on the delays.
>> >
>> > A delay 75% too short is possible. Roger that.
>> >
>> >> I get the impression that we're just going around in circles, and what
>> >> you're trying to do is to get me to agree with your point of view.
>> >> That's not going to happen, because I know the history over about the
>> >> last /24/ years of kernel development (which is how long I've been
>> >> involved with the kernel.)  That's almost a quarter of a century!
>> >>
>> >> I know how things were done years ago (which is relevant because we
>> >> still have support in the kernel for these systems), and I also know the
>> >> history of facilities like cpufreq - I was the one who took the work
>> >> that Erik Mouw and others involved with the LART project, and turned it
>> >> into something a little more generic.  The idea of dynamically scaling
>> >> the CPU frequency on ARM SoCs was something that the SoC manufacturer
>> >> had not even considered - it was innovative.
>> >>
>> >> I know that udelay() can return short delays when used in a kernel with
>> >> cpufreq enabled, and I also know that's almost an impossible problem to
>> >> solve without going to a timer-based delay.
>> >>
>> >> So, when you think that sending an email about a udelay() that can be
>> >> 10x shorter might be somehow new information, and might convince people
>> >> that there's a problem, I'm afraid that it isn't really new information.
>> >> The SA1110 cpufreq driver is dated 2001, and carries my copyright, and
>> >> has the ability to make udelay()s 4x shorter or 4x longer depending on
>> >> the direction of change.
>> >>
>> >> We've discussed solutions in the past (probably 10 years ago) about
>> >> this, and what can be done, and the conclusion to that was, as Nicolas
>> >> has said, to switch to using a timer-based delay mechanism where
>> >> possible.  Where this is not possible, the platform is stuck with the
>> >> loops based delays, and their inherent variability and inaccuracy.
>> >>
>> >> These platforms have been tested with such a setup over many years.
>> >> They work even with udelay() having this behaviour, because it's a
>> >> known issue and drivers cater for it in ways that I've already covered
>> >> in my many previous emails to you.
>> >>
>> >> These issues are known.  They've been known for the last 15 odd years.
>> >
>> > So you've known for umpteen years that fixing loop-based delays is
>> > intractable, yet you wrote:
>> >
>> >> udelay() needs to offer a consistent interface so that drivers know
>> >> what to expect no matter what the implementation is.  Making one
>> >> implementation conform to your ideas while leaving the other
>> >> implementations with other expectations is a recipe for bugs.
>> >>
>> >> If you really want to do this, fix the loops_per_jiffy implementation
>> >> as well so that the consistency is maintained.
>> >
>> > In other words, "I'll consider your patch as soon as Hell freezes over".
>> >
>> > Roger that. I'll drop the subject then.
>>
>> Presumably, though, you could introduce a new API like:
>>
>>   udelay_atleast()
>>
>> That was guaranteed to delay at least the given number of
>> microseconds.  Unlike the current udelay(), the new udelay_atleast()
>> wouldn't really try that hard to get a delay that's approximately the
>> one requested, it would just guarantee not to ever delay _less_ than
>> the amount requested.
>
> I look forward to reviewing your implementation.

It's unlikely I'll post a patch in the near term since this isn't
presenting me with a big problem right now.  Mostly I saw Marc's patch
and thought it would be a good patch to land and I knew this type of
thing had bitten me in the past.

One happy result of this whole discussion, though, is that you now
sound as if you'll be happy the next time someone brings this up since
you're looking forward to reviewing an implementation.  That's a nice
change from the original statement questioning why someone was asking
about this again.  :)

As I've expressed in the past, IMHO fixing the timer based delays is
still a sane thing to do even without also fixing all the loop-based
implementations.  As someone said earlier in this thread, all the old
platforms using loop-based delays are working great for the platforms
they support and there's no reason to mess with them, but that doesn't
mean we shouldn't fix the problems that are easy to fix.


-Doug