"Michael S. Tsirkin" <mst@xxxxxxxxxx> writes:
> On Wed, Mar 06, 2013 at 03:54:42PM +1100, Rusty Russell wrote:
>> In the coming vringh_test, we share an mmap with another userspace process
>> for testing. This requires real barriers.
>>
>> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
>>
>> diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
>> index aff61e1..7a63693 100644
>> --- a/tools/virtio/asm/barrier.h
>> +++ b/tools/virtio/asm/barrier.h
>> @@ -3,8 +3,8 @@
>> #define mb() __sync_synchronize()
>>
>> #define smp_mb() mb()
>> -# define smp_rmb() barrier()
>> -# define smp_wmb() barrier()
>> +# define smp_rmb() mb()
>> +# define smp_wmb() mb()
>> /* Weak barriers should be used. If not - it's a bug */
>> # define rmb() abort()
>> # define wmb() abort()
>
> Hmm this seems wrong on x86 which has strong order in hardware.
> It should not matter whether the other side is a userspace
> process or a kernel thread.
Actually, this code is completely generic now, though overkill for x86 smp_wmb():
Interestingly, when I try defining them, 32-bit x86 slows down (it seems
that gcc is using "lock orl $0x0,(%esp)" for __sync_synchronize()).:
On my 32-bit laptop: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz
Before:
Wall time:1.660000-1.790000(1.682500)
After:
Wall time:1.930000-3.620000(1.960625)
64 bit it's a win:
On 2.6.32-358.el6.x86_64, gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-3), Intel(R) Xeon(R) CPU E5620 @ 2.40GHz:
Before:
real 0m2.937000-8.339000(3.123979)s
user 0m2.811000-8.233000(2.954813)s
sys 0m0.052000-0.154000(0.089396)s
After:
real 0m2.559000-2.936000(2.726729)s
user 0m2.397000-2.651000(2.506396)s
sys 0m0.055000-0.152000(0.090667)s
Raw performance doesn't really matter, of course, but it's tempting to
use these asm barriers for __x86_64__, and use __sync_synchronize()
everywhere for everyone else.
Thoughts?
Rusty.
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index 7a63693..8de720a 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,11 +1,12 @@
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
-#define mb() __sync_synchronize()
-#define smp_mb() mb()
-# define smp_rmb() mb()
-# define smp_wmb() mb()
+#define smp_mb() asm volatile("mfence":::"memory")
+#define smp_rmb() asm volatile("lfence":::"memory")
+#define smp_wmb() asm volatile("sfence" ::: "memory")
+
/* Weak barriers should be used. If not - it's a bug */
+# define mb() abort()
# define rmb() abort()
# define wmb() abort()
#else
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
