From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCH 2/2] tools/virtio: make barriers stronger. Date: Thu, 07 Mar 2013 14:48:24 +1100 Message-ID: <87a9qfyinr.fsf@rustcorp.com.au> References: <1362491468-16681-1-git-send-email-sjur.brandeland@stericsson.com> <871ubtcezb.fsf@rustcorp.com.au> <87y5e1b03l.fsf@rustcorp.com.au> <87vc95b019.fsf@rustcorp.com.au> <20130306102017.GB16921@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20130306102017.GB16921@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: "Michael S. Tsirkin" Cc: sjur@brendeland.net, Linus Walleij , Erwan Yvin , virtualization@lists.linux-foundation.org, sjur.brandeland@stericsson.com List-Id: virtualization@lists.linuxfoundation.org "Michael S. Tsirkin" 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 >> >> 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