atomic: use static inlines instead of macros

This is some coverity-inspired tidying.

Coverity has some grief analysing the call sites of atomic_read().  This is
believed to be a bug in Coverity itself when expanding the nested macros, but
there is no legitimate reason for it to be a macro in the first place.

This patch changes {,_}atomic_{read,set}() from being macros to being static
inline functions, thus gaining some type safety.

One issue which is not immediately obvious is that the non-atomic variants take
their atomic_t at a different level of indirection to the atomic variants.

This is not suitable for _atomic_set() (when used to initialise an atomic_t)
which is converted to take its parameter as a pointer.  One callsite of
_atomic_set() is updated, while the other two callsites are updated to
ATOMIC_INIT().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan<tim@xen.org>
Acked-by: Keir Fraser <keir@xen.org>
[For the arm bits:]
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/xen/common/domain.c b/xen/common/domain.c
index ad8a1b638a4cbd9c27ae935a98805f1f14e1d09d..b414a7d6bd2e7bd562a7498fda615698f7b571e4 100644 (file)
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -754,13 +754,12 @@ static void complete_domain_destroy(struct rcu_head *head)
 void domain_destroy(struct domain *d)
 {
     struct domain **pd;
-    atomic_t      old, new;
+    atomic_t old = ATOMIC_INIT(0);
+    atomic_t new = ATOMIC_INIT(DOMAIN_DESTROYED);
 
     BUG_ON(!d->is_dying);
 
     /* May be already destroyed, or get_domain() can race us. */
-    _atomic_set(old, 0);
-    _atomic_set(new, DOMAIN_DESTROYED);
     old = atomic_compareandswap(old, new, &d->refcnt);
     if ( _atomic_read(old) != 0 )
         return;

diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 2c92de92ad5bb1fb545d31481cbe25279bb55494..7d15fb0df7dd1556e301d79d0c7c1bd191f55ca7 100644 (file)
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -84,11 +84,25 @@ typedef struct { int counter; } atomic_t;
  * strex/ldrex monitor on some implementations. The reason we can use it for
  * atomic_set() is the clrex or dummy strex done on every exception return.
  */
-#define _atomic_read(v) ((v).counter)
-#define atomic_read(v)  (*(volatile int *)&(v)->counter)
+static inline int atomic_read(atomic_t *v)
+{
+    return *(volatile int *)&v->counter;
+}
+
+static inline int _atomic_read(atomic_t v)
+{
+    return v.counter;
+}
 
-#define _atomic_set(v,i) (((v).counter) = (i))
-#define atomic_set(v,i) (((v)->counter) = (i))
+static inline void atomic_set(atomic_t *v, int i)
+{
+    v->counter = i;
+}
+
+static inline void _atomic_set(atomic_t *v, int i)
+{
+    v->counter = i;
+}
 
 #if defined(CONFIG_ARM_32)
 # include <asm/arm32/atomic.h>

diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index e476ab5142f8750f69c8a4596e74f9b60670dfd0..89724639a4a083e76fd48f08837790e35d8cbcea 100644 (file)
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -66,21 +66,50 @@ typedef struct { int counter; } atomic_t;
 /**
  * atomic_read - read atomic variable
  * @v: pointer of type atomic_t
- * 
+ *
  * Atomically reads the value of @v.
  */
-#define _atomic_read(v)  ((v).counter)
-#define atomic_read(v)   read_atomic(&((v)->counter))
+static inline int atomic_read(atomic_t *v)
+{
+    return read_atomic(&v->counter);
+}
+
+/**
+ * _atomic_read - read atomic variable non-atomically
+ * @v atomic_t
+ *
+ * Non-atomically reads the value of @v
+ */
+static inline int _atomic_read(atomic_t v)
+{
+    return v.counter;
+}
+
 
 /**
  * atomic_set - set atomic variable
  * @v: pointer of type atomic_t
  * @i: required value
- * 
+ *
  * Atomically sets the value of @v to @i.
- */ 
-#define _atomic_set(v,i) (((v).counter) = (i))
-#define atomic_set(v,i)  write_atomic(&((v)->counter), (i))
+ */
+static inline void atomic_set(atomic_t *v, int i)
+{
+    write_atomic(&v->counter, i);
+}
+
+/**
+ * _atomic_set - set atomic variable non-atomically
+ * @v: pointer of type atomic_t
+ * @i: required value
+ *
+ * Non-atomically sets the value of @v to @i.
+ */
+static inline void _atomic_set(atomic_t *v, int i)
+{
+    v->counter = i;
+}
+
 
 /**
  * atomic_add - add integer to atomic variable

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 94c7db342ae360b4b266363371d4da8f8af7378a..a9257761bf9cb17a32c0a7813a1c3d1d779b262f 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -487,7 +487,7 @@ static always_inline int get_domain(struct domain *d)
         old = seen;
         if ( unlikely(_atomic_read(old) & DOMAIN_DESTROYED) )
             return 0;
-        _atomic_set(new, _atomic_read(old) + 1);
+        _atomic_set(&new, _atomic_read(old) + 1);
         seen = atomic_compareandswap(old, new, &d->refcnt);
     }
     while ( unlikely(_atomic_read(seen) != _atomic_read(old)) );