diff mbox series

[v2,1/2] xen/arm: Remove cmpxchg_local() and drop _mb from the other helpers

Message ID 20200911160622.19721-2-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series Introduce cmpxchg64() and guest_cmpxchg64() | expand

Commit Message

Julien Grall Sept. 11, 2020, 4:06 p.m. UTC 
From: Julien Grall <jgrall@amazon.com>

The current set of helpers are quite confusing to follow as the naming
is not very consistent.

Given that cmpxchg_local() is not used in Xen, drop it completely.
Furthermore, adopt a naming with _mb so all names are now consistent.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/arm32/cmpxchg.h | 31 ++++++-----------------
 xen/include/asm-arm/arm64/cmpxchg.h | 38 +++++++----------------------
 xen/include/asm-arm/guest_atomics.h |  6 ++---
 3 files changed, 19 insertions(+), 56 deletions(-)

Comments

Stefano Stabellini Sept. 14, 2020, 11:59 p.m. UTC | #1 
On Fri, 11 Sep 2020, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> The current set of helpers are quite confusing to follow as the naming
> is not very consistent.
> 
> Given that cmpxchg_local() is not used in Xen, drop it completely.
> Furthermore, adopt a naming with _mb so all names are now consistent.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/include/asm-arm/arm32/cmpxchg.h | 31 ++++++-----------------
>  xen/include/asm-arm/arm64/cmpxchg.h | 38 +++++++----------------------
>  xen/include/asm-arm/guest_atomics.h |  6 ++---
>  3 files changed, 19 insertions(+), 56 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm32/cmpxchg.h b/xen/include/asm-arm/arm32/cmpxchg.h
> index 0770f272ee99..3ef1e5c63276 100644
> --- a/xen/include/asm-arm/arm32/cmpxchg.h
> +++ b/xen/include/asm-arm/arm32/cmpxchg.h
> @@ -112,23 +112,12 @@ static always_inline unsigned long __cmpxchg(volatile void *ptr,
>  					     unsigned long new,
>  					     int size)
>  {
> +	smp_mb();
>  	if (!__int_cmpxchg(ptr, &old, new, size, false, 0))
>  		ASSERT_UNREACHABLE();
> -
> -	return old;
> -}
> -
> -static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
> -                                                unsigned long old,
> -                                                unsigned long new, int size)
> -{
> -	unsigned long ret;
> -
> -	smp_mb();
> -	ret = __cmpxchg(ptr, old, new, size);
>  	smp_mb();
>  
> -	return ret;
> +	return old;
>  }
>  
>  /*
> @@ -141,11 +130,11 @@ static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
>   * The helper will return true when the update has succeeded (i.e no
>   * timeout) and false if the update has failed.
>   */
> -static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
> -					       unsigned long *old,
> -					       unsigned long new,
> -					       int size,
> -					       unsigned int max_try)
> +static always_inline bool __cmpxchg_timeout(volatile void *ptr,
> +					    unsigned long *old,
> +					    unsigned long new,
> +					    int size,
> +					    unsigned int max_try)
>  {
>  	bool ret;
>  
> @@ -157,12 +146,6 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
>  }
>  
>  #define cmpxchg(ptr,o,n)						\
> -	((__typeof__(*(ptr)))__cmpxchg_mb((ptr),			\
> -					  (unsigned long)(o),		\
> -					  (unsigned long)(n),		\
> -					  sizeof(*(ptr))))
> -
> -#define cmpxchg_local(ptr,o,n)						\
>  	((__typeof__(*(ptr)))__cmpxchg((ptr),				\
>  				       (unsigned long)(o),		\
>  				       (unsigned long)(n),		\
> diff --git a/xen/include/asm-arm/arm64/cmpxchg.h b/xen/include/asm-arm/arm64/cmpxchg.h
> index fc5c60f0bd74..f4a8c1ccba80 100644
> --- a/xen/include/asm-arm/arm64/cmpxchg.h
> +++ b/xen/include/asm-arm/arm64/cmpxchg.h
> @@ -125,23 +125,12 @@ static always_inline unsigned long __cmpxchg(volatile void *ptr,
>  					     unsigned long new,
>  					     int size)
>  {
> +	smp_mb();
>  	if (!__int_cmpxchg(ptr, &old, new, size, false, 0))
>  		ASSERT_UNREACHABLE();
> -
> -	return old;
> -}
> -
> -static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
> -						unsigned long old,
> -						unsigned long new, int size)
> -{
> -	unsigned long ret;
> -
> -	smp_mb();
> -	ret = __cmpxchg(ptr, old, new, size);
>  	smp_mb();
>  
> -	return ret;
> +	return old;
>  }
>  
>  /*
> @@ -154,11 +143,11 @@ static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
>   * The helper will return true when the update has succeeded (i.e no
>   * timeout) and false if the update has failed.
>   */
> -static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
> -					       unsigned long *old,
> -					       unsigned long new,
> -					       int size,
> -					       unsigned int max_try)
> +static always_inline bool __cmpxchg_timeout(volatile void *ptr,
> +					    unsigned long *old,
> +					    unsigned long new,
> +					    int size,
> +					    unsigned int max_try)
>  {
>  	bool ret;
>  
> @@ -173,17 +162,8 @@ static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
>  ({ \
>  	__typeof__(*(ptr)) __ret; \
>  	__ret = (__typeof__(*(ptr))) \
> -		__cmpxchg_mb((ptr), (unsigned long)(o), (unsigned long)(n), \
> -			     sizeof(*(ptr))); \
> -	__ret; \
> -})
> -
> -#define cmpxchg_local(ptr, o, n) \
> -({ \
> -	__typeof__(*(ptr)) __ret; \
> -	__ret = (__typeof__(*(ptr))) \
> -		__cmpxchg((ptr), (unsigned long)(o), \
> -			  (unsigned long)(n), sizeof(*(ptr))); \
> +		__cmpxchg((ptr), (unsigned long)(o), (unsigned long)(n), \
> +			  sizeof(*(ptr))); \
>  	__ret; \
>  })
>  
> diff --git a/xen/include/asm-arm/guest_atomics.h b/xen/include/asm-arm/guest_atomics.h
> index af27cc627bf3..20347849e56c 100644
> --- a/xen/include/asm-arm/guest_atomics.h
> +++ b/xen/include/asm-arm/guest_atomics.h
> @@ -96,14 +96,14 @@ static inline unsigned long __guest_cmpxchg(struct domain *d,
>  
>      perfc_incr(atomics_guest);
>  
> -    if ( __cmpxchg_mb_timeout(ptr, &oldval, new, size,
> -                              this_cpu(guest_safe_atomic_max)) )
> +    if ( __cmpxchg_timeout(ptr, &oldval, new, size,
> +                           this_cpu(guest_safe_atomic_max)) )
>          return oldval;
>  
>      perfc_incr(atomics_guest_paused);
>  
>      domain_pause_nosync(d);
> -    oldval = __cmpxchg_mb(ptr, old, new, size);
> +    oldval = __cmpxchg(ptr, old, new, size);
>      domain_unpause(d);
>  
>      return oldval;
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/xen/include/asm-arm/arm32/cmpxchg.h b/xen/include/asm-arm/arm32/cmpxchg.h
index 0770f272ee99..3ef1e5c63276 100644
--- a/xen/include/asm-arm/arm32/cmpxchg.h
+++ b/xen/include/asm-arm/arm32/cmpxchg.h
@@ -112,23 +112,12 @@  static always_inline unsigned long __cmpxchg(volatile void *ptr,
 					     unsigned long new,
 					     int size)
 {
+	smp_mb();
 	if (!__int_cmpxchg(ptr, &old, new, size, false, 0))
 		ASSERT_UNREACHABLE();
-
-	return old;
-}
-
-static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
-                                                unsigned long old,
-                                                unsigned long new, int size)
-{
-	unsigned long ret;
-
-	smp_mb();
-	ret = __cmpxchg(ptr, old, new, size);
 	smp_mb();
 
-	return ret;
+	return old;
 }
 
 /*
@@ -141,11 +130,11 @@  static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
  * The helper will return true when the update has succeeded (i.e no
  * timeout) and false if the update has failed.
  */
-static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
-					       unsigned long *old,
-					       unsigned long new,
-					       int size,
-					       unsigned int max_try)
+static always_inline bool __cmpxchg_timeout(volatile void *ptr,
+					    unsigned long *old,
+					    unsigned long new,
+					    int size,
+					    unsigned int max_try)
 {
 	bool ret;
 
@@ -157,12 +146,6 @@  static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
 }
 
 #define cmpxchg(ptr,o,n)						\
