lkml.org 
[lkml]   [2010]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/5] kgdb: Use atomic operators which use barriers


On Fri, 2 Apr 2010, Jason Wessel wrote:
>
> A cpu_relax() does not mandate that there is an smp memory barrier.
> As a result on the arm smp architecture the kernel debugger can hang
> on entry from time to time, as shown by the kgdb regression tests.
>
> The solution is simply to use the atomic operators which include a
> proper smp memory barrier, instead of using atomic_set() and
> atomic_read().

Hmm. While I absolutely agree that 'cpu_relax()' does not imply a memory
barrier, I disagree that this change should be needed. If ARM has odd
semantics where it will never see changes in a busy loop, then ARM is
buggy, and that has _nothing_ to do with the Linux notion of memory
barriers.

The _whole_ point of "cpu_relax()" is to have busy loops. And the point of
busy loops is that they are waiting for something to change. So if this
loop:

> for_each_online_cpu(i) {
> - while (atomic_read(&cpu_in_kgdb[i]))
> + while (atomic_add_return(0, &cpu_in_kgdb[i]))
> cpu_relax();
> }

can somehow lock up because "cpu_relax()" doesn't work with an infinite
"while (atomic_read(..))" loop, then the ARM implementation of cpu_relax()
is buggy.

Here's a simple example of exactly these kinds of busy loops waiting for
something to change using cpu_relax() from generic kernel code:

ipc/mqueue.c- while (ewp->state == STATE_PENDING)
ipc/mqueue.c: cpu_relax();

ipc/msg.c- while (msg == NULL) {
ipc/msg.c: cpu_relax();

kernel/sched.c- while (task_is_waking(p))
kernel/sched.c: cpu_relax();

kernel/smp.c- while (data->flags & CSD_FLAG_LOCK)
kernel/smp.c: cpu_relax();

so I'd like to understand what the ARM issue is.

Does ARM have some broken cache coherency model where writes by other
CPU's _never_ show up unless the reading CPU does some memory sync thing?
If so, then cpu_relax() obviously does need to do that syncing
instruction.

And no, that does NOT mean that "cpu_relax()" has any memory barrier
semantics. All it means is that cpu_relax() obviously is some
architecture-specific way of saying "I'm in a busy loop, waiting for
something".

Linus


\
 
 \ /
  Last update: 2010-04-02 21:19    [W:0.102 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site