[ovs-dev] [master fix] recirculation: RCU postpone the free of dpif_backer_recirc_node.

Alex Wang alexw at nicira.com
Tue Dec 23 18:48:47 UTC 2014


Got an offline Ack from Ethan, applied to master,

On Tue, Dec 23, 2014 at 10:30 AM, Alex Wang <alexw at nicira.com> wrote:

> This commit RCU postpones the free of 'struct dpif_backer_recirc_node',
> after it is removed from the cmap.  This is in that other threads may
> be accessing the struct at the same time.
>
> Signed-off-by: Alex Wang <alexw at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 2166e91..846ef57 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -59,6 +59,7 @@
>  #include "ofproto-dpif-upcall.h"
>  #include "ofproto-dpif-xlate.h"
>  #include "poll-loop.h"
> +#include "ovs-rcu.h"
>  #include "ovs-router.h"
>  #include "seq.h"
>  #include "simap.h"
> @@ -868,7 +869,11 @@ dpif_backer_recirc_clear_ofproto(struct dpif_backer
> *backer,
>                       "is destructed", node->recirc_id, ofproto->up.name);
>              cmap_remove(&backer->recirc_map, &node->cmap_node,
>                          node->recirc_id);
> -            free(node);
> +            /* Does not matter whether directly free or use
> ovsrcu_postpone,
> +             * since all datapath flows are already purged before calling
> this
> +             * function, and no 'recirc_id' could be associated to
> 'ofproto'.
> +             */
> +            ovsrcu_postpone(free, node);
>          }
>      }
>      ovs_mutex_unlock(&backer->recirc_mutex);
> @@ -5457,7 +5462,9 @@ ofproto_dpif_free_recirc_id(struct ofproto_dpif
> *ofproto, uint32_t recirc_id)
>          cmap_remove(&backer->recirc_map, &node->cmap_node,
> node->recirc_id);
>          ovs_mutex_unlock(&backer->recirc_mutex);
>          recirc_id_free(backer->rid_pool, node->recirc_id);
> -        free(node);
> +        /* RCU postpone the free, since other threads may be referring
> +         * to 'node' at same time. */
> +        ovsrcu_postpone(free, node);
>      }
>  }
>
> --
> 1.7.9.5
>
>



More information about the dev mailing list