Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
From:   Tung Quang Nguyen <tung.q.nguyen@dektech.com.au>
To:     xu <xu.xin.sc@gmail.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "jmaloy@redhat.com" <jmaloy@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "tipc-discussion@lists.sourceforge.net" 
        <tipc-discussion@lists.sourceforge.net>,
        "xu.xin16@zte.com.cn" <xu.xin16@zte.com.cn>,
        "ying.xue@windriver.com" <ying.xue@windriver.com>,
        "yang.yang29@zte.com.cn" <yang.yang29@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+zrDrCHRXnwgAAp2oCAAAQK0A==
Date:   Thu, 23 Nov 2023 06:54:11 +0000
Message-ID: <DB9PR05MB9078EEC976944CACEC531C1388B9A@DB9PR05MB9078.eurprd05.prod.outlook.com>
References: <DB9PR05MB9078636E4D78B9F4442898B088B9A@DB9PR05MB9078.eurprd05.prod.outlook.com>
 <20231123062205.2038335-1-xu.xin16@zte.com.cn>
In-Reply-To: <20231123062205.2038335-1-xu.xin16@zte.com.cn>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?ZP3EMBUIwKNPlOKg/R7WIExGDywlzOqRWDVqXPThxbjs3si3TAFPof6lTJv5?=
 =?us-ascii?Q?IyZD8oYL4RlD1PeB8RJkzfKUiYNke9OgOYyxGDBJpEu2Ir+Lz8tKy4qq8dbG?=
 =?us-ascii?Q?y1uFAgiKBjXXJ6Oi3pQDbMSQenh6fnk55/HFrnPgfX7wiIZmHs3OTiiJWYnM?=
 =?us-ascii?Q?qwf9jMBWQu6QVBtqdWA2PRW0hGLtTQOs6yHTUElCFVdmlTL+SRDakLaOSxB+?=
 =?us-ascii?Q?Jq+0oyRWKKMW4aGHywxhOBFxLoNGVuaUnTIDX+mEGL+DtdONbPGsnybVEStx?=
 =?us-ascii?Q?TDpb9t5GleC5GkdO8Y1F5qil2iKGdP0zQ729QvUUZjozRignYV4BGGL4+ubx?=
 =?us-ascii?Q?sI7gYFWiXCSYfOeZAlOLQlGBYjjLX8C6QCQ/5Tn1jJTIfJinY0DSp5yvJwGR?=
 =?us-ascii?Q?xo3I6n2RxbS0R/skVHV7jQPLSniYLVUbdxi2VyIKMihpgZaVFSoMLUt3tQ/B?=
 =?us-ascii?Q?WcCZkhV5jSxoRHYF6f/EexUubV0eUBW+Tq5ElK1sF93OfLLjxcymd2dqRtps?=
 =?us-ascii?Q?45jsEDjw+SkTmhZiOmZWYuhUbAjavk4A/FmKBz0Uzahain1pQ5NBu1RcwAUU?=
 =?us-ascii?Q?nXJ+QRcZbG1np4M3ZjkZfVh2WKMD0wp7mJGP3I26xZKb0cPXbRW92BMm8Rtb?=
 =?us-ascii?Q?qIXoiBVCrkVoovyXKJH2ARu8WwCKDMPsm9O92wvc1Jj2xEpge8i4DygWsU3H?=
 =?us-ascii?Q?41oXJSOkRfDksExGcMJGWDMNoCAiYl3uwYIfecJ/Iuw41udJheKbJIt9kR9x?=
 =?us-ascii?Q?7g42Xq8RFdGn/ip0ZfPqxCckvw1nHwLnZJMomXchiViuf9knfJPtly2saYGn?=
 =?us-ascii?Q?3K1HEZ3hYtsWdoORcoQBI447jkeURkZxrgatlzDXdipkujWUUsVeW8z9YUjH?=
 =?us-ascii?Q?aNBwnDAUQCwEzfA28KXDt5mxH85oQSLz6LfAZzCsrXEjqO4FD1+/k/Fe+Rmh?=
 =?us-ascii?Q?DpTast96xbosIq0Hfv7WL4jNcOz8V3emVSGbZXJjAJ44hSBDiO2qxoCG0Qr6?=
 =?us-ascii?Q?E5w8cXFOdmBsVc+TgGMPA6/8VeAOepBtMB/G2mR1XKEj05dDMcKpNrYkeOhH?=
 =?us-ascii?Q?abohTu74vYx+iCposu8Vr1fFWaGruiWG9xemjucA0r4wPzNT5RAXvoVwAfy4?=
 =?us-ascii?Q?8Frwp9H/VE1TXNvpvEoEMUa7x4TDUXjbqOJ6mTJPfPBt0co7kfGhkR058Y8Q?=
 =?us-ascii?Q?W6AvuQXpRLRj29/3Pp6L49czRpLXwvTeoueemfvaYnqdpaEfBwLP2CCq1shi?=
 =?us-ascii?Q?6iW0FmmPz+NLZTqgDDf3yRelEzu6y8gniQB3REZ+NPVQ5LQS8B1074uL/6KT?=
 =?us-ascii?Q?IRo+uZXJdrjKtvVJdEtctToXgg7Gysk2JLUM0lhe0nBhiYpjm5m8GPRD3DHQ?=
 =?us-ascii?Q?aaQAWUsrTc36mq+46r9+WWdU7xiz56pSQne4cyADebC3ImNRF3HbDZhWI0lR?=
 =?us-ascii?Q?HTje46NF94IGdfSFmuUDjKKZ7y1hm4FWibQWxAM+se/6mD+dmYzEyQGSHX0S?=
 =?us-ascii?Q?uDptwY7yMmjVtPigFHQvpgQUdJUjv+L5tA3TzHQMjkv6/eLhRAfd30LjldnW?=
 =?us-ascii?Q?G/2SRE93O8mj1zupZPvp5evtDoiVsVOa9z2PG2sIBMHPxlu/SrrdzsXJ/yej?=
 =?us-ascii?Q?lg=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: d5c51e34-1e96-4941-7211-08dbebf0ff62
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Nov 2023 06:54:11.2169
 (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: d2yF0tFyOTFFZVhkH1D4Z1I4VB0MujPuQGEDTOwjyX7AhHagl7XHB5JEMNMu6n9RIc3Giudw7zglI5qJtEtmqdcmohWLLGoAmqhFZTgAvHs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR05MB7638
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 lipwig.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 (lipwig.vger.email [0.0.0.0]); Wed, 22 Nov 2023 22:54:26 -0800 (PST)

>>This patch is wrong. le->link and link status must be protected by node l=
ock. 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 =3D le->link;
>>      ...
>>      if (delete) {
>>         kfree(l);
>>         le->link =3D 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
I should have explained it clearly:
1/ link status must be protected.
tipc_node_timeout()
   tipc_node_link_down()
   {
      struct tipc_link *l =3D le->link;
  =20
      ...
     __tipc_node_link_down(); <-- link status is referred.=20
      ...
     if (delete) {
        kfree(l);
        le->link =3D NULL;
     }
     ...
   }

__tipc_node_link_down()
{
    ...
   if (!l || tipc_link_is_reset(l)) <-- read link status
    ...
    tipc_link_reset(l); <--- this function will reset all things related to=
 link.
}

2/ le->link must be protected.
bearer_disable()
{
   ...
   tipc_node_delete_links(net, bearer_id); <--- this will delete all links.
   ...
}

tipc_node_delete_links()
{
   ...
  tipc_node_link_down(n, bearer_id, true);
   ...
}