On Fri, Mar 14, 2014 at 01:36:15AM -0700, Andy Zhou wrote:
> Acked-by: Andy Zhou <az...@nicira.com>
Thanks.
> On Tue, Mar 11, 2014 at 1:56 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> > RCU allows multiple threads to read objects in parallel without any
> > performance penalty. The following commit will introduce the first use.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > + * Use ovsrcu_set() to write an RCU-protected pointer and
> > ovsrcu_postpone() to
> > + * free the previous data. If more than one thread can write the
> > pointer, then
> > + * some form of external synchronization, e.g. a mutex, is needed to
> > prevent
> > + * writers from interfering with one another. For example, to write the
> > + * pointer variable declared above while safely freeing the old value:
> > + *
> > + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> > + *
> > + * static void
> > + * free_flow(struct flow *flow)
> > + * {
> > + * free(flow);
> > + * }
> > + *
> > + * void
> > + * change_flow(struct flow *new_flow)
> > + * {
> > + * ovs_mutex_lock(&mutex);
> > + * ovsrcu_postpone(free_flow, ovsrcu_get(struct flow *, &flowp));
> > + * ovsrcu_set(&flowp, new_flow);
> > + * ovs_mutex_unlock(&mutex);
> >
> I assume the mutex lock and unlock should have embeded memory barrier
> operations to make sure all previous writes are visble. If true, then
> ovsrcu_get() and ovsrcu_set() does not seem necessary in this example. They
> are correct though.
They are correct, but not really necessary, as you say. How about the
following incremental:
diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
index 6721273..710870a 100644
--- a/lib/ovs-rcu.h
+++ b/lib/ovs-rcu.h
@@ -98,7 +98,8 @@
* change_flow(struct flow *new_flow)
* {
* ovs_mutex_lock(&mutex);
- * ovsrcu_postpone(free_flow, ovsrcu_get(struct flow *, &flowp));
+ * ovsrcu_postpone(free_flow,
+ * ovsrcu_get_protected(struct flow *, &flowp));
* ovsrcu_set(&flowp, new_flow);
* ovs_mutex_unlock(&mutex);
* }
@@ -118,33 +119,44 @@
*
* struct flow *flow = ovsrcu_get(struct flow *, flowp);
*
+ * If the pointer variable is currently protected against change (because
+ * the current thread holds a mutex that protects it), ovsrcu_get_protected()
+ * may be used instead. Only on the Alpha architecture is this likely to
+ * generate different code, but it may be useful documentation.
+ *
* (With GNU C or Clang, you get a compiler error if TYPE is wrong; other
* compilers will merrily carry along accepting the wrong type.)
*/
#if __GNUC__
#define OVSRCU_TYPE(TYPE) struct { ATOMIC(TYPE) p; }
-#define ovsrcu_get__(TYPE, VAR) \
+#define ovsrcu_get__(TYPE, VAR, ORDER) \
({ \
TYPE value__; \
\
atomic_read_explicit(CONST_CAST(ATOMIC(TYPE) *, &(VAR)->p), \
- &value__, memory_order_consume); \
+ &value__, ORDER); \
\
value__; \
})
-#define ovsrcu_get(TYPE, VAR) CONST_CAST(TYPE, ovsrcu_get__(TYPE, VAR))
+#define ovsrcu_get(TYPE, VAR) \
+ 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))
#else /* not GNU C */
typedef struct ovsrcu_pointer { ATOMIC(void *) p; };
#define OVSRCU_TYPE(TYPE) struct ovsrcu_pointer
static inline void *
-ovsrcu_get__(const struct ovsrcu_pointer *pointer)
+ovsrcu_get__(const struct ovsrcu_pointer *pointer, memory_order order)
{
void *value;
atomic_read_explicit(&CONST_CAST(struct ovsrcu_pointer *, pointer)->p,
- &value, memory_order_consume);
+ &value, order);
return value;
}
-#define ovsrcu_get(TYPE, VAR) CONST_CAST(TYPE, ovsrcu_get__(VAR))
+#define ovsrcu_get(TYPE, VAR) \
+ CONST_CAST(TYPE, ovsrcu_get__(VAR, memory_order_consume))
+#define ovsrcu_get_protected(TYPE, VAR) \
+ CONST_CAST(TYPE, ovsrcu_get__(VAR, memory_order_relaxed))
#endif
/* Writes VALUE to the RCU-protected pointer whose address is VAR.
> > +/* Calls FUNCTION passing ARG as its pointer-type argument following the
> > next
> > + * grace period. See "Usage" above for example. */
> > +void ovsrcu_postpone__(void (*function)(void *aux), void *aux);
> > +#define ovsrcu_postpone(FUNCTION, ARG) \
> > + ((void) sizeof((FUNCTION)(ARG), 1),
>
> I suppose the idea of using sizeof is for type checking. It is not obvious
> to me why "1" required.
It's because 'function' has to have a void return type, so
sizeof(function()) is sizeof(void), which yields a warning.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
