Re: [PATCH 1/2] locking/xchg/alpha: Use smp_mb() in place of __ASM__MB

2018-02-22 Thread Paul E. McKenney
On Thu, Feb 22, 2018 at 10:24:29AM +0100, Andrea Parri wrote:
> Replace each occurrence of __ASM__MB with a (trailing) smp_mb() in
> xchg(), cmpxchg(), and remove the now unused __ASM__MB definitions;
> this improves readability, with no additional synchronization cost.
> 
> Suggested-by: Will Deacon 
> Signed-off-by: Andrea Parri 

I am a bit confused by the use of out-of-line branches to do a backwards
branch, but those were in place to start with.  Maybe the point is to
defeat backwards-branch prediction or some such.

Regardless...

Acked-by: Paul E. McKenney 

> Cc: Peter Zijlstra 
> Cc: Paul E. McKenney 
> Cc: Alan Stern 
> Cc: Ivan Kokshaysky 
> Cc: Matt Turner 
> Cc: Richard Henderson 
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  arch/alpha/include/asm/cmpxchg.h |  6 --
>  arch/alpha/include/asm/xchg.h| 16 
>  2 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/cmpxchg.h 
> b/arch/alpha/include/asm/cmpxchg.h
> index 46ebf14aed4e5..8a2b331e43feb 100644
> --- a/arch/alpha/include/asm/cmpxchg.h
> +++ b/arch/alpha/include/asm/cmpxchg.h
> @@ -6,7 +6,6 @@
>   * Atomic exchange routines.
>   */
> 
> -#define __ASM__MB
>  #define xchg(type, args...)  __xchg ## type ## _local(args)
>  #define cmpxchg(type, args...)   __cmpxchg ## type ## _local(args)
>  #include 
> @@ -33,10 +32,6 @@
>   cmpxchg_local((ptr), (o), (n)); \
>  })
> 
> -#ifdef CONFIG_SMP
> -#undef __ASM__MB
> -#define __ASM__MB"\tmb\n"
> -#endif
>  #undef xchg
>  #undef cmpxchg
>  #define xchg(type, args...)  __xchg ##type(args)
> @@ -64,7 +59,6 @@
>   cmpxchg((ptr), (o), (n));   \
>  })
> 
> -#undef __ASM__MB
>  #undef cmpxchg
> 
>  #endif /* _ALPHA_CMPXCHG_H */
> diff --git a/arch/alpha/include/asm/xchg.h b/arch/alpha/include/asm/xchg.h
> index e2660866ce972..e1facf6fc2446 100644
> --- a/arch/alpha/include/asm/xchg.h
> +++ b/arch/alpha/include/asm/xchg.h
> @@ -28,12 +28,12 @@ xchg(_u8, volatile char *m, unsigned long val)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%3)\n"
>   "   beq %2,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br  1b\n"
>   ".previous"
>   : "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
>   : "r" ((long)m), "1" (val) : "memory");
> + smp_mb();
> 
>   return ret;
>  }
> @@ -52,12 +52,12 @@ xchg(_u16, volatile short *m, unsigned long val)
>   "   or  %1,%2,%2\n"
>   "   stq_c   %2,0(%3)\n"
>   "   beq %2,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br  1b\n"
>   ".previous"
>   : "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
>   : "r" ((long)m), "1" (val) : "memory");
> + smp_mb();
> 
>   return ret;
>  }
> @@ -72,12 +72,12 @@ xchg(_u32, volatile int *m, unsigned long val)
>   "   bis $31,%3,%1\n"
>   "   stl_c %1,%2\n"
>   "   beq %1,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br 1b\n"
>   ".previous"
>   : "=" (val), "=" (dummy), "=m" (*m)
>   : "rI" (val), "m" (*m) : "memory");
> + smp_mb();
> 
>   return val;
>  }
> @@ -92,12 +92,12 @@ xchg(_u64, volatile long *m, unsigned long val)
>   "   bis $31,%3,%1\n"
>   "   stq_c %1,%2\n"
>   "   beq %1,2f\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "2: br 1b\n"
>   ".previous"
>   : "=" (val), "=" (dummy), "=m" (*m)
>   : "rI" (val), "m" (*m) : "memory");
> + smp_mb();
> 
>   return val;
>  }
> @@ -150,12 +150,12 @@ cmpxchg(_u8, volatile char *m, unsigned char old, 
> unsigned char new)
>   "   stq_c   %2,0(%4)\n"
>   "   beq %2,3f\n"
>   "2:\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "3: br  1b\n"
>   ".previous"
>   : "=" (prev), "=" (new), "=" (tmp), "=" (cmp), "=" (addr64)
>   : "r" ((long)m), "Ir" (old), "1" (new) : "memory");
> + smp_mb();
> 
>   return prev;
>  }
> @@ -177,12 +177,12 @@ cmpxchg(_u16, volatile short *m, unsigned short 
> old, unsigned short new)
>   "   stq_c   %2,0(%4)\n"
>   "   beq %2,3f\n"
>   "2:\n"
> - __ASM__MB
>   ".subsection 2\n"
>   "3: br  1b\n"
>   ".previous"
>   : "=" (prev), "=" (new), "=" (tmp), "=" (cmp), "=" (addr64)
>   : "r" ((long)m), "Ir" (old), "1" (new) : "memory");
> + smp_mb();
> 
>   return prev;
>  }
> @@ -200,12 +200,12 @@ cmpxchg(_u32, volatile int *m, int old, int new)
>   "   stl_c %1,%2\n"
>   "   

[PATCH 1/2] locking/xchg/alpha: Use smp_mb() in place of __ASM__MB

2018-02-22 Thread Andrea Parri
Replace each occurrence of __ASM__MB with a (trailing) smp_mb() in
xchg(), cmpxchg(), and remove the now unused __ASM__MB definitions;
this improves readability, with no additional synchronization cost.

Suggested-by: Will Deacon 
Signed-off-by: Andrea Parri 
Cc: Peter Zijlstra 
Cc: Paul E. McKenney 
Cc: Alan Stern 
Cc: Ivan Kokshaysky 
Cc: Matt Turner 
Cc: Richard Henderson 
Cc: linux-alpha@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
 arch/alpha/include/asm/cmpxchg.h |  6 --
 arch/alpha/include/asm/xchg.h| 16 
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/alpha/include/asm/cmpxchg.h b/arch/alpha/include/asm/cmpxchg.h
index 46ebf14aed4e5..8a2b331e43feb 100644
--- a/arch/alpha/include/asm/cmpxchg.h
+++ b/arch/alpha/include/asm/cmpxchg.h
@@ -6,7 +6,6 @@
  * Atomic exchange routines.
  */
 
-#define __ASM__MB
 #define xchg(type, args...)__xchg ## type ## _local(args)
 #define cmpxchg(type, args...) __cmpxchg ## type ## _local(args)
 #include 
@@ -33,10 +32,6 @@
cmpxchg_local((ptr), (o), (n)); \
 })
 
-#ifdef CONFIG_SMP
-#undef __ASM__MB
-#define __ASM__MB  "\tmb\n"
-#endif
 #undef xchg
 #undef cmpxchg
 #define xchg(type, args...)__xchg ##type(args)
@@ -64,7 +59,6 @@
cmpxchg((ptr), (o), (n));   \
 })
 
-#undef __ASM__MB
 #undef cmpxchg
 
 #endif /* _ALPHA_CMPXCHG_H */
diff --git a/arch/alpha/include/asm/xchg.h b/arch/alpha/include/asm/xchg.h
index e2660866ce972..e1facf6fc2446 100644
--- a/arch/alpha/include/asm/xchg.h
+++ b/arch/alpha/include/asm/xchg.h
@@ -28,12 +28,12 @@ xchg(_u8, volatile char *m, unsigned long val)
"   or  %1,%2,%2\n"
"   stq_c   %2,0(%3)\n"
"   beq %2,2f\n"
-   __ASM__MB
".subsection 2\n"
"2: br  1b\n"
".previous"
: "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
: "r" ((long)m), "1" (val) : "memory");
+   smp_mb();
 
return ret;
 }
@@ -52,12 +52,12 @@ xchg(_u16, volatile short *m, unsigned long val)
"   or  %1,%2,%2\n"
"   stq_c   %2,0(%3)\n"
"   beq %2,2f\n"
-   __ASM__MB
".subsection 2\n"
"2: br  1b\n"
".previous"
: "=" (ret), "=" (val), "=" (tmp), "=" (addr64)
: "r" ((long)m), "1" (val) : "memory");
+   smp_mb();
 
return ret;
 }
@@ -72,12 +72,12 @@ xchg(_u32, volatile int *m, unsigned long val)
"   bis $31,%3,%1\n"
"   stl_c %1,%2\n"
"   beq %1,2f\n"
-   __ASM__MB
".subsection 2\n"
"2: br 1b\n"
".previous"
: "=" (val), "=" (dummy), "=m" (*m)
: "rI" (val), "m" (*m) : "memory");
+   smp_mb();
 
return val;
 }
@@ -92,12 +92,12 @@ xchg(_u64, volatile long *m, unsigned long val)
"   bis $31,%3,%1\n"
"   stq_c %1,%2\n"
"   beq %1,2f\n"
-   __ASM__MB
".subsection 2\n"
"2: br 1b\n"
".previous"
: "=" (val), "=" (dummy), "=m" (*m)
: "rI" (val), "m" (*m) : "memory");
+   smp_mb();
 
return val;
 }
@@ -150,12 +150,12 @@ cmpxchg(_u8, volatile char *m, unsigned char old, 
unsigned char new)
"   stq_c   %2,0(%4)\n"
"   beq %2,3f\n"
"2:\n"
-   __ASM__MB
".subsection 2\n"
"3: br  1b\n"
".previous"
: "=" (prev), "=" (new), "=" (tmp), "=" (cmp), "=" (addr64)
: "r" ((long)m), "Ir" (old), "1" (new) : "memory");
+   smp_mb();
 
return prev;
 }
@@ -177,12 +177,12 @@ cmpxchg(_u16, volatile short *m, unsigned short old, 
unsigned short new)
"   stq_c   %2,0(%4)\n"
"   beq %2,3f\n"
"2:\n"
-   __ASM__MB
".subsection 2\n"
"3: br  1b\n"
".previous"
: "=" (prev), "=" (new), "=" (tmp), "=" (cmp), "=" (addr64)
: "r" ((long)m), "Ir" (old), "1" (new) : "memory");
+   smp_mb();
 
return prev;
 }
@@ -200,12 +200,12 @@ cmpxchg(_u32, volatile int *m, int old, int new)
"   stl_c %1,%2\n"
"   beq %1,3f\n"
"2:\n"
-   __ASM__MB
".subsection 2\n"
"3: br 1b\n"
".previous"
: "="(prev), "="(cmp), "=m"(*m)
: "r"((long) old), "r"(new), "m"(*m) : "memory");
+   smp_mb();
 
return prev;
 }
@@ -223,12 +223,12 @@ cmpxchg(_u64, volatile long *m, unsigned long old, 
unsigned long new)
"   stq_c %1,%2\n"
"   beq %1,3f\n"
"2:\n"
-