On Wed, Jan 15, 2014 at 1:06 PM, Andy Zhou <az...@nicira.com> wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index b42fd8b..6ae4ecf 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -1305,10 +1306,13 @@ static void __dp_destroy(struct datapath *dp)
> list_del_rcu(&dp->list_node);
>
> /* OVSP_LOCAL is datapath internal port. We need to make sure that
> - * all port in datapath are destroyed first before freeing datapath.
> - */
> + * all ports in datapath are destroyed first before freeing datapath.
> + */
> ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL));
>
> + /* Remove all flows while holding ovs_mutex */
> + ovs_flow_tbl_flush(&dp->table);
> +
> call_rcu(&dp->rcu, destroy_dp_rcu);
It might be simpler to just destroy the table at this point -
otherwise we flush now and then destroy later in the RCU callback
(even though these operations are essentially the same thing).
> diff --git a/datapath/flow_table.c b/datapath/flow_table.c
> index 4232b82..3ebea78 100644
> --- a/datapath/flow_table.c
> +++ b/datapath/flow_table.c
> -static void __table_instance_destroy(struct table_instance *ti)
> +static void table_instance_free_flows(struct table_instance *ti, bool
> deferred)
I believe this is only called from table_instance_destroy() - maybe we
should just fold it in? It is somewhat of a discrete block of code but
in this case I think consolidating functions helps readability since
there are so many paths.
> -skip_flows:
> +static void __table_instance_destroy(struct table_instance *ti)
> +{
> free_buckets(ti->buckets);
> kfree(ti);
> }
> @@ -262,7 +262,8 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu)
> {
> struct table_instance *ti = container_of(rcu, struct table_instance,
> rcu);
>
> - __table_instance_destroy(ti);
> + free_buckets(ti->buckets);
> + kfree(ti);
> }
Should we use the helper function here?
This is definitely very intricate code. I think the basic principle
here is that the only operation that should be performed in a deferred
block is freeing memory - nothing more complex should take advantage
of an existing RCU callback, which means that we have a series of
callbacks in parallel instead of a chain. I think this principle is
now respected everywhere.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
