Re: [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems

From: Nicolas Pitre
Date: Thu May 08 2014 - 15:59:48 EST


On Thu, 8 May 2014, Doug Anderson wrote:

> Nicolas,
>
> On Thu, May 8, 2014 at 10:43 AM, Nicolas Pitre <nicolas.pitre@xxxxxxxxxx> wrote:
> > On Thu, 8 May 2014, Doug Anderson wrote:
>
> >> Longer delays aren't very good, but IMHO having some delays of 100 =>
> >> 1000 is better than having delays of 100 => 75. The former will cause
> >> mostly performance problems and the later will cause real correctness
> >> problems.
> >> I'm not saying that 100 => 1000 is good, it's just less bad.
> >
> > There might be some cases where precise timing is needed though.
> > I thought I came across one such case in the past but I can't remember
> > which.
>
> If precise timing is needed, shouldn't it be using ktime?

I meant "precise" in the sense that you might have to poke at some
hardware within some deadline e.g. set bit, wait 13us, clear bit, and
not exceed 15us between both bit events. Arguably this is best hangled
with actual FIQs (when they're available). But I don't have any actual
case of this to bring as example.

> >> I will make the argument that this patch makes things less broken
> >> overall on any systems that actually end up running this code, but if
> >> you want NAK it then it won't cause me any heartache. ;)
> >
> > What I insist on is for this issue to be solved using a stable counter
> > such a timer when available. It _is_ available on one of the target you
> > mentioned so that is the solution you should add to your tree.
>
> Yup, we're working on it.
>
>
> > Investigating a similar solution for your other target should be
> > preferred to hacking the udelay loop. This way you're guaranteed to
> > solve this problem fully.
>
> I have no other target in mind. I'm merely sending this up there just
> in case there is some cpufreq running ARM board that is SMP and has no
> timer-based udelay. Those are the only boards that could possibly be
> running this code anyway.
>
> I guess I would say that my patch is unhacking the this code. The
> code after my patch is simpler. I would perhaps argue that (ec971ea
> ARM: add cpufreq transiton notifier to adjust loops_per_jiffy for smp)
> should never have landed to begin with.

I would agree to qualify your patch as brokenness mitigation and that
might be better than nothing. But this should probably print a warning
to that effect.

Yet, wouldn't using ktime to implement udelay() a better solution in
that case? Suffice to make sure a clock source with microsecs
resolution is available.


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