Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
From:   Tung Quang Nguyen <tung.q.nguyen@dektech.com.au>
To:     "xu.xin.sc@gmail.com" <xu.xin.sc@gmail.com>,
        "jmaloy@redhat.com" <jmaloy@redhat.com>,
        "ying.xue@windriver.com" <ying.xue@windriver.com>,
        "davem@davemloft.net" <davem@davemloft.net>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "tipc-discussion@lists.sourceforge.net" 
        <tipc-discussion@lists.sourceforge.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "xu.xin16@zte.com.cn" <xu.xin16@zte.com.cn>
Subject: RE: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in
 tipc_rcv
Thread-Topic: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in
 tipc_rcv
Thread-Index: AQHaHbcmsQ7KURUzRkaicoeZO+zrDrCHRXnw
Date:   Thu, 23 Nov 2023 04:12:54 +0000
Message-ID: <DB9PR05MB9078636E4D78B9F4442898B088B9A@DB9PR05MB9078.eurprd05.prod.outlook.com>
References: <20231123024510.2037882-1-xu.xin.sc@gmail.com>
In-Reply-To: <20231123024510.2037882-1-xu.xin.sc@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?YI1o7lRAvWXURV7LKjGXu/hv3fOfb200Hpf2hCPzVLUTKsezP2tjZbOie3FE?=
 =?us-ascii?Q?mrE9PmrtSCUoTLa61YHVHjDu4gYisP9+jSCJtWUgJLf/DSj9Cd5eY0F14nib?=
 =?us-ascii?Q?6E6/u14KAIKuaX/K6+Y6NOrdYXVBhQ57ZGSI7WxNzxsYQs8XcF4sLcCCaDOu?=
 =?us-ascii?Q?t2p2cp4ROSpij/JdezaPJhZCY0fQL5REyCuax9dossMtAf235Pt3Tb9tbyit?=
 =?us-ascii?Q?1fS2qPZX4vMxgXU/VCEa2AEP2/YWbFwE3bLMWFYfNkhIDE0x/6A1OCl1h+L1?=
 =?us-ascii?Q?8egkJnzlTOmxkoknkYI03aZng2z9iepfhubV431ffPpy58ty/P2HKwSXn+mU?=
 =?us-ascii?Q?2RwMvdDr3QJRrFZb3yXKYhWvtMmMM9P8G8mCevFCEexMI982Nr/2P19UN8dj?=
 =?us-ascii?Q?LJvwzf4s0qFGVruo/BT+1ICj4kPQpQdqqo9IGIYTzp+eFAojG1s4wj7S3R7l?=
 =?us-ascii?Q?vfZopTr272omkZJxl6C9WVbF4ULCVokY1hKMnSUiMOErPqTKydbcGxE7pY0O?=
 =?us-ascii?Q?fRAr6Iy97vrWguLAHuBPILMOaSlWxUwRI5yBR8o20EF1Ci2UEBJjcHPaPtUb?=
 =?us-ascii?Q?cyciBkfwdiumiQxscHyBAiSB9hgTSXBGGzDIIQyNKZ3lyaCcIdpwovC1HyuI?=
 =?us-ascii?Q?++ZTs13Y8D/F0kE9DBdT9D+aYgcUevFoBWOvpTo19zmkQmh+rDpRv0C8mP4P?=
 =?us-ascii?Q?pmKTpSoOBwm/DKhJ6cKLUPlc/BTG2pxEw9yICcYbh/vJlPMyRNsJSo7VPWDt?=
 =?us-ascii?Q?evYOz/xUdB9LIrlwHR0knaX568b8phV2qXfrfZjmVxSk4diph0f3N26KduzN?=
 =?us-ascii?Q?Qz+URJC/DiQo1yTStC/5kvZsTHcZQ35tAaUxCkQtZZiQQrO5SHXcSSDTqkO0?=
 =?us-ascii?Q?4HAQfqznvyCAQWTDKNZ5LV9f8FFWG7Hf9gI0/ri4vRJf4la0acZGox8NQMUV?=
 =?us-ascii?Q?C3L4SuJXmezg12TKxJeoXNoc4r8Hy+rdk0e8sax/DXh8U45TNgEHNcmop76l?=
 =?us-ascii?Q?UTIt55SDbqBp6+CyB8WxK/8eAUAigC/TdFRRdZrYEnhRxBAbd3NrCvYM8fIK?=
 =?us-ascii?Q?ACLPs7400lYsuygrDCEXqlpN84FrSS/bwxGCaXRqUFuuv07FCU2Svum80m0M?=
 =?us-ascii?Q?YUdU4ykIiYNZgtbJ1aaZsHgnFjxcwhhkbkA7D7C9h+3GDuKmAkr9SqOgxSxn?=
 =?us-ascii?Q?MCLCVsINmixLyzgFrkYm1RLbtMSnpX0oXdPzZS4b6DWr+OvZCh75gnI9u8zh?=
 =?us-ascii?Q?F+ZUvB07/t5GGB7rfGrdUZa4ydrlKhNfJ/LeJlC7sMsnNQf1qGguJIIMwGzy?=
 =?us-ascii?Q?avi5cqgomLh7I0Av+n9g7Ge3T5R/JI/zVIJ2DprXqM5rhPUT8gNkIWokhJTk?=
 =?us-ascii?Q?TVAA+vOOi/38oQnHT+g+g6ROaNmF2zSM2PzIkKNzI6mO2Gyqyd9e5Zul25ii?=
 =?us-ascii?Q?+GsNtzwZWKnVQIBv3kUAeySH4/OKnLXDu7xCOvreoWciRaSPUDLw19K3mygX?=
 =?us-ascii?Q?zdV5c28l76PXvol+aNrwNxhUOnhN3lBC/MlysR+2PbCvEApwu8YG09mnHzip?=
 =?us-ascii?Q?cSbW14wdoFycRm7V+DI+5JEqVD77s/mY1LGaX75p6nKSgic6i7jAHPf5AK9Y?=
 =?us-ascii?Q?bA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: dektech.com.au
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DB9PR05MB9078.eurprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5eac0478-0109-44ac-e7c6-08dbebda77c3
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Nov 2023 04:12:54.7715
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 1957ea50-0dd8-4360-8db0-c9530df996b2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 3TmYu//426YkZs3is9FJaLkT5suh0SodniYp1e3Ygt8a9WtK1o6hxNwxAgJZGZPwEzGk6jse2Og3h1W8XHdU+/jlSRAGFAkE/ErqJoOoFxs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR05MB8305
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Wed, 22 Nov 2023 20:13:13 -0800 (PST)

> net/tipc/node.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/net/tipc/node.c b/net/tipc/node.c index 3105abe97bb9..2a036b8=
a7da3 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 =3D=3D SELF_UP_PEER_UP) && (usr !=3D TUNNEL_PROTOCO=
L))) {
>+		tipc_node_read_unlock(n);
> 		spin_lock_bh(&le->lock);
> 		if (le->link) {
> 			rc =3D tipc_link_rcv(le->link, skb, &xmitq);
> 			skb =3D 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 =3D tipc_link_rcv(le->link, skb, &xmitq);
> 				skb =3D 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 loc=
k. See what happens if tipc_node_timeout() is called, and the link goes dow=
n:
tipc_node_timeout()
   tipc_node_link_down()
   {
      struct tipc_link *l =3D le->link;
      ...
      if (delete) {
         kfree(l);
         le->link =3D NULL;
      }
      ...
   }