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

YAMAMOTO Takashi yamamoto at valinux.co.jp
Tue Jun 3 04:24:25 UTC 2014


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

evaluated

> 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__);     \

ovs-atomic-gcc4+ version of atomic_store_explicit also has
a variable named order__.  is it safe?

otherwise,
Acked-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>

YAMAMOTO Takashi

> +        (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
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list