Messages in this thread | | | From | "Eelco Chaudron" <> | Subject | Re: INFO: task hung in ovs_exit_net | Date | Thu, 23 Jul 2020 13:48:59 +0200 |
| |
On 23 Jul 2020, at 13:06, Hillf Danton wrote:
> Wed, 22 Jul 2020 23:27:19 -0700 >> syzbot found the following issue on: <SNIP> > > Fixes: eac87c413bf9 ("net: openvswitch: reorder masks array based on > usage") > by moving cancel_delayed_work_sync() in to the rcu cb, therefore out > of ovs > lock. To facilitate that, add a flag in datapath to inform the kworker > that > there is no more work needed.
I was thinking of re-working the patch and move the handling to the “struct ovs_net” instead of the datapath. This way the rebalance worker can rebalance all datapaths in the netns. Than I can move cancel_delayed_work_sync() from __dp_destroy() to ovs_exit_net(), i.e. outside the ovs lock scope.
However, your fix would be way less intrusive. Are you planning on sending it as a patch? If so, maybe add a comment around the called_rcu variable to be more clear where it’s used for, or maybe rename it to something like called_destory_rcu?
If you think my approach would be better let me know, and I work on a patch.
Feedback anyone?
> --- a/net/openvswitch/datapath.h > +++ b/net/openvswitch/datapath.h > @@ -82,6 +82,7 @@ struct datapath { > > u32 max_headroom; > > + int called_rcu; > /* Switch meters. */ > struct dp_meter_table meter_tbl; > > --- a/net/openvswitch/datapath.c > +++ b/net/openvswitch/datapath.c > @@ -161,6 +161,7 @@ static void destroy_dp_rcu(struct rcu_he > { > struct datapath *dp = container_of(rcu, struct datapath, rcu); > > + cancel_delayed_work_sync(&dp->masks_rebalance); > ovs_flow_tbl_destroy(&dp->table); > free_percpu(dp->stats_percpu); > kfree(dp->ports); > @@ -1760,11 +1761,9 @@ static void __dp_destroy(struct datapath > */ > ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL)); > > + dp->called_rcu = true; > /* RCU destroy the flow table */ > call_rcu(&dp->rcu, destroy_dp_rcu); > - > - /* Cancel remaining work. */ > - cancel_delayed_work_sync(&dp->masks_rebalance); > } > > static int ovs_dp_cmd_del(struct sk_buff *skb, struct genl_info > *info) > @@ -2356,6 +2355,8 @@ static void ovs_dp_masks_rebalance(struc > ovs_flow_masks_rebalance(&dp->table); > ovs_unlock(); > > + if (dp->called_rcu) > + return; > schedule_delayed_work(&dp->masks_rebalance, > msecs_to_jiffies(DP_MASKS_REBALANCE_INTERVAL)); > }
| |