Re: [PATCH] spinlock: fix atomic and out of order execution

[Stephen Hemminger <stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org>]

On Sat, 21 Dec 2013 00:37:36 +0100
Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> wrote:

> From: Damien Millescamps <damien.millescamps-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> Add lock prefix before xchg instructions in order to be atomic
> and flush speculative values to ensure effective execution order
> (as an acquire barrier).
> 
> MPLOCKED is a "lock" in multicore case.
> 
> Signed-off-by: Damien Millescamps <damien.millescamps-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> ---
>  lib/librte_eal/common/include/rte_spinlock.h |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_spinlock.h b/lib/librte_eal/common/include/rte_spinlock.h
> index f7a245a..8edb971 100644
> --- a/lib/librte_eal/common/include/rte_spinlock.h
> +++ b/lib/librte_eal/common/include/rte_spinlock.h
> @@ -51,6 +51,7 @@
>  extern "C" {
>  #endif
>  
> +#include <rte_atomic.h>
>  #include <rte_lcore.h>
>  #ifdef RTE_FORCE_INTRINSICS
>  #include <rte_common.h>
> @@ -93,7 +94,7 @@ rte_spinlock_lock(rte_spinlock_t *sl)
>  	int lock_val = 1;
>  	asm volatile (
>  			"1:\n"
> -			"xchg %[locked], %[lv]\n"
> +			MPLOCKED "xchg %[locked], %[lv]\n"
>  			"test %[lv], %[lv]\n"
>  			"jz 3f\n"
>  			"2:\n"
> @@ -124,7 +125,7 @@ rte_spinlock_unlock (rte_spinlock_t *sl)
>  #ifndef RTE_FORCE_INTRINSICS
>  	int unlock_val = 0;
>  	asm volatile (
> -			"xchg %[locked], %[ulv]\n"
> +			MPLOCKED "xchg %[locked], %[ulv]\n"
>  			: [locked] "=m" (sl->locked), [ulv] "=q" (unlock_val)
>  			: "[ulv]" (unlock_val)
>  			: "memory");
> @@ -148,7 +149,7 @@ rte_spinlock_trylock (rte_spinlock_t *sl)
>  	int lockval = 1;
>  
>  	asm volatile (
> -			"xchg %[locked], %[lockval]"
> +			MPLOCKED "xchg %[locked], %[lockval]"
>  			: [locked] "=m" (sl->locked), [lockval] "=q" (lockval)
>  			: "[lockval]" (lockval)
>  			: "memory");

The locked prefix is required for xchg instruction.
The processor does it automatically.

http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html

"The XCHG (exchange) instruction swaps the contents of two operands. This instruction takes the place of three
MOV instructions and does not require a temporary location to save the contents of one operand location while the
other is being loaded. When a memory operand is used with the XCHG instruction, the processor’s LOCK signal is
automatically asserted.