Re: [PATCH net-next v4 4/4] net: openvswitch: avoid double-rcu wait period
From: Eelco Chaudron
Date: Mon Jun 15 2026 - 09:58:29 EST
On 11 Jun 2026, at 6:58, Adrian Moreno wrote:
> Avoid waiting for two rcu periods by scheduling the deletion of the
> flow_table and all of its referenced structs at the same time.
Hi Adrian,
One small nit below; the code itself looks fine to me.
Cheers,
Eelco
> Signed-off-by: Adrian Moreno <amorenoz@xxxxxxxxxx>
> ---
> net/openvswitch/flow_table.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
> index 3934873a44c3..35232e1af8aa 100644
> --- a/net/openvswitch/flow_table.c
> +++ b/net/openvswitch/flow_table.c
> @@ -527,30 +527,33 @@ static void table_instance_destroy(struct table_instance *ti,
> call_rcu(&ufid_ti->rcu, flow_tbl_destroy_rcu_cb);
> }
>
> -/* No need for locking this function is called from RCU callback. */
> static void ovs_flow_tbl_destroy_rcu(struct rcu_head *rcu)
> {
> struct flow_table *table = container_of(rcu, struct flow_table, rcu);
>
> - struct table_instance *ti = rcu_dereference_raw(table->ti);
> - struct table_instance *ufid_ti = rcu_dereference_raw(table->ufid_ti);
> - struct mask_cache *mc = rcu_dereference_raw(table->mask_cache);
> - struct mask_array *ma = rcu_dereference_raw(table->mask_array);
> -
> - call_rcu(&mc->rcu, mask_cache_rcu_cb);
> - call_rcu(&ma->rcu, mask_array_rcu_cb);
> - table_instance_destroy(ti, ufid_ti);
> mutex_destroy(&table->lock);
> kfree(table);
> }
>
> void ovs_flow_tbl_put(struct flow_table *table)
> {
> + struct table_instance *ufid_ti;
> + struct table_instance *ti;
> + struct mask_cache *mc;
> + struct mask_array *ma;
> +
> if (refcount_dec_and_test(&table->refcnt)) {
> mutex_lock(&table->lock);
> - table_instance_flow_flush(table,
> - ovs_tbl_dereference(table->ti, table),
> - ovs_tbl_dereference(table->ufid_ti, table));
> + ufid_ti = ovs_tbl_dereference(table->ufid_ti, table);
> + ti = ovs_tbl_dereference(table->ti, table);
> + table_instance_flow_flush(table, ti, ufid_ti);
> + table_instance_destroy(ti, ufid_ti);
> +
> + mc = ovs_tbl_dereference(table->mask_cache, table);
> + ma = ovs_tbl_dereference(table->mask_array, table);
> + call_rcu(&mc->rcu, mask_cache_rcu_cb);
> + call_rcu(&ma->rcu, mask_array_rcu_cb);
nit: Would it be cleaner to extract the destruction logic in
ovs_flow_tbl_put() into a separate static function, e.g.
ovs_flow_tbl_destroy(), to separate the refcount handling
from the actual cleanup?
> mutex_unlock(&table->lock);
> call_rcu(&table->rcu, ovs_flow_tbl_destroy_rcu);
> }