On Mon, Aug 10, 2020 at 8:27 PM Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx> wrote: > > On Tue, Aug 11, 2020 at 10:24 AM Cong Wang <xiyou.wangcong@xxxxxxxxx> wrote: > > > > On Mon, Aug 10, 2020 at 6:16 PM Tonghao Zhang <xiangxia.m.yue@xxxxxxxxx> wrote: > > > Hi all, I send a patch to fix this. The rcu warnings disappear. I > > > don't reproduce the double free issue. > > > But I guess this patch may address this issue. > > > > > > http://patchwork.ozlabs.org/project/netdev/patch/20200811011001.75690-1-xiangxia.m.yue@xxxxxxxxx/ > > > > I don't see how your patch address the double-free, as we still > > free mask array twice after your patch: once in tbl_mask_array_realloc() > > and once in ovs_flow_tbl_destroy(). > Hi Cong. > Before my patch, we use the ovsl_dereference > (rcu_dereference_protected) in the rcu callback. > ovs_flow_tbl_destroy > ->table_instance_destroy > ->table_instance_flow_free > ->flow_mask_remove > ASSERT_OVSL(will print warning) > ->tbl_mask_array_del_mask > ovsl_dereference(rcu usage warning) > I understand how your patch addresses the RCU annotation issue, which is different from double-free. > so we should invoke the table_instance_destroy or others under > ovs_lock to avoid (ASSERT_OVSL and rcu usage warning). Of course... I never doubt it. > with this patch, we reallocate the mask_array under ovs_lock, and free > it in the rcu callback. Without it, we reallocate and free it in the > rcu callback. > I think we may fix it with this patch. Does it matter which context tbl_mask_array_realloc() is called? Even with ovs_lock, we can still double free: ovs_lock() tbl_mask_array_realloc() => call_rcu(&old->rcu, mask_array_rcu_cb); ovs_unlock() ... ovs_flow_tbl_destroy() => call_rcu(&old->rcu, mask_array_rcu_cb); So still twice, right? To fix the double-free, we have to eliminate one of them, don't we? ;) > > > Have you tried my patch which is supposed to address this double-free? > I don't reproduce it. but your patch does not avoid ruc usage warning > and ASSERT_OVSL. Sure, I never intend to fix anything else but double-free. The $subject is about double free, I double checked. ;) Thanks.
