Re: [ovs-dev] [PATCH] ovs-rcu: Add new ovsrcu_index type.

[Daniele Di Proietto]

Applied to master, thanks!



On 03/08/2016 15:00, "Jarno Rajahalme"  wrote:

>Looks good to me,
>
>Acked-by: Jarno Rajahalme 
>
>> On Aug 2, 2016, at 5:03 PM, Daniele Di Proietto  
>> wrote:
>> 
>> With RCU in Open vSwitch it's very easy to protect objects accessed by
>> a pointer, but sometimes a pointer is not available.
>> 
>> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
>> was used to access a vhost device with RCU semantics.  From DPDK 16.07
>> an integer id (which is an array index) is used to access a vhost
>> device.  Ideally, we want the exact same RCU semantics that we had for
>> the pointer, on the integer (atomicity, memory barriers, behaviour
>> around quiescent states)
>> 
>> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
>> implemented ovsrcu_index_*() functions should be used to access the
>> type.
>> 
>> Even though we say "Do not, in general, declare a typedef for a struct,
>> union, or enum.", I think we're not in the "general" case.
>> 
>> Signed-off-by: Daniele Di Proietto 
>> ---
>> lib/ovs-rcu.h | 84 
>> +++
>> 1 file changed, 84 insertions(+)
>> 
>> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
>> index dc75749..2887bb8 100644
>> --- a/lib/ovs-rcu.h
>> +++ b/lib/ovs-rcu.h
>> @@ -125,6 +125,36 @@
>>  * ovs_mutex_unlock();
>>  * }
>>  *
>> + * In some rare cases an object may not be addressable with a pointer, but 
>> only
>> + * through an array index (e.g. because it's provided by another library).  
>> It
>> + * is still possible to have RCU semantics by using the ovsrcu_index type.
>> + *
>> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
>> + *
>> + * ovsrcu_index port_id;
>> + *
>> + * void tx()
>> + * {
>> + * int id = ovsrcu_index_get(_id);
>> + * if (id == -1) {
>> + * return;
>> + * }
>> + * port_tx(id);
>> + * }
>> + *
>> + * void delete()
>> + * {
>> + * int id;
>> + *
>> + * ovs_mutex_lock();
>> + * id = ovsrcu_index_get_protected(_id);
>> + * ovsrcu_index_set(_id, -1);
>> + * ovs_mutex_unlock();
>> + *
>> + * ovsrcu_synchronize();
>> + * port_delete(id);
>> + * }
>> + *
>>  */
>> 
>> #include "compiler.h"
>> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void *aux), 
>> void *aux);
>>  (void) sizeof(*(ARG)), \
>>  ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
>> 
>> +/* An array index protected by RCU semantics.  This is an easier 
>> alternative to
>> + * an RCU protected pointer to a malloc'd int. */
>> +typedef struct { atomic_int v; } ovsrcu_index;
>> +
>> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order 
>> order)
>> +{
>> +int ret;
>> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
>> +return ret;
>> +}
>> +
>> +/* Returns the index contained in 'i'.  The returned value can be used until
>> + * the next grace period. */
>> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
>> +{
>> +return ovsrcu_index_get__(i, memory_order_consume);
>> +}
>> +
>> +/* Returns the index contained in 'i'.  This is an alternative to
>> + * ovsrcu_index_get() that can be used when there's no possible concurrent
>> + * writer. */
>> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
>> +{
>> +return ovsrcu_index_get__(i, memory_order_relaxed);
>> +}
>> +
>> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
>> +  memory_order order)
>> +{
>> +atomic_store_explicit(>v, value, order);
>> +}
>> +
>> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
>> + * used by readers until the next grace period. */
>> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
>> +{
>> +ovsrcu_index_set__(i, value, memory_order_release);
>> +}
>> +
>> +/* Writes the index 'value' in 'i'.  This is an alternative to
>> + * ovsrcu_index_set() that can be used when there's no possible concurrent
>> + * reader. */
>> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
>> +{
>> +ovsrcu_index_set__(i, value, memory_order_relaxed);
>> +}
>> +
>> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
>> no
>> + * concurrent readers. */
>> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
>> +{
>> +atomic_init(>v, value);
>> +}
>> +
>> /* Quiescent states. */
>> void ovsrcu_quiesce_start(void);
>> void ovsrcu_quiesce_end(void);
>> -- 
>> 2.8.1
>> 
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-rcu: Add new ovsrcu_index type.

[Jarno Rajahalme]

Looks good to me,

Acked-by: Jarno Rajahalme 

> On Aug 2, 2016, at 5:03 PM, Daniele Di Proietto  
> wrote:
> 
> With RCU in Open vSwitch it's very easy to protect objects accessed by
> a pointer, but sometimes a pointer is not available.
> 
> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
> was used to access a vhost device with RCU semantics.  From DPDK 16.07
> an integer id (which is an array index) is used to access a vhost
> device.  Ideally, we want the exact same RCU semantics that we had for
> the pointer, on the integer (atomicity, memory barriers, behaviour
> around quiescent states)
> 
> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
> implemented ovsrcu_index_*() functions should be used to access the
> type.
> 
> Even though we say "Do not, in general, declare a typedef for a struct,
> union, or enum.", I think we're not in the "general" case.
> 
> Signed-off-by: Daniele Di Proietto 
> ---
> lib/ovs-rcu.h | 84 +++
> 1 file changed, 84 insertions(+)
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..2887bb8 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -125,6 +125,36 @@
>  * ovs_mutex_unlock();
>  * }
>  *
> + * In some rare cases an object may not be addressable with a pointer, but 
> only
> + * through an array index (e.g. because it's provided by another library).  
> It
> + * is still possible to have RCU semantics by using the ovsrcu_index type.
> + *
> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> + *
> + * ovsrcu_index port_id;
> + *
> + * void tx()
> + * {
> + * int id = ovsrcu_index_get(_id);
> + * if (id == -1) {
> + * return;
> + * }
> + * port_tx(id);
> + * }
> + *
> + * void delete()
> + * {
> + * int id;
> + *
> + * ovs_mutex_lock();
> + * id = ovsrcu_index_get_protected(_id);
> + * ovsrcu_index_set(_id, -1);
> + * ovs_mutex_unlock();
> + *
> + * ovsrcu_synchronize();
> + * port_delete(id);
> + * }
> + *
>  */
> 
> #include "compiler.h"
> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void *aux), void 
> *aux);
>  (void) sizeof(*(ARG)), \
>  ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
> 
> +/* An array index protected by RCU semantics.  This is an easier alternative 
> to
> + * an RCU protected pointer to a malloc'd int. */
> +typedef struct { atomic_int v; } ovsrcu_index;
> +
> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order 
> order)
> +{
> +int ret;
> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +return ret;
> +}
> +
> +/* Returns the index contained in 'i'.  The returned value can be used until
> + * the next grace period. */
> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_consume);
> +}
> +
> +/* Returns the index contained in 'i'.  This is an alternative to
> + * ovsrcu_index_get() that can be used when there's no possible concurrent
> + * writer. */
> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
> +  memory_order order)
> +{
> +atomic_store_explicit(>v, value, order);
> +}
> +
> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
> + * used by readers until the next grace period. */
> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_release);
> +}
> +
> +/* Writes the index 'value' in 'i'.  This is an alternative to
> + * ovsrcu_index_set() that can be used when there's no possible concurrent
> + * reader. */
> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_relaxed);
> +}
> +
> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
> no
> + * concurrent readers. */
> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
> +{
> +atomic_init(>v, value);
> +}
> +
> /* Quiescent states. */
> void ovsrcu_quiesce_start(void);
> void ovsrcu_quiesce_end(void);
> -- 
> 2.8.1
> 

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-rcu: Add new ovsrcu_index type.

[Loftus, Ciara]

> 
> With RCU in Open vSwitch it's very easy to protect objects accessed by
> a pointer, but sometimes a pointer is not available.
> 
> One example is the vhost id for DPDK 16.07.  Until DPDK 16.04 a pointer
> was used to access a vhost device with RCU semantics.  From DPDK 16.07
> an integer id (which is an array index) is used to access a vhost
> device.  Ideally, we want the exact same RCU semantics that we had for
> the pointer, on the integer (atomicity, memory barriers, behaviour
> around quiescent states)
> 
> This commit implements a new type in ovs-rcu: ovsrcu_index. The newly
> implemented ovsrcu_index_*() functions should be used to access the
> type.
> 
> Even though we say "Do not, in general, declare a typedef for a struct,
> union, or enum.", I think we're not in the "general" case.
> 
> Signed-off-by: Daniele Di Proietto 
> ---
>  lib/ovs-rcu.h | 84
> ++
> +
>  1 file changed, 84 insertions(+)
> 
> diff --git a/lib/ovs-rcu.h b/lib/ovs-rcu.h
> index dc75749..2887bb8 100644
> --- a/lib/ovs-rcu.h
> +++ b/lib/ovs-rcu.h
> @@ -125,6 +125,36 @@
>   * ovs_mutex_unlock();
>   * }
>   *
> + * In some rare cases an object may not be addressable with a pointer, but
> only
> + * through an array index (e.g. because it's provided by another library).  
> It
> + * is still possible to have RCU semantics by using the ovsrcu_index type.
> + *
> + * static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> + *
> + * ovsrcu_index port_id;
> + *
> + * void tx()
> + * {
> + * int id = ovsrcu_index_get(_id);
> + * if (id == -1) {
> + * return;
> + * }
> + * port_tx(id);
> + * }
> + *
> + * void delete()
> + * {
> + * int id;
> + *
> + * ovs_mutex_lock();
> + * id = ovsrcu_index_get_protected(_id);
> + * ovsrcu_index_set(_id, -1);
> + * ovs_mutex_unlock();
> + *
> + * ovsrcu_synchronize();
> + * port_delete(id);
> + * }
> + *
>   */
> 
>  #include "compiler.h"
> @@ -213,6 +243,60 @@ void ovsrcu_postpone__(void (*function)(void
> *aux), void *aux);
>   (void) sizeof(*(ARG)), \
>   ovsrcu_postpone__((void (*)(void *))(FUNCTION), ARG))
> 
> +/* An array index protected by RCU semantics.  This is an easier alternative
> to
> + * an RCU protected pointer to a malloc'd int. */
> +typedef struct { atomic_int v; } ovsrcu_index;
> +
> +static inline int ovsrcu_index_get__(const ovsrcu_index *i, memory_order
> order)
> +{
> +int ret;
> +atomic_read_explicit(CONST_CAST(atomic_int *, >v), , order);
> +return ret;
> +}
> +
> +/* Returns the index contained in 'i'.  The returned value can be used until
> + * the next grace period. */
> +static inline int ovsrcu_index_get(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_consume);
> +}
> +
> +/* Returns the index contained in 'i'.  This is an alternative to
> + * ovsrcu_index_get() that can be used when there's no possible
> concurrent
> + * writer. */
> +static inline int ovsrcu_index_get_protected(const ovsrcu_index *i)
> +{
> +return ovsrcu_index_get__(i, memory_order_relaxed);
> +}
> +
> +static inline void ovsrcu_index_set__(ovsrcu_index *i, int value,
> +  memory_order order)
> +{
> +atomic_store_explicit(>v, value, order);
> +}
> +
> +/* Writes the index 'value' in 'i'.  The previous value of 'i' may still be
> + * used by readers until the next grace period. */
> +static inline void ovsrcu_index_set(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_release);
> +}
> +
> +/* Writes the index 'value' in 'i'.  This is an alternative to
> + * ovsrcu_index_set() that can be used when there's no possible
> concurrent
> + * reader. */
> +static inline void ovsrcu_index_set_hidden(ovsrcu_index *i, int value)
> +{
> +ovsrcu_index_set__(i, value, memory_order_relaxed);
> +}
> +
> +/* Initializes 'i' with 'value'.  This is safe to call as long as there are 
> no
> + * concurrent readers. */
> +static inline void ovsrcu_index_init(ovsrcu_index *i, int value)
> +{
> +atomic_init(>v, value);
> +}
> +
>  /* Quiescent states. */
>  void ovsrcu_quiesce_start(void);
>  void ovsrcu_quiesce_end(void);
> --
> 2.8.1

Tested-by: Ciara Loftus 

> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev