lkml.org 
[lkml]   [2015]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [tip:locking/core] locking/pvqspinlock: Replace xchg() by the more descriptive set_mb()
On Mon, May 11, 2015 at 10:50:42AM -0700, Linus Torvalds wrote:
> On Mon, May 11, 2015 at 7:54 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Hmm, so I looked at the set_mb() definitions and I figure we want to do
> > something like the below, right?
>
> I don't think you need to do this for the non-smp cases.

Well, its the store tearing thing again, we use WRITE_ONCE() in
smp_store_release() for the same reason. We want it to be a single
store.

> The whole
> thing is about smp memory ordering, so on UP you don't even need the
> WRITE_ONCE(), much less a barrier.

No, we actually need both still on UP.

Imagine the following sequence:

for (;;) {
set_current_state(TASK_KILLABLE);
if (cond)
break;

schedule();
}
__set_current_state(TASK_RUNNING);

vs

<IRQ>
wake_up_process(p);

As we know, set_current_state() is set_mb(), and thus will look like:

current->state = TASK_KILLABLE;
smp_mb();
if (cond)
break;

So without the WRITE_ONCE() we can get store tearing, and suppose our
compiler is insane and translates the store into 4 byte stores.

current->state[0] = TASK_UNINTERRUPTIBLE;
current->state[1] = TASK_WAKEKILL >> 8;
current->state[2] = 0;
current->state[3] = 0;

The obvious fail here is to get the wakeup interrupt between [0] and
[1].

current->state[0] = TASK_UNINTERRUPTIBLE;

<IRQ>
wake_up_process(p);
p->state = TASK_RUNNING;

current->state[1] = TASK_WAKEKILL >> 8;
current->state[2] = 0;
current->state[3] = 0;

With the end result that ->state == TASK_WAKEKILL, from which we'll not
wake up unless killed.

Similarly, without the barrier(), our friendly compiler is allowed to
do:

if (cond)
break
current->state = TASK_KILLABLE;
schedule();

Which we all know to be broken.

So no, set_mb() (or smp_store_mb()) very much does need the WRITE_ONCE()
and a barrier() on UP.


\
 
 \ /
  Last update: 2015-05-12 11:21    [W:0.065 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site