Messages in this thread | | | From | xu <> | Subject | RE: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in tipc_rcv | Date | Thu, 23 Nov 2023 06:22:05 +0000 |
| |
>>diff --git a/net/tipc/node.c b/net/tipc/node.c index 3105abe97bb9..2a036b8a7da3 100644 >>--- a/net/tipc/node.c >>+++ b/net/tipc/node.c >>@@ -2154,14 +2154,15 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b) >> /* Receive packet directly if conditions permit */ >> tipc_node_read_lock(n); >> if (likely((n->state == SELF_UP_PEER_UP) && (usr != TUNNEL_PROTOCOL))) { >>+ tipc_node_read_unlock(n); >> spin_lock_bh(&le->lock); >> if (le->link) { >> rc = tipc_link_rcv(le->link, skb, &xmitq); >> skb = NULL; >> } >> spin_unlock_bh(&le->lock); >>- } >>- tipc_node_read_unlock(n); >>+ } else >>+ tipc_node_read_unlock(n); >> >> /* Check/update node state before receiving */ >> if (unlikely(skb)) { >>@@ -2169,12 +2170,13 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b) >> goto out_node_put; >> tipc_node_write_lock(n); >> if (tipc_node_check_state(n, skb, bearer_id, &xmitq)) { >>+ tipc_node_write_unlock(n); >> if (le->link) { >> rc = tipc_link_rcv(le->link, skb, &xmitq); >> skb = NULL; >> } >>- } >>- tipc_node_write_unlock(n); >>+ } else >>+ tipc_node_write_unlock(n); >> } >> >> if (unlikely(rc & TIPC_LINK_UP_EVT)) >>-- >>2.15.2 >> >> >This patch is wrong. le->link and link status must be protected by node lock. See what happens if tipc_node_timeout() is called, and the link goes down: >tipc_node_timeout() > tipc_node_link_down() > { > struct tipc_link *l = le->link; > ... > if (delete) { > kfree(l); > le->link = NULL; > } > ... > }
Happy to see your reply. But Why? 'delete' is false from tipc_node_timeout(). Refer to: https://elixir.bootlin.com/linux/v6.7-rc2/source/net/tipc/node.c#L844
| |