Re: [PATCH RESEND v3 1/6] powerpc: atomic: Make *xchg and *cmpxchg a full barrier

On Wed, Oct 14, 2015 at 11:10:00AM +1100, Michael Ellerman wrote:

On Mon, 2015-10-12 at 22:30 +0800, Boqun Feng wrote:

quoted

According to memory-barriers.txt, xchg, cmpxchg and their atomic{,64}_
versions all need to imply a full barrier, however they are now just
RELEASE+ACQUIRE, which is not a full barrier.

So replace PPC_RELEASE_BARRIER and PPC_ACQUIRE_BARRIER with
PPC_ATOMIC_ENTRY_BARRIER and PPC_ATOMIC_EXIT_BARRIER in
__{cmp,}xchg_{u32,u64} respectively to guarantee a full barrier
semantics of atomic{,64}_{cmp,}xchg() and {cmp,}xchg().

This patch is a complement of commit b97021f85517 ("powerpc: Fix
atomic_xxx_return barrier semantics").

Cc: <redacted> # 3.4.y-
Signed-off-by: Boqun Feng <redacted>
---
 arch/powerpc/include/asm/cmpxchg.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Hi Boqun,

Hello, Michael

Thanks for fixing this. In future you should send a patch like this as a
separate patch. I've not been paying attention to it because I assumed it was

Got it. However, here is the thing, in previous version, this fix
depends on some of other patches in this patchset. So to make this fix
applied cleanly, I reorder my patchset to put this patch first, and the
result is that some of other patches in this patchset depends on
this(they need to remove code modified by this patch).

So I guess I'd better to stop Cc stable for this one, and wait until
this patchset merged and send a separate patch for -stable tree. Does
that work for you? I think this is what Peter want to suggests me to do
when he asked me about this, right, Peter?

part of your full series and was still under discussion like the other patches.

I don't think we've seen any crashes caused by this have we? So I guess I'll

No, we haven't seen any.

put it in next to let it get some wider testing rather than sending it straight
to Linus.

Good idea, thank you ;-)

To be clear you're doing:

quoted

-	PPC_RELEASE_BARRIER
+	PPC_ATOMIC_ENTRY_BARRIER

Which is correct but doesn't actually change anything at the moment, because
both macros turn into LWSYNC.

On the other hand:

quoted

-	PPC_ACQUIRE_BARRIER
+	PPC_ATOMIC_EXIT_BARRIER

Is changing an isync (which is then patched to lwsync on some cpus), with a sync.

These macros are introduced by commit b97021f85517 ("powerpc: Fix
atomic_xxx_return barrier semantics") to fix a similar problem, so I use
them to keep code similar.

Also I'm not clear what your stable line means:

quoted

Cc: <redacted> # 3.4.y-

Do you mean 3.4 and anything after? I usually write that as 3.4+, but I'm not
sure if that's the correct syntax either.

Quote from Documentation/stable_kernel_rules.txt:

"""
Also, some patches may have kernel version prerequisites.  This can be
specified in the following format in the sign-off area:

     Cc:  [off-list ref] # 3.3.x-

   The tag has the meaning of:
     git cherry-pick <this commit>

   For each "-stable" tree starting with the specified version.
"""

But yes, I have seen several people use like "3.4+", I'm not sure either

Regards,
Boqun