On Thu, Dec 01, 2011 at 12:58:59PM +1030, Rusty Russell wrote:

On Thu, 1 Dec 2011 01:13:07 +0200, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

For x86, stores into memory are ordered. So I think that yes, smp_XXX
can be selected at compile time.

So let's forget the virtio strangeness for a minute,

Hmm, we got away with light barriers because we knew we were not
*really* talking to a device.  But now with virtio-mmio, turns out we
are :)

You think virtio-mmio this issue too?  It's reported on remoteproc...

I'm really tempted to revert d57ed95 for 3.2, and we can revisit this
optimization later if it proves worthwhile.

Thoughts?
Rusty. 

Generally it does seem the best we can do for 3.2.

Given it's rc3, I'd be a bit wary of introducing regressions - I'll try
to find some real setups (as in - not my laptop) to run some benchmarks
on, to verify there's no major problem.
I hope I can report on this in about a week from now - want to hold onto this meanwhile?

Further, if we do revert, need to remember to apply the following
beforehand, to avoid breaking virtio tool:

tools/virtio: implement mandatory barriers for x86

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 68b8b8d..1bf0e80 100644
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h

@@ -172,11 +172,18 @@ struct virtqueue {
 #define MODULE_LICENSE(__MODULE_LICENSE_value) \
 	const char *__MODULE_LICENSE_name = __MODULE_LICENSE_value
 
 #define CONFIG_SMP
 
 #if defined(__i386__) || defined(__x86_64__)
 #define barrier() asm volatile("" ::: "memory")
 #define mb() __sync_synchronize()
+#if defined(__i386__)
+#define wmb() mb()
+#define rmb() mb()
+#else
+#define wmb() asm volatile("sfence" ::: "memory")
+#define rmb() asm volatile("lfence" ::: "memory")
+#endif
 
 #define smp_mb()	mb()
 # define smp_rmb()	barrier()

-- 
MST