"Michael S. Tsirkin" <[email protected]> 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 <[email protected]>
>> 
>> 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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to