[ovs-dev] [PATCH 2/3] lib/ovs-rcu: Evaluate parameters before ovsrcu_set and ovsrcu_init.

Jarno Rajahalme jrajahalme at nicira.com
Tue Jun 3 00:32:15 UTC 2014


ovsrcu_set() and ovsrcu_init() look like functions, so users are
justified in expecting the arguments to be evalueated before any of
the body of ovsrcu_set or ovsrcu_init().

With ovs-atomic-pthreads, a fallback ovs-atomics implementation used
when no C11 atomics are available or with GCC older than 4.0, the
prior definitions led to nested mutex locking when the 'VALUE'
argument was an atomic_read().  This is resolved by ensuring function
argument semantics for the parameters.  For non-GCC compilers this is
done with an inline function.  GCC implementation of ovs-rcu does not
fix the RCU variable type, so a GCC macro needs to be used instead.

Reported-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 lib/ovs-rcu.h |   25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index c3e06ad..e27dfad 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -131,6 +131,20 @@
     CONST_CAST(TYPE, ovsrcu_get__(TYPE, VAR, memory_order_consume))
 #define ovsrcu_get_protected(TYPE, VAR) \
     CONST_CAST(TYPE, ovsrcu_get__(TYPE, VAR, memory_order_relaxed))
+
+/* 'VALUE' may be an atomic operation, which must be evaluated before
+ * any of the body of the atomic_store_explicit.  Since the type of
+ * 'VAR' is not fixed, we cannot use an inline function to get
+ * function semantics for this. */
+#define ovsrcu_set__(VAR, VALUE, ORDER)                         \
+    ({                                                          \
+        typeof(VAR) var__ = (VAR);                              \
+        typeof(VALUE) value__ = (VALUE);                        \
+        memory_order order__ = (ORDER);                         \
+                                                                \
+        atomic_store_explicit(&var__->p, value__, order__);     \
+        (void *) 0;                                             \
+    })
 #else  /* not GNU C */
 struct ovsrcu_pointer { ATOMIC(void *) p; };
 #define OVSRCU_TYPE(TYPE) struct ovsrcu_pointer
@@ -147,6 +161,13 @@ ovsrcu_get__(const struct ovsrcu_pointer *pointer, memory_order order)
     CONST_CAST(TYPE, ovsrcu_get__(VAR, memory_order_consume))
 #define ovsrcu_get_protected(TYPE, VAR) \
     CONST_CAST(TYPE, ovsrcu_get__(VAR, memory_order_relaxed))
+
+static inline void ovsrcu_set__(struct ovsrcu_pointer *pointer,
+                                const void *value,
+                                memory_order order)
+{
+    atomic_store_explicit(&pointer->p, CONST_CAST(void *, value), order);
+}
 #endif
 
 /* Writes VALUE to the RCU-protected pointer whose address is VAR.
@@ -154,13 +175,13 @@ ovsrcu_get__(const struct ovsrcu_pointer *pointer, memory_order order)
  * Users require external synchronization (e.g. a mutex).  See "Usage" above
  * for an example. */
 #define ovsrcu_set(VAR, VALUE) \
-    atomic_store_explicit(&(VAR)->p, VALUE, memory_order_release)
+    ovsrcu_set__(VAR, VALUE, memory_order_release)
 
 /* This can be used for initializing RCU pointers before any readers can
  * see them.  A later ovsrcu_set() needs to make the bigger structure this
  * is part of visible to the readers. */
 #define ovsrcu_init(VAR, VALUE) \
-    atomic_store_explicit(&(VAR)->p, VALUE, memory_order_relaxed)
+    ovsrcu_set__(VAR, VALUE, memory_order_relaxed)
 
 /* Calls FUNCTION passing ARG as its pointer-type argument following the next
  * grace period.  See "Usage" above for example.  */
-- 
1.7.10.4




More information about the dev mailing list