lib/ovs-rcu: Evaluate parameters before ovsrcu_set.

ovsrcu_set() looks like a function, so users are justified in expecting
the arguments to be evaluated before any of the body of ovsrcu_set().

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@valinux.co.jp>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp>

diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index 6b809f1..110129c 100644 (file)
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -142,6 +142,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) ovsrcu_var = (VAR);                                 \
+        typeof(VALUE) ovsrcu_value = (VALUE);                           \
+        memory_order ovsrcu_order = (ORDER);                            \
+                                                                        \
+        atomic_store_explicit(&ovsrcu_var->p, ovsrcu_value, ovsrcu_order); \
+        (void *) 0;                                                     \
+    })
 #else  /* not GNU C */
 struct ovsrcu_pointer { ATOMIC(void *) p; };
 #define OVSRCU_TYPE(TYPE) struct ovsrcu_pointer
@@ -157,6 +171,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.
@@ -164,7 +185,7 @@ 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)
 
 /* Calls FUNCTION passing ARG as its pointer-type argument following the next
  * grace period.  See "Usage" above for example.  */