On Fri, Sep 9, 2011 at 10:34 AM, Paul E. McKenney
[off-list ref] wrote:

On Fri, Sep 09, 2011 at 10:23:33AM -0700, Olof Johansson wrote:

quoted

[+linuxppc-dev]

On Tue, Sep 6, 2011 at 11:00 AM, Paul E. McKenney
[off-list ref] wrote:

quoted

The trailing isync/lwsync in PowerPC value-returning atomics needs
to be a sync in order to provide the required ordering properties.
The leading lwsync/eieio can remain, as the remainder of the required
ordering guarantees are provided by the atomic instructions: Any
reordering will cause the stwcx to fail, which will result in a retry.

Admittedly, my powerpc barrier memory is starting to fade, but isn't
isync sufficient here? It will make sure all instructions before it
have retired, and will restart any speculative/issued instructions
beyond it.

lwsync not being sufficient makes sense since a load can overtake it.

As I understand it, although isync waits for the prior stwcx to execute,
it does not guarantee that the corresponding store is visible to all
processors before any following loads.

Ah yes, combined with brushing up on the semantics in
memory-barriers.txt, this sounds reasonable to me.

quoted

quoted

diff --git a/arch/powerpc/include/asm/synch.h b/arch/powerpc/include/a=

sm/synch.h

quoted

quoted

index d7cab44..4d97fbe 100644

--- a/arch/powerpc/include/asm/synch.h
+++ b/arch/powerpc/include/asm/synch.h

@@ -37,11 +37,7 @@ static inline void isync(void)

=A0#endif

=A0#ifdef CONFIG_SMP
-#define __PPC_ACQUIRE_BARRIER =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=

 =A0 =A0 =A0\

quoted

quoted

- =A0 =A0 =A0 START_LWSYNC_SECTION(97); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 \

quoted

quoted

- =A0 =A0 =A0 isync; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =

=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\

quoted

quoted

- =A0 =A0 =A0 MAKE_LWSYNC_SECTION_ENTRY(97, __lwsync_fixup);
-#define PPC_ACQUIRE_BARRIER =A0 =A0"\n" stringify_in_c(__PPC_ACQUIRE_=

BARRIER)

quoted

quoted

+#define PPC_ACQUIRE_BARRIER =A0 =A0"\n" stringify_in_c(sync;)

This can just be done as "\n\tsync\n" instead of the stringify stuff.

That does sound a bit more straightforward, now that you mention it. =A0;=

-)

With that change, I'm:

Acked-by: Olof Johansson <redacted>

But at least Ben or Anton should sign off on it too.


-Olof