From: Peter Zijlstra <peterz@infradead•org>
To: "Paul E. McKenney" <paulmck@linux•vnet.ibm.com>
Cc: Boqun Feng <boqun.feng@gmail•com>,
linux-kernel@vger•kernel.org, linuxppc-dev@lists•ozlabs.org,
Ingo Molnar <mingo@kernel•org>,
Benjamin Herrenschmidt <benh@kernel•crashing.org>,
Paul Mackerras <paulus@samba•org>,
Michael Ellerman <mpe@ellerman•id.au>,
Thomas Gleixner <tglx@linutronix•de>,
Will Deacon <will.deacon@arm•com>,
Waiman Long <waiman.long@hp•com>,
Davidlohr Bueso <dave@stgolabs•net>,
stable@vger•kernel.org
Subject: Re: [PATCH tip/locking/core v4 1/6] powerpc: atomic: Make *xchg and *cmpxchg a full barrier
Date: Wed, 14 Oct 2015 23:04:19 +0200 [thread overview]
Message-ID: <20151014210419.GY3604@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20151014201916.GB3910@linux.vnet.ibm.com>
On Wed, Oct 14, 2015 at 01:19:17PM -0700, Paul E. McKenney wrote:
> Suppose we have something like the following, where "a" and "x" are both
> initially zero:
>
> CPU 0 CPU 1
> ----- -----
>
> WRITE_ONCE(x, 1); WRITE_ONCE(a, 2);
> r3 = xchg(&a, 1); smp_mb();
> r3 = READ_ONCE(x);
>
> If xchg() is fully ordered, we should never observe both CPUs'
> r3 values being zero, correct?
>
> And wouldn't this be represented by the following litmus test?
>
> PPC SB+lwsync-RMW2-lwsync+st-sync-leading
> ""
> {
> 0:r1=1; 0:r2=x; 0:r3=3; 0:r10=0 ; 0:r11=0; 0:r12=a;
> 1:r1=2; 1:r2=x; 1:r3=3; 1:r10=0 ; 1:r11=0; 1:r12=a;
> }
> P0 | P1 ;
> stw r1,0(r2) | stw r1,0(r12) ;
> lwsync | sync ;
> lwarx r11,r10,r12 | lwz r3,0(r2) ;
> stwcx. r1,r10,r12 | ;
> bne Fail0 | ;
> mr r3,r11 | ;
> Fail0: | ;
> exists
> (0:r3=0 /\ a=2 /\ 1:r3=0)
>
> I left off P0's trailing sync because there is nothing for it to order
> against in this particular litmus test. I tried adding it and verified
> that it has no effect.
>
> Am I missing something here? If not, it seems to me that you need
> the leading lwsync to instead be a sync.
So the scenario that would fail would be this one, right?
a = x = 0
CPU0 CPU1
r3 = load_locked (&a);
a = 2;
sync();
r3 = x;
x = 1;
lwsync();
if (!store_cond(&a, 1))
goto again
Where we hoist the load way up because lwsync allows this.
I always thought this would fail because CPU1's store to @a would fail
the store_cond() on CPU0 and we'd do the 'again' thing, re-issuing the
load and now seeing the new value (2).
> Of course, if I am not missing something, then this applies also to the
> value-returning RMW atomic operations that you pulled this pattern from.
> If so, it would seem that I didn't think through all the possibilities
> back when PPC_ATOMIC_EXIT_BARRIER moved to sync... In fact, I believe
> that I worried about the RMW atomic operation acting as a barrier,
> but not as the load/store itself. :-/
AARGH64 does something very similar; it does something like:
ll
...
sc-release
mb
Which I assumed worked for the same reason, any change to the variable
would fail the sc, and we go for round 2, now observing the new value.
next prev parent reply other threads:[~2015-10-14 21:04 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-14 15:55 [PATCH tip/locking/core v4 0/6] atomics: powerpc: Implement relaxed/acquire/release variants of some atomics Boqun Feng
2015-10-14 15:55 ` [PATCH tip/locking/core v4 1/6] powerpc: atomic: Make *xchg and *cmpxchg a full barrier Boqun Feng
2015-10-14 20:19 ` Paul E. McKenney
2015-10-14 21:04 ` Peter Zijlstra [this message]
2015-10-14 21:44 ` Paul E. McKenney
2015-10-15 0:53 ` Boqun Feng
2015-10-15 1:22 ` Boqun Feng
2015-10-15 3:07 ` Paul E. McKenney
2015-10-15 3:07 ` Paul E. McKenney
2015-10-15 4:48 ` Boqun Feng
2015-10-15 16:30 ` Paul E. McKenney
2015-10-19 0:19 ` Boqun Feng
2015-10-15 3:11 ` Boqun Feng
2015-10-15 3:33 ` Paul E. McKenney
2015-10-15 10:35 ` Will Deacon
2015-10-15 14:40 ` Boqun Feng
2015-10-15 14:50 ` Will Deacon
2015-10-15 16:29 ` Paul E. McKenney
2015-10-15 15:42 ` Paul E. McKenney
2015-10-15 14:49 ` Boqun Feng
2015-10-15 16:16 ` Paul E. McKenney
2015-10-20 7:15 ` Boqun Feng
2015-10-20 9:21 ` Peter Zijlstra
2015-10-20 21:28 ` Paul E. McKenney
2015-10-21 8:18 ` Peter Zijlstra
2015-10-21 19:36 ` Paul E. McKenney
2015-10-26 2:06 ` Boqun Feng
2015-10-26 2:20 ` Michael Ellerman
2015-10-26 8:55 ` Boqun Feng
2015-10-26 3:20 ` Paul Mackerras
2015-10-26 8:58 ` Boqun Feng
2015-10-21 8:45 ` Boqun Feng
2015-10-21 19:35 ` Paul E. McKenney
2015-10-21 19:48 ` Peter Zijlstra
2015-10-22 12:07 ` Boqun Feng
2015-10-24 10:26 ` Peter Zijlstra
2015-10-24 11:53 ` Boqun Feng
2015-10-25 13:14 ` Boqun Feng
2015-10-14 15:55 ` [PATCH tip/locking/core v4 2/6] atomics: Add test for atomic operations with _relaxed variants Boqun Feng
2015-10-14 15:55 ` [PATCH tip/locking/core v4 3/6] atomics: Allow architectures to define their own __atomic_op_* helpers Boqun Feng
2015-10-14 15:55 ` [PATCH tip/locking/core v4 4/6] powerpc: atomic: Implement atomic{, 64}_*_return_* variants Boqun Feng
2015-10-14 15:56 ` [PATCH tip/locking/core v4 5/6] powerpc: atomic: Implement xchg_* and atomic{, 64}_xchg_* variants Boqun Feng
2015-10-14 15:56 ` [PATCH tip/locking/core v4 6/6] powerpc: atomic: Implement cmpxchg{, 64}_* and atomic{, 64}_cmpxchg_* variants Boqun Feng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151014210419.GY3604@twins.programming.kicks-ass.net \
--to=peterz@infradead$(echo .)org \
--cc=benh@kernel$(echo .)crashing.org \
--cc=boqun.feng@gmail$(echo .)com \
--cc=dave@stgolabs$(echo .)net \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mingo@kernel$(echo .)org \
--cc=mpe@ellerman$(echo .)id.au \
--cc=paulmck@linux$(echo .)vnet.ibm.com \
--cc=paulus@samba$(echo .)org \
--cc=stable@vger$(echo .)kernel.org \
--cc=tglx@linutronix$(echo .)de \
--cc=waiman.long@hp$(echo .)com \
--cc=will.deacon@arm$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox