]>
Commit | Line | Data |
---|---|---|
963efa89 GKH |
1 | From foo@baz Mon Jan 13 09:28:30 PST 2014 |
2 | From: Jason Wang <jasowang@redhat.com> | |
3 | Date: Fri, 13 Dec 2013 17:21:27 +0800 | |
4 | Subject: netvsc: don't flush peers notifying work during setting mtu | |
5 | ||
6 | From: Jason Wang <jasowang@redhat.com> | |
7 | ||
8 | [ Upstream commit 50dc875f2e6e2e04aed3b3033eb0ac99192d6d02 ] | |
9 | ||
10 | There's a possible deadlock if we flush the peers notifying work during setting | |
11 | mtu: | |
12 | ||
13 | [ 22.991149] ====================================================== | |
14 | [ 22.991173] [ INFO: possible circular locking dependency detected ] | |
15 | [ 22.991198] 3.10.0-54.0.1.el7.x86_64.debug #1 Not tainted | |
16 | [ 22.991219] ------------------------------------------------------- | |
17 | [ 22.991243] ip/974 is trying to acquire lock: | |
18 | [ 22.991261] ((&(&net_device_ctx->dwork)->work)){+.+.+.}, at: [<ffffffff8108af95>] flush_work+0x5/0x2e0 | |
19 | [ 22.991307] | |
20 | but task is already holding lock: | |
21 | [ 22.991330] (rtnl_mutex){+.+.+.}, at: [<ffffffff81539deb>] rtnetlink_rcv+0x1b/0x40 | |
22 | [ 22.991367] | |
23 | which lock already depends on the new lock. | |
24 | ||
25 | [ 22.991398] | |
26 | the existing dependency chain (in reverse order) is: | |
27 | [ 22.991426] | |
28 | -> #1 (rtnl_mutex){+.+.+.}: | |
29 | [ 22.991449] [<ffffffff810dfdd9>] __lock_acquire+0xb19/0x1260 | |
30 | [ 22.991477] [<ffffffff810e0d12>] lock_acquire+0xa2/0x1f0 | |
31 | [ 22.991501] [<ffffffff81673659>] mutex_lock_nested+0x89/0x4f0 | |
32 | [ 22.991529] [<ffffffff815392b7>] rtnl_lock+0x17/0x20 | |
33 | [ 22.991552] [<ffffffff815230b2>] netdev_notify_peers+0x12/0x30 | |
34 | [ 22.991579] [<ffffffffa0340212>] netvsc_send_garp+0x22/0x30 [hv_netvsc] | |
35 | [ 22.991610] [<ffffffff8108d251>] process_one_work+0x211/0x6e0 | |
36 | [ 22.991637] [<ffffffff8108d83b>] worker_thread+0x11b/0x3a0 | |
37 | [ 22.991663] [<ffffffff81095e5d>] kthread+0xed/0x100 | |
38 | [ 22.991686] [<ffffffff81681c6c>] ret_from_fork+0x7c/0xb0 | |
39 | [ 22.991715] | |
40 | -> #0 ((&(&net_device_ctx->dwork)->work)){+.+.+.}: | |
41 | [ 22.991715] [<ffffffff810de817>] check_prevs_add+0x967/0x970 | |
42 | [ 22.991715] [<ffffffff810dfdd9>] __lock_acquire+0xb19/0x1260 | |
43 | [ 22.991715] [<ffffffff810e0d12>] lock_acquire+0xa2/0x1f0 | |
44 | [ 22.991715] [<ffffffff8108afde>] flush_work+0x4e/0x2e0 | |
45 | [ 22.991715] [<ffffffff8108e1b5>] __cancel_work_timer+0x95/0x130 | |
46 | [ 22.991715] [<ffffffff8108e303>] cancel_delayed_work_sync+0x13/0x20 | |
47 | [ 22.991715] [<ffffffffa03404e4>] netvsc_change_mtu+0x84/0x200 [hv_netvsc] | |
48 | [ 22.991715] [<ffffffff815233d4>] dev_set_mtu+0x34/0x80 | |
49 | [ 22.991715] [<ffffffff8153bc2a>] do_setlink+0x23a/0xa00 | |
50 | [ 22.991715] [<ffffffff8153d054>] rtnl_newlink+0x394/0x5e0 | |
51 | [ 22.991715] [<ffffffff81539eac>] rtnetlink_rcv_msg+0x9c/0x260 | |
52 | [ 22.991715] [<ffffffff8155cdd9>] netlink_rcv_skb+0xa9/0xc0 | |
53 | [ 22.991715] [<ffffffff81539dfa>] rtnetlink_rcv+0x2a/0x40 | |
54 | [ 22.991715] [<ffffffff8155c41d>] netlink_unicast+0xdd/0x190 | |
55 | [ 22.991715] [<ffffffff8155c807>] netlink_sendmsg+0x337/0x750 | |
56 | [ 22.991715] [<ffffffff8150d219>] sock_sendmsg+0x99/0xd0 | |
57 | [ 22.991715] [<ffffffff8150d63e>] ___sys_sendmsg+0x39e/0x3b0 | |
58 | [ 22.991715] [<ffffffff8150eba2>] __sys_sendmsg+0x42/0x80 | |
59 | [ 22.991715] [<ffffffff8150ebf2>] SyS_sendmsg+0x12/0x20 | |
60 | [ 22.991715] [<ffffffff81681d19>] system_call_fastpath+0x16/0x1b | |
61 | ||
62 | This is because we hold the rtnl_lock() before ndo_change_mtu() and try to flush | |
63 | the work in netvsc_change_mtu(), in the mean time, netdev_notify_peers() may be | |
64 | called from worker and also trying to hold the rtnl_lock. This will lead the | |
65 | flush won't succeed forever. Solve this by not canceling and flushing the work, | |
66 | this is safe because the transmission done by NETDEV_NOTIFY_PEERS was | |
67 | synchronized with the netif_tx_disable() called by netvsc_change_mtu(). | |
68 | ||
69 | Reported-by: Yaju Cao <yacao@redhat.com> | |
70 | Tested-by: Yaju Cao <yacao@redhat.com> | |
71 | Cc: K. Y. Srinivasan <kys@microsoft.com> | |
72 | Cc: Haiyang Zhang <haiyangz@microsoft.com> | |
73 | Signed-off-by: Jason Wang <jasowang@redhat.com> | |
74 | Acked-by: Haiyang Zhang <haiyangz@microsoft.com> | |
75 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
76 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
77 | --- | |
78 | drivers/net/hyperv/netvsc_drv.c | 1 - | |
79 | 1 file changed, 1 deletion(-) | |
80 | ||
81 | --- a/drivers/net/hyperv/netvsc_drv.c | |
82 | +++ b/drivers/net/hyperv/netvsc_drv.c | |
83 | @@ -321,7 +321,6 @@ static int netvsc_change_mtu(struct net_ | |
84 | return -EINVAL; | |
85 | ||
86 | nvdev->start_remove = true; | |
87 | - cancel_delayed_work_sync(&ndevctx->dwork); | |
88 | cancel_work_sync(&ndevctx->work); | |
89 | netif_tx_disable(ndev); | |
90 | rndis_filter_device_remove(hdev); |