Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1524972 > unrolled thread

spin_lock behavior with ARM64 big.Little/HMP

Started byVikram Mulukutla <markivx@codeaurora.org>
First post2016-11-18 03:30 +0100
Last post2016-11-21 16:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  spin_lock behavior with ARM64 big.Little/HMP Vikram Mulukutla <markivx@codeaurora.org> - 2016-11-18 03:30 +0100
    Re: spin_lock behavior with ARM64 big.Little/HMP Sudeep Holla <sudeep.holla@arm.com> - 2016-11-18 11:40 +0100
      Re: spin_lock behavior with ARM64 big.Little/HMP Vikram Mulukutla <markivx@codeaurora.org> - 2016-11-18 21:30 +0100
        Re: spin_lock behavior with ARM64 big.Little/HMP Sudeep Holla <sudeep.holla@arm.com> - 2016-11-21 16:30 +0100

#1524972 — spin_lock behavior with ARM64 big.Little/HMP

FromVikram Mulukutla <markivx@codeaurora.org>
Date2016-11-18 03:30 +0100
Subjectspin_lock behavior with ARM64 big.Little/HMP
Message-ID<sEH69-RN-3@gated-at.bofh.it>
Hello,

This isn't really a bug report, but just a description of a 
frequency/IPC
dependent behavior that I'm curious if we should worry about. The 
behavior
is exposed by questionable design so I'm leaning towards don't-care.

Consider these threads running in parallel on two ARM64 CPUs running 
mainline
Linux:

(Ordering of lines between the two columns does not indicate a sequence 
of
execution. Assume flag=0 initially.)

LittleARM64_CPU @ 300MHz (e.g.A53)   |  BigARM64_CPU @ 1.5GHz (e.g. A57)
-------------------------------------+----------------------------------
spin_lock_irqsave(s)                 |  local_irq_save()
/* critical section */
flag = 1                             |  spin_lock(s)
spin_unlock_irqrestore(s)            |  while (!flag) {
                                      |      spin_unlock(s)
                                      |      cpu_relax();
                                      |      spin_lock(s)
                                      |  }
                                      |  spin_unlock(s)
                                      |  local_irq_restore()

I see a livelock occurring where the LittleCPU is never able to acquire 
the
lock, and the BigCPU is stuck forever waiting on 'flag' to be set.

Even with ticket spinlocks, this bit of code can cause a livelock (or 
very
long delays) if BigCPU runs fast enough. Afaics this can only happen if 
the
LittleCPU is unable to put its ticket in the queue (i.e, increment the 
next
field) since the store-exclusive keeps failing.

The problem is not present on SMP, and is mitigated by adding enough
additional clock cycles between the unlock and lock in the loop running
on the BigCPU. On big.Little, if both threads are scheduled on the same
cluster within the same clock domain, the problem is avoided.

Now the infinite loop may seem like questionable design but the problem
isn't entirely hypothetical; if BigCPU calls hrtimer_cancel with
interrupts disabled, this scenario can result if the hrtimer is about
to run on a littleCPU. It's of course possible that there's just enough
intervening code for the problem to not occur. At the very least it 
seems
that loops like the one running in the BigCPU above should come with a
WARN_ON(irqs_disabled) or with a sufficient udelay() instead of the 
cpu_relax.

Thoughts?

Thanks,
Vikram

[toc] | [next] | [standalone]


#1525164

FromSudeep Holla <sudeep.holla@arm.com>
Date2016-11-18 11:40 +0100
Message-ID<sEOKm-5WQ-9@gated-at.bofh.it>
In reply to#1524972
Hi Vikram,

On 18/11/16 02:22, Vikram Mulukutla wrote:
> Hello,
>
> This isn't really a bug report, but just a description of a frequency/IPC
> dependent behavior that I'm curious if we should worry about. The behavior
> is exposed by questionable design so I'm leaning towards don't-care.
>
> Consider these threads running in parallel on two ARM64 CPUs running
> mainline
> Linux:
>

Are you seeing this behavior with the mainline kernel on any platforms
as we have a sort of workaround for this ?

> (Ordering of lines between the two columns does not indicate a sequence of
> execution. Assume flag=0 initially.)
>
> LittleARM64_CPU @ 300MHz (e.g.A53)   |  BigARM64_CPU @ 1.5GHz (e.g. A57)
> -------------------------------------+----------------------------------
> spin_lock_irqsave(s)                 |  local_irq_save()
> /* critical section */
> flag = 1                             |  spin_lock(s)
> spin_unlock_irqrestore(s)            |  while (!flag) {
>                                      |      spin_unlock(s)
>                                      |      cpu_relax();
>                                      |      spin_lock(s)
>                                      |  }
>                                      |  spin_unlock(s)
>                                      |  local_irq_restore()
>
> I see a livelock occurring where the LittleCPU is never able to acquire the
> lock, and the BigCPU is stuck forever waiting on 'flag' to be set.
>

Yes we saw this issue 3 years back on TC2 which has A7(with lowest
frequency of 300MHz IIRC) and A15(with 1.2 GHz). We were observing that
inter-cluster events are missed since the two clusters are operating at
different frequencies (details below).

The hardware recommendation is that there should be glue logic between
the two clusters which captures events from one cluster and replays then
on the other if its operating at a different frequency.

Generally EVENTO from cluster 1 is connected to the EVENTI of the
cluster 2 and vice versa. The only extra logic required is the double
synchronizer in the receiving clock domain.

This issue arise in reality if the synchronizer is missing and different
CPUs hold EVENTO for different clock cycles.

However there was a different requirement to implement timer event
stream in Linux for some user-space locking and that indirectly help to
resolve the issue on TC2. That event stream feature is enabled by
default in Linux and should fix the issue and hence I asked you if you
still see that issue.

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]


#1525705

FromVikram Mulukutla <markivx@codeaurora.org>
Date2016-11-18 21:30 +0100
Message-ID<sEXXk-3vd-13@gated-at.bofh.it>
In reply to#1525164
Hi Sudeep,

Thanks for taking a look!

On 2016-11-18 02:30, Sudeep Holla wrote:
> Hi Vikram,
> 
> On 18/11/16 02:22, Vikram Mulukutla wrote:
>> Hello,
>> 
>> This isn't really a bug report, but just a description of a 
>> frequency/IPC
>> dependent behavior that I'm curious if we should worry about. The 
>> behavior
>> is exposed by questionable design so I'm leaning towards don't-care.
>> 
>> Consider these threads running in parallel on two ARM64 CPUs running
>> mainline
>> Linux:
>> 
> 
> Are you seeing this behavior with the mainline kernel on any platforms
> as we have a sort of workaround for this ?
> 

If I understand that workaround correctly, the ARM timer event stream is 
used
to periodically wake up CPUs that are waiting in WFE, is that right? I 
think
my scenario below may be different because LittleCPU doesn't actually 
wait
on a WFE event in the loop that is trying to increment lock->next, i.e. 
it's
stuck in the following loop:

         ARM64_LSE_ATOMIC_INSN(
         /* LL/SC */
"       prfm    pstl1strm, %3\n"
"1:     ldaxr   %w0, %3\n"
"       add     %w1, %w0, %w5\n"
"       stxr    %w2, %w1, %3\n"
"       cbnz    %w2, 1b\n",


I have been testing internal platforms; I'll try to test on something
available publicly that's b.L. In any case, the timer event stream was 
enabled
when I tried this out.

>> (Ordering of lines between the two columns does not indicate a 
>> sequence of
>> execution. Assume flag=0 initially.)
>> 
>> LittleARM64_CPU @ 300MHz (e.g.A53)   |  BigARM64_CPU @ 1.5GHz (e.g. 
>> A57)
>> -------------------------------------+----------------------------------
>> spin_lock_irqsave(s)                 |  local_irq_save()
>> /* critical section */
>> flag = 1                             |  spin_lock(s)
>> spin_unlock_irqrestore(s)            |  while (!flag) {
>>                                      |      spin_unlock(s)
>>                                      |      cpu_relax();
>>                                      |      spin_lock(s)
>>                                      |  }
>>                                      |  spin_unlock(s)
>>                                      |  local_irq_restore()
>> 

[...]

Thanks,
Vikram

[toc] | [prev] | [next] | [standalone]


#1526790

FromSudeep Holla <sudeep.holla@arm.com>
Date2016-11-21 16:30 +0100
Message-ID<sFYHJ-2Tm-17@gated-at.bofh.it>
In reply to#1525705

On 18/11/16 20:22, Vikram Mulukutla wrote:
>
> Hi Sudeep,
>
> Thanks for taking a look!
>
> On 2016-11-18 02:30, Sudeep Holla wrote:
>> Hi Vikram,
>>
>> On 18/11/16 02:22, Vikram Mulukutla wrote:
>>> Hello,
>>>
>>> This isn't really a bug report, but just a description of a
>>> frequency/IPC
>>> dependent behavior that I'm curious if we should worry about. The
>>> behavior
>>> is exposed by questionable design so I'm leaning towards don't-care.
>>>
>>> Consider these threads running in parallel on two ARM64 CPUs running
>>> mainline
>>> Linux:
>>>
>>
>> Are you seeing this behavior with the mainline kernel on any platforms
>> as we have a sort of workaround for this ?
>>
>
> If I understand that workaround correctly, the ARM timer event stream is
> used
> to periodically wake up CPUs that are waiting in WFE, is that right? I
> think
> my scenario below may be different because LittleCPU doesn't actually wait
> on a WFE event in the loop that is trying to increment lock->next, i.e.
> it's
> stuck in the following loop:
>
>         ARM64_LSE_ATOMIC_INSN(
>         /* LL/SC */
> "       prfm    pstl1strm, %3\n"
> "1:     ldaxr   %w0, %3\n"
> "       add     %w1, %w0, %w5\n"
> "       stxr    %w2, %w1, %3\n"
> "       cbnz    %w2, 1b\n",
>

Interesting. Just curious if this is r0p0/p1 A53 ? If so, is the errata
819472 enabled ?

-- 
Regards,
Sudeep

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web