Re: [patch 1/2] powerpc: rmb fix

On Tue, Aug 21, 2007 at 09:43:17PM +0200, Segher Boessenkool wrote:

quoted

quoted

#define mb()   __asm__ __volatile__ ("sync" : : : "memory")
-#define rmb()  __asm__ __volatile__ (__stringify(LWSYNC) : : : 
"memory")
+#define rmb()  __asm__ __volatile__ ("sync" : : : "memory")
#define wmb()  __asm__ __volatile__ ("sync" : : : "memory")
#define read_barrier_depends()  do { } while(0)

@@ -42,7 +42,7 @@

#ifdef __KERNEL__
#ifdef CONFIG_SMP
#define smp_mb()	mb()
-#define smp_rmb()	rmb()
+#define smp_rmb()	__asm__ __volatile__ (__stringify(LWSYNC) : : : 
"memory")
#define smp_wmb()	eieio()
#define smp_read_barrier_depends()	read_barrier_depends()
#else

I had to think about this one for awhile.  It looks at first glance to 
be the right
thing to do.  But I do wonder how long rmb() has been lwsync

Since the {ppc,ppc64} -> powerpc merge.

quoted

and if as a practical matter that has caused any problems?

It has not as far as I know.

quoted

If this isn't causing any problems maybe there
is some loigic we are overlooking?

The I/O accessor functions enforce the necessary ordering
already I believe.

Ah, it looks like you might be right, IO should appear to go in-order, in
which case the rmb() would simply need to order cacheable loads. Interesting
way to do things... are drivers simply not up to scratch enough to allow
out of order IO?

Anyway, this raises another question -- if IO accessors have the right
ordering, why is wmb() not an lwsync as well? There appears to be many
more wmb() calls than rmb()...

Thanks,
Nick