From: "Eelco Chaudron" <echaudro@redhat.com>
To: "Hillf Danton" <hdanton@sina.com>
Cc: syzbot <syzbot+2c4ff3614695f75ce26c@syzkaller.appspotmail.com>,
davem@davemloft.net, dev@openvswitch.org, kuba@kernel.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
"Paolo Abeni" <pabeni@redhat.com>,
pshelar@ovn.org, syzkaller-bugs@googlegroups.com,
"Markus Elfring" <Markus.Elfring@web.de>
Subject: Re: INFO: task hung in ovs_exit_net
Date: Thu, 23 Jul 2020 13:48:59 +0200 [thread overview]
Message-ID: <35ADBFE5-87FF-4E9E-A8FD-BB586E9F663F@redhat.com> (raw)
In-Reply-To: <20200723110655.12856-1-hdanton@sina.com>
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));
> }
next parent reply other threads:[~2020-07-23 11:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200723110655.12856-1-hdanton@sina.com>
2020-07-23 11:48 ` Eelco Chaudron [this message]
2020-07-23 6:27 INFO: task hung in ovs_exit_net syzbot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=35ADBFE5-87FF-4E9E-A8FD-BB586E9F663F@redhat.com \
--to=echaudro@redhat.com \
--cc=Markus.Elfring@web.de \
--cc=davem@davemloft.net \
--cc=dev@openvswitch.org \
--cc=hdanton@sina.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pshelar@ovn.org \
--cc=syzbot+2c4ff3614695f75ce26c@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).