udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[chpoph]

If ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is
used for udelay, then __loop_delay and __loop_const_udelay is used to
delay a specific time interval.  but in delay-loop.S, loops_per_jiffy
(not per cpu data) is used to calculate the number of loops. in SMP
system, udelay delays the wrong time interval if two cpus running at
different frequency.


In arch/arm/lib/delay.c arm_delay_ops is set to arm_delay_ops.
/*
 * 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,
};

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[Russell King - ARM Linux]

On Sat, Mar 16, 2013 at 12:08:54AM +0800, chpoph wrote:
> If ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is
> used for udelay, then __loop_delay and __loop_const_udelay is used to
> delay a specific time interval.  but in delay-loop.S, loops_per_jiffy
> (not per cpu data) is used to calculate the number of loops. in SMP
> system, udelay delays the wrong time interval if two cpus running at
> different frequency.

We don't support different CPUs running at different frequencies with
the delay loop.  Sorry.

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[chpoph]

On Sat, Mar 16, 2013 at 2:14 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> We don't support different CPUs running at different frequencies with
> the delay loop.  Sorry.

Does it means that a timer-based delay implementation must be used to
get an accurate delay in SMP. I think it should print a warning
message if the CPU delay loop is used in SMP.  In my system, the wrong
delay interval fluctuated with CPU frequencies caused a control
problem.

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[Will Deacon]

On Sat, Mar 16, 2013 at 03:32:43AM +0000, chpoph wrote:
> On Sat, Mar 16, 2013 at 2:14 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > We don't support different CPUs running at different frequencies with
> > the delay loop.  Sorry.
> 
> Does it means that a timer-based delay implementation must be used to
> get an accurate delay in SMP. I think it should print a warning
> message if the CPU delay loop is used in SMP.  In my system, the wrong
> delay interval fluctuated with CPU frequencies caused a control
> problem.

I've been playing around with loops_per_jiffy recently, in an attempt to
clean up the cpufreq scaling code so that the SMP-ness is in core code,
rather than being duplicated by every architecture:

  git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git lpj

With those changes, it's pretty easy to get different delays depending on
the current CPU, but it would require preempt_{enable,disable} calls around
the delay, which I haven't convinced myself about.

Do you actually have an ARM platform that can scale the CPU frequencies
independently?

Will

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[Russell King - ARM Linux]

On Sun, Mar 17, 2013 at 08:05:43PM +0000, Will Deacon wrote:
> On Sat, Mar 16, 2013 at 03:32:43AM +0000, chpoph wrote:
> > On Sat, Mar 16, 2013 at 2:14 AM, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> > > We don't support different CPUs running at different frequencies with
> > > the delay loop.  Sorry.
> > 
> > Does it means that a timer-based delay implementation must be used to
> > get an accurate delay in SMP. I think it should print a warning
> > message if the CPU delay loop is used in SMP.  In my system, the wrong
> > delay interval fluctuated with CPU frequencies caused a control
> > problem.
> 
> I've been playing around with loops_per_jiffy recently, in an attempt to
> clean up the cpufreq scaling code so that the SMP-ness is in core code,
> rather than being duplicated by every architecture:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git lpj
> 
> With those changes, it's pretty easy to get different delays depending on
> the current CPU, but it would require preempt_{enable,disable} calls around
> the delay, which I haven't convinced myself about.

Exactly, and that's why I said what I said.  If you start doing that,
then you might as well turn kernel preemption off altogether, because
the delays will impact your system latency.

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[chpoph]

 On Sun, Mar 17, 2013 at 08:05:43PM +0000, Will Deacon wrote:
>Do you actually have an ARM platform that can scale the CPU frequencies
independently?

Yes,  my smart phone use Qualcomm's 8x25 and 8064 platform, which can
scale the CPU frequencies independently. I test the delay loop, the
phone can't get accurate delayed time intervals. I think there are
four reasons: the task scheduling, IRQ handling, memory accessing, and
udelay may be called between CPU frequency changing and
loops_per_jiffy changing. The system's performance decreased greatly
after adding preempt_{enable, disable}.

I used the current timer to delay a time period at last, the timer's
frequency is constant.

Re: udelay function delays the wrong time interval in multiprocessor system, if ARCH_HAS_READ_CURRENT_TIMER is not defined and on current timer is used.

[Russell King - ARM Linux]

On Mon, Mar 18, 2013 at 10:40:56PM +0800, chpoph wrote:
>  On Sun, Mar 17, 2013 at 08:05:43PM +0000, Will Deacon wrote:
> >Do you actually have an ARM platform that can scale the CPU frequencies
> independently?
> 
> Yes,  my smart phone use Qualcomm's 8x25 and 8064 platform, which can
> scale the CPU frequencies independently. I test the delay loop, the
> phone can't get accurate delayed time intervals.

That is expected.  udelay() is only approximate.  Sometimes, it will give
you a slightly shorter delay than asked for, or if preempted, it can give
you a much longer delay - because it has no idea how long it has been
preempted for.

This is why in a SMP system it is much better to use a timer-based udelay()
implementation which is more independent of these effects.