From 450cbdd0125cfa5d7bbf9e2a6b6961cc48d29730 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Fri, 27 Oct 2017 19:14:31 +0300 Subject: locking/x86: Use LOCK ADD for smp_mb() instead of MFENCE MFENCE appears to be way slower than a locked instruction - let's use LOCK ADD unconditionally, as we always did on old 32-bit. Performance testing results: perf stat -r 10 -- ./virtio_ring_0_9 --sleep --host-affinity 0 --guest-affinity 0 Before: 0.922565990 seconds time elapsed ( +- 1.15% ) After: 0.578667024 seconds time elapsed ( +- 1.21% ) i.e. about ~60% faster. Just poking at SP would be the most natural, but if we then read the value from SP, we get a false dependency which will slow us down. This was noted in this article: http://shipilev.net/blog/2014/on-the-fence-with-dependencies/ And is easy to reproduce by sticking a barrier in a small non-inline function. So let's use a negative offset - which avoids this problem since we build with the red zone disabled. For userspace, use an address just below the redzone. The one difference between LOCK ADD and MFENCE is that LOCK ADD does not affect CLFLUSH, previous patches converted all uses of CLFLUSH to call mb(), such that changes to smp_mb() won't affect it. Update mb/rmb/wmb() on 32-bit to use the negative offset, too, for consistency. As a follow-up, it might be worth considering switching users of CLFLUSH to another API (e.g. clflush_mb()?) - we will then be able to convert mb() to smp_mb() again. Also arguably, GCC should switch to use LOCK ADD for __sync_synchronize(). This might be worth pursuing separately. Suggested-by: Andy Lutomirski Signed-off-by: Michael S. Tsirkin Acked-by: Peter Zijlstra Acked-by: Thomas Gleixner Cc: Andrew Morton Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Denys Vlasenko Cc: H. Peter Anvin Cc: Josh Poimboeuf Cc: Linus Torvalds Cc: Paul E. McKenney Cc: qemu-devel@nongnu.org Cc: virtualization@lists.linux-foundation.org Link: http://lkml.kernel.org/r/1509118355-4890-1-git-send-email-mst@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/include/asm/barrier.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'arch') diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h index 01727dbc294a..7fb336210e1b 100644 --- a/arch/x86/include/asm/barrier.h +++ b/arch/x86/include/asm/barrier.h @@ -12,11 +12,11 @@ */ #ifdef CONFIG_X86_32 -#define mb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "mfence", \ +#define mb() asm volatile(ALTERNATIVE("lock; addl $0,-4(%%esp)", "mfence", \ X86_FEATURE_XMM2) ::: "memory", "cc") -#define rmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "lfence", \ +#define rmb() asm volatile(ALTERNATIVE("lock; addl $0,-4(%%esp)", "lfence", \ X86_FEATURE_XMM2) ::: "memory", "cc") -#define wmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "sfence", \ +#define wmb() asm volatile(ALTERNATIVE("lock; addl $0,-4(%%esp)", "sfence", \ X86_FEATURE_XMM2) ::: "memory", "cc") #else #define mb() asm volatile("mfence":::"memory") @@ -31,7 +31,11 @@ #endif #define dma_wmb() barrier() -#define __smp_mb() mb() +#ifdef CONFIG_X86_32 +#define __smp_mb() asm volatile("lock; addl $0,-4(%%esp)" ::: "memory", "cc") +#else +#define __smp_mb() asm volatile("lock; addl $0,-4(%%rsp)" ::: "memory", "cc") +#endif #define __smp_rmb() dma_rmb() #define __smp_wmb() barrier() #define __smp_store_mb(var, value) do { (void)xchg(&var, value); } while (0) -- cgit v1.2.3-58-ga151