-	((__typeof__(*(ptr)))__cmpxchg_mb((ptr),			\
-					  (unsigned long)(o),		\
-					  (unsigned long)(n),		\
-					  sizeof(*(ptr))))
-
-#define cmpxchg_local(ptr,o,n)						\
 	((__typeof__(*(ptr)))__cmpxchg((ptr),				\
 				       (unsigned long)(o),		\
 				       (unsigned long)(n),		\
diff --git a/xen/include/asm-arm/arm64/cmpxchg.h b/xen/include/asm-arm/arm64/cmpxchg.h
index fc5c60f0bd74..f4a8c1ccba80 100644
--- a/xen/include/asm-arm/arm64/cmpxchg.h
+++ b/xen/include/asm-arm/arm64/cmpxchg.h
@@ -125,23 +125,12 @@  static always_inline unsigned long __cmpxchg(volatile void *ptr,
 					     unsigned long new,
 					     int size)
 {
+	smp_mb();
 	if (!__int_cmpxchg(ptr, &old, new, size, false, 0))
 		ASSERT_UNREACHABLE();
-
-	return old;
-}
-
-static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
-						unsigned long old,
-						unsigned long new, int size)
-{
-	unsigned long ret;
-
-	smp_mb();
-	ret = __cmpxchg(ptr, old, new, size);
 	smp_mb();
 
-	return ret;
+	return old;
 }
 
 /*
@@ -154,11 +143,11 @@  static always_inline unsigned long __cmpxchg_mb(volatile void *ptr,
  * The helper will return true when the update has succeeded (i.e no
  * timeout) and false if the update has failed.
  */
-static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
-					       unsigned long *old,
-					       unsigned long new,
-					       int size,
-					       unsigned int max_try)
+static always_inline bool __cmpxchg_timeout(volatile void *ptr,
+					    unsigned long *old,
+					    unsigned long new,
+					    int size,
+					    unsigned int max_try)
 {
 	bool ret;
 
@@ -173,17 +162,8 @@  static always_inline bool __cmpxchg_mb_timeout(volatile void *ptr,
 ({ \
 	__typeof__(*(ptr)) __ret; \
 	__ret = (__typeof__(*(ptr))) \
-		__cmpxchg_mb((ptr), (unsigned long)(o), (unsigned long)(n), \
-			     sizeof(*(ptr))); \
-	__ret; \
-})
-
-#define cmpxchg_local(ptr, o, n) \
-({ \
-	__typeof__(*(ptr)) __ret; \
-	__ret = (__typeof__(*(ptr))) \
-		__cmpxchg((ptr), (unsigned long)(o), \
-			  (unsigned long)(n), sizeof(*(ptr))); \
+		__cmpxchg((ptr), (unsigned long)(o), (unsigned long)(n), \
+			  sizeof(*(ptr))); \
 	__ret; \
 })
 
diff --git a/xen/include/asm-arm/guest_atomics.h b/xen/include/asm-arm/guest_atomics.h
index af27cc627bf3..20347849e56c 100644
--- a/xen/include/asm-arm/guest_atomics.h
+++ b/xen/include/asm-arm/guest_atomics.h
@@ -96,14 +96,14 @@  static inline unsigned long __guest_cmpxchg(struct domain *d,
 
     perfc_incr(atomics_guest);
 
-    if ( __cmpxchg_mb_timeout(ptr, &oldval, new, size,
-                              this_cpu(guest_safe_atomic_max)) )
+    if ( __cmpxchg_timeout(ptr, &oldval, new, size,
+                           this_cpu(guest_safe_atomic_max)) )
         return oldval;
 
     perfc_incr(atomics_guest_paused);
 
     domain_pause_nosync(d);
-    oldval = __cmpxchg_mb(ptr, old, new, size);
+    oldval = __cmpxchg(ptr, old, new, size);
     domain_unpause(d);
 
     return oldval;