summaryrefslogtreecommitdiffstats
path: root/net/tipc/bcast.c
diff options
context:
space:
mode:
authorYing Xue <ying.xue@windriver.com>2015-03-26 06:10:23 -0400
committerDavid S. Miller <davem@davemloft.net>2015-03-29 15:40:27 -0400
commitb952b2befb6f6b009e91f087285b9a0a6beb1cc8 (patch)
tree39723b22169b63f4f6b81fdb60ad618053271879 /net/tipc/bcast.c
parentfaadb05f4b7f11b3063a20d7fc5afcbf1124dbeb (diff)
tipc: fix potential deadlock when all links are reset
[ 60.988363] ====================================================== [ 60.988754] [ INFO: possible circular locking dependency detected ] [ 60.989152] 3.19.0+ #194 Not tainted [ 60.989377] ------------------------------------------------------- [ 60.989781] swapper/3/0 is trying to acquire lock: [ 60.990079] (&(&n_ptr->lock)->rlock){+.-...}, at: [<ffffffffa0006dca>] tipc_link_retransmit+0x1aa/0x240 [tipc] [ 60.990743] [ 60.990743] but task is already holding lock: [ 60.991106] (&(&bclink->lock)->rlock){+.-...}, at: [<ffffffffa00004be>] tipc_bclink_lock+0x8e/0xa0 [tipc] [ 60.991738] [ 60.991738] which lock already depends on the new lock. [ 60.991738] [ 60.992174] [ 60.992174] the existing dependency chain (in reverse order) is: [ 60.992174] -> #1 (&(&bclink->lock)->rlock){+.-...}: [ 60.992174] [<ffffffff810a9c0c>] lock_acquire+0x9c/0x140 [ 60.992174] [<ffffffff8179c41f>] _raw_spin_lock_bh+0x3f/0x50 [ 60.992174] [<ffffffffa00004be>] tipc_bclink_lock+0x8e/0xa0 [tipc] [ 60.992174] [<ffffffffa0000f57>] tipc_bclink_add_node+0x97/0xf0 [tipc] [ 60.992174] [<ffffffffa0011815>] tipc_node_link_up+0xf5/0x110 [tipc] [ 60.992174] [<ffffffffa0007783>] link_state_event+0x2b3/0x4f0 [tipc] [ 60.992174] [<ffffffffa00193c0>] tipc_link_proto_rcv+0x24c/0x418 [tipc] [ 60.992174] [<ffffffffa0008857>] tipc_rcv+0x827/0xac0 [tipc] [ 60.992174] [<ffffffffa0002ca3>] tipc_l2_rcv_msg+0x73/0xd0 [tipc] [ 60.992174] [<ffffffff81646e66>] __netif_receive_skb_core+0x746/0x980 [ 60.992174] [<ffffffff816470c1>] __netif_receive_skb+0x21/0x70 [ 60.992174] [<ffffffff81647295>] netif_receive_skb_internal+0x35/0x130 [ 60.992174] [<ffffffff81648218>] napi_gro_receive+0x158/0x1d0 [ 60.992174] [<ffffffff81559e05>] e1000_clean_rx_irq+0x155/0x490 [ 60.992174] [<ffffffff8155c1b7>] e1000_clean+0x267/0x990 [ 60.992174] [<ffffffff81647b60>] net_rx_action+0x150/0x360 [ 60.992174] [<ffffffff8105ec43>] __do_softirq+0x123/0x360 [ 60.992174] [<ffffffff8105f12e>] irq_exit+0x8e/0xb0 [ 60.992174] [<ffffffff8179f9f5>] do_IRQ+0x65/0x110 [ 60.992174] [<ffffffff8179da6f>] ret_from_intr+0x0/0x13 [ 60.992174] [<ffffffff8100de9f>] arch_cpu_idle+0xf/0x20 [ 60.992174] [<ffffffff8109dfa6>] cpu_startup_entry+0x2f6/0x3f0 [ 60.992174] [<ffffffff81033cda>] start_secondary+0x13a/0x150 [ 60.992174] -> #0 (&(&n_ptr->lock)->rlock){+.-...}: [ 60.992174] [<ffffffff810a8f7d>] __lock_acquire+0x163d/0x1ca0 [ 60.992174] [<ffffffff810a9c0c>] lock_acquire+0x9c/0x140 [ 60.992174] [<ffffffff8179c41f>] _raw_spin_lock_bh+0x3f/0x50 [ 60.992174] [<ffffffffa0006dca>] tipc_link_retransmit+0x1aa/0x240 [tipc] [ 60.992174] [<ffffffffa0001e11>] tipc_bclink_rcv+0x611/0x640 [tipc] [ 60.992174] [<ffffffffa0008646>] tipc_rcv+0x616/0xac0 [tipc] [ 60.992174] [<ffffffffa0002ca3>] tipc_l2_rcv_msg+0x73/0xd0 [tipc] [ 60.992174] [<ffffffff81646e66>] __netif_receive_skb_core+0x746/0x980 [ 60.992174] [<ffffffff816470c1>] __netif_receive_skb+0x21/0x70 [ 60.992174] [<ffffffff81647295>] netif_receive_skb_internal+0x35/0x130 [ 60.992174] [<ffffffff81648218>] napi_gro_receive+0x158/0x1d0 [ 60.992174] [<ffffffff81559e05>] e1000_clean_rx_irq+0x155/0x490 [ 60.992174] [<ffffffff8155c1b7>] e1000_clean+0x267/0x990 [ 60.992174] [<ffffffff81647b60>] net_rx_action+0x150/0x360 [ 60.992174] [<ffffffff8105ec43>] __do_softirq+0x123/0x360 [ 60.992174] [<ffffffff8105f12e>] irq_exit+0x8e/0xb0 [ 60.992174] [<ffffffff8179f9f5>] do_IRQ+0x65/0x110 [ 60.992174] [<ffffffff8179da6f>] ret_from_intr+0x0/0x13 [ 60.992174] [<ffffffff8100de9f>] arch_cpu_idle+0xf/0x20 [ 60.992174] [<ffffffff8109dfa6>] cpu_startup_entry+0x2f6/0x3f0 [ 60.992174] [<ffffffff81033cda>] start_secondary+0x13a/0x150 [ 60.992174] [ 60.992174] other info that might help us debug this: [ 60.992174] [ 60.992174] Possible unsafe locking scenario: [ 60.992174] [ 60.992174] CPU0 CPU1 [ 60.992174] ---- ---- [ 60.992174] lock(&(&bclink->lock)->rlock); [ 60.992174] lock(&(&n_ptr->lock)->rlock); [ 60.992174] lock(&(&bclink->lock)->rlock); [ 60.992174] lock(&(&n_ptr->lock)->rlock); [ 60.992174] [ 60.992174] *** DEADLOCK *** [ 60.992174] [ 60.992174] 3 locks held by swapper/3/0: [ 60.992174] #0: (rcu_read_lock){......}, at: [<ffffffff81646791>] __netif_receive_skb_core+0x71/0x980 [ 60.992174] #1: (rcu_read_lock){......}, at: [<ffffffffa0002c35>] tipc_l2_rcv_msg+0x5/0xd0 [tipc] [ 60.992174] #2: (&(&bclink->lock)->rlock){+.-...}, at: [<ffffffffa00004be>] tipc_bclink_lock+0x8e/0xa0 [tipc] [ 60.992174] The correct the sequence of grabbing n_ptr->lock and bclink->lock should be that the former is first held and the latter is then taken, which exactly happened on CPU1. But especially when the retransmission of broadcast link is failed, bclink->lock is first held in tipc_bclink_rcv(), and n_ptr->lock is taken in link_retransmit_failure() called by tipc_link_retransmit() subsequently, which is demonstrated on CPU0. As a result, deadlock occurs. If the order of holding the two locks happening on CPU0 is reversed, the deadlock risk will be relieved. Therefore, the node lock taken in link_retransmit_failure() originally is moved to tipc_bclink_rcv() so that it's obtained before bclink lock. But the precondition of the adjustment of node lock is that responding to bclink reset event must be moved from tipc_bclink_unlock() to tipc_node_unlock(). Reviewed-by: Erik Hugne <erik.hugne@ericsson.com> Signed-off-by: Ying Xue <ying.xue@windriver.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/tipc/bcast.c')
-rw-r--r--net/tipc/bcast.c23
1 files changed, 1 insertions, 22 deletions
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 79355531c3e2..4289dd62f589 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -62,21 +62,8 @@ static void tipc_bclink_lock(struct net *net)
62static void tipc_bclink_unlock(struct net *net) 62static void tipc_bclink_unlock(struct net *net)
63{ 63{
64 struct tipc_net *tn = net_generic(net, tipc_net_id); 64 struct tipc_net *tn = net_generic(net, tipc_net_id);
65 struct tipc_node *node = NULL;
66 65
67 if (likely(!tn->bclink->flags)) {
68 spin_unlock_bh(&tn->bclink->lock);
69 return;
70 }
71
72 if (tn->bclink->flags & TIPC_BCLINK_RESET) {
73 tn->bclink->flags &= ~TIPC_BCLINK_RESET;
74 node = tipc_bclink_retransmit_to(net);
75 }
76 spin_unlock_bh(&tn->bclink->lock); 66 spin_unlock_bh(&tn->bclink->lock);
77
78 if (node)
79 tipc_link_reset_all(node);
80} 67}
81 68
82void tipc_bclink_input(struct net *net) 69void tipc_bclink_input(struct net *net)
@@ -91,13 +78,6 @@ uint tipc_bclink_get_mtu(void)
91 return MAX_PKT_DEFAULT_MCAST; 78 return MAX_PKT_DEFAULT_MCAST;
92} 79}
93 80
94void tipc_bclink_set_flags(struct net *net, unsigned int flags)
95{
96 struct tipc_net *tn = net_generic(net, tipc_net_id);
97
98 tn->bclink->flags |= flags;
99}
100
101static u32 bcbuf_acks(struct sk_buff *buf) 81static u32 bcbuf_acks(struct sk_buff *buf)
102{ 82{
103 return (u32)(unsigned long)TIPC_SKB_CB(buf)->handle; 83 return (u32)(unsigned long)TIPC_SKB_CB(buf)->handle;
@@ -156,7 +136,6 @@ static void bclink_update_last_sent(struct tipc_node *node, u32 seqno)
156 seqno : node->bclink.last_sent; 136 seqno : node->bclink.last_sent;
157} 137}
158 138
159
160/** 139/**
161 * tipc_bclink_retransmit_to - get most recent node to request retransmission 140 * tipc_bclink_retransmit_to - get most recent node to request retransmission
162 * 141 *
@@ -476,13 +455,13 @@ void tipc_bclink_rcv(struct net *net, struct sk_buff *buf)
476 goto unlock; 455 goto unlock;
477 if (msg_destnode(msg) == tn->own_addr) { 456 if (msg_destnode(msg) == tn->own_addr) {
478 tipc_bclink_acknowledge(node, msg_bcast_ack(msg)); 457 tipc_bclink_acknowledge(node, msg_bcast_ack(msg));
479 tipc_node_unlock(node);
480 tipc_bclink_lock(net); 458 tipc_bclink_lock(net);
481 bcl->stats.recv_nacks++; 459 bcl->stats.recv_nacks++;
482 tn->bclink->retransmit_to = node; 460 tn->bclink->retransmit_to = node;
483 bclink_retransmit_pkt(tn, msg_bcgap_after(msg), 461 bclink_retransmit_pkt(tn, msg_bcgap_after(msg),
484 msg_bcgap_to(msg)); 462 msg_bcgap_to(msg));
485 tipc_bclink_unlock(net); 463 tipc_bclink_unlock(net);
464 tipc_node_unlock(node);
486 } else { 465 } else {
487 tipc_node_unlock(node); 466 tipc_node_unlock(node);
488 bclink_peek_nack(net, msg); 467 bclink_peek_nack(net, msg);
generated by cgit v1.2.2 (git 2.25.0) at 2025-02-21 00:07:07 -0500