From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0CC6BA057B; Tue, 14 Apr 2020 13:26:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 639E81C1C9; Tue, 14 Apr 2020 13:26:42 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id A2DB41C1C4; Tue, 14 Apr 2020 13:26:40 +0200 (CEST) IronPort-SDR: Krt9hu/T562ZHB7mXnZwOZ/tNbqQOQ9A0R1l8F30VHTLyXbWxRQlCsvteg6Jq/e9+zWlm31A62 vR0qE8vr/Uvg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Apr 2020 04:26:39 -0700 IronPort-SDR: uLI+yfjabIt0K3q1A2VErp2NqdkE2FGiq2R60C5PmP/kxpQkRjxjGRsCkimABR11PyoRyz4DAS n9McazuHjdQA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,382,1580803200"; d="scan'208";a="241963313" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga007.jf.intel.com with ESMTP; 14 Apr 2020 04:26:39 -0700 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 04:26:38 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.41) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 14 Apr 2020 04:26:19 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=dy1M04W25x3olSAx3o2mt5Yf0Y/ZLDJgFN891D74/+h8Cuqn3rm/5OXInlq3etSyToS7rFvWBW7h3Ilg45ZS/lvUWKRi3wT8/RvyipAD7C50IlHiPf3uS0l45S3jkrcemK3f2lnvi0Qw2FHZAB4PMfbDA7Rn843xeN3lyPg6i+cxAQem5VvyPojSD2rBtI+tUJbN0j5xQBVQ0I8GX4gY9Icw247QA4msnSzLoKtdLmqlr0rsfMixrPw9ju9dBfXw/wn7WWJbjJLVrZJBsTMazFo8O9Pjx8xiUPhGWZMtIEIwWhTiygHeji3jNopRFluhMTb3A+5W0TVB/jnMtCQC2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=meUS7XwBt530RYnHY+XUw6TwNZi4wKb/7m5rjOC2LqQ=; b=ems5Rhae+oxwlGAIWtd3Pr2v1rrA4TkMlzABmzkwObz4Ts+Dn/bHFw5cf8qEc+41jIX418WKZaz+fCM9BmTkZ5ZuYBZuLI1alpVUhWqCW/2RNa0lxZfovvzyDwVWcQOVtmTDMlmaRF81dPb2IcMtdD4GzMibq3LyWh6PL3bn50spF3gyG9c9ojAKKfW1x5t1k5ePJCQXPAqZSnK9CW1XqjCqc0uBg1PJMr0BzJG6+wOzu9Qoh3htO1uF0Mkr/Wt2cvIqiBZxs/a16/otWfLZRjHyxMgfowyXmZmrF9hIzyS9HN3U8ZUMgXCOSrbx+EcvqUkE2qhrL5LpLlStTCUDVg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=meUS7XwBt530RYnHY+XUw6TwNZi4wKb/7m5rjOC2LqQ=; b=jI4zuF1/VNoEmuMFKZlyRIkAK7Hlz2950q5fNSRO1/qZaxek4vI7WXcygmwVScAAL5tEkZLIJebnz9ZbVd0lTZ0KZaERho4ExjAyHcr7VPMgN0Wtozy/kvPNKTK9aSdN5vK0oDq/s5s80TTeEuxUhgtMBd4Q7TSzHCLEpk/TEAo= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (20.177.185.90) by BYAPR11MB3192.namprd11.prod.outlook.com (20.177.184.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.26; Tue, 14 Apr 2020 11:26:10 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4%6]) with mapi id 15.20.2900.028; Tue, 14 Apr 2020 11:26:10 +0000 From: "Ananyev, Konstantin" To: "Zhu, TaoX" , "Lu, Wenzhuo" , "Ye, Xiaolong" CC: "dev@dpdk.org" , "martin.weiser@allegro-packets.com" , "stable@dpdk.org" Thread-Topic: [PATCH v2] net/ixgbe: fix resource leak after thread exits normally Thread-Index: AQHWDwB2lbqMQzERmU2LHCAgEll6GqhyQPRggAWZ+oCAAJLo0IAAEohA Date: Tue, 14 Apr 2020 11:26:09 +0000 Message-ID: References: <1586495895-9610-1-git-send-email-taox.zhu@intel.com> <20200410062227.24201-1-taox.zhu@intel.com> <60652C6914E08D41B9AA1580751B3CA9015ED8D2@SHSMSX103.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=konstantin.ananyev@intel.com; x-originating-ip: [192.198.151.172] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b2de2afe-938c-44b0-2ea1-08d7e066a1b3 x-ms-traffictypediagnostic: BYAPR11MB3192: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1824; x-forefront-prvs: 0373D94D15 x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(346002)(396003)(39850400004)(366004)(136003)(376002)(6506007)(53546011)(55016002)(64756008)(7696005)(2906002)(110136005)(33656002)(2940100002)(52536014)(26005)(9686003)(6636002)(76116006)(66476007)(66946007)(186003)(66556008)(66446008)(316002)(478600001)(5660300002)(8936002)(4326008)(54906003)(8676002)(71200400001)(86362001)(81156014)(309714004); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: FqwaJV64FqzoY3d4/xfLNrOpo9otYgH7tP8qPQE1tvlHaY9gtlu9J70hS36vXxdBSP3A80Hk/8i0F8gj1R5aAgwa5iX7UGl7/RyFcjhajGGXdOwZlk/pA3BGUXxV849kl3/eAxWPzU6pkHSjJi+Ly5MMAR/wrIz9oPX+c75jjeDrBoDkJBLC/jxpFAWm3f5R8IkIkIY2ianCmCy0HEZxx7mqJaB7oVDwWxE2RFv0HNt8WQOXzpXSoR3b6he2G8W1Xb/IlvOLU8VV4cMSFZddGKmtDymvhyQAJaVdaQWvSyAs1uDMbWujgC4FtWTWzgES2Y51iBAm1A/WMDeqYeSE+J8b+VOL/IPwZmbD6d+gT9WKmvAmp2ho8QPBtb58QIP7fcDTh+n6bmi9FSayFa+HsD8Fit5qXlWzU6FwLMSMcqxqLKd7bC9dR074kf3/ERvkMAGi2weWbvckdx3zWa3FtjKB207+cZbbtjOK/tX9a3c= x-ms-exchange-antispam-messagedata: 5RAZ6pQhY5/tCmZF4tAHUEKCg4iDDoF4nAWToVyOuOYWqTYM5QvM1It9LvEQic3H6ErWVKBa1/FjKMApMDJabtSpY1AhuErz/9v3K8DGT3GRUwNTK7zKd8YogZ/FJa0lihK5w12pCL4HqE89se2dfw== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b2de2afe-938c-44b0-2ea1-08d7e066a1b3 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Apr 2020 11:26:09.9565 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: WoV+L8VoiQyOOhUwQ0qn8C3wOlvmiGjRZ1Bt13gnwylad9PzVDsDB+cw3toqBKF9lH2Oze2ZFftB1SsMfDAvZOi5z2gNLzpHUVCFdBtFT8U= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3192 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix resource leak after thread exits normally X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > Hi Tao, >=20 >=20 > > Because ixgbe_dev_wait_setup_link_complete() doesn't guarantee that the= thread will always complete properly. So after the wait > > timeout, need to call pthread_cancel() to cancel the thread. >=20 > But pthread_cancel() also doesn't guarantee immediate thread termination. > You'll have to wait for link_thread_running again to make sure. > Another thing - in theory if link_thread was already terminated, > then its TID can be reused by other thread and we will terminate differen= t thread > (though chances are quite small). >=20 > > Definition of function ixgbe_dev_wait_setup_link_complete() as below: > > > > /* return 1: setup complete, return 0: setup not complete, and wait tim= eout*/ static int ixgbe_dev_wait_setup_link_complete(struct > > rte_eth_dev *dev) { #define DELAY_INTERVAL 100 /* 100ms */ As another thought - instead of unconditionally defining timeout inside the function itself, can we have it a s function parameter? Can caller can decide for how long he is ok to wait. > > #define MAX_TIMEOUT 90 /* 9s (90 * 100ms) in total */ > > struct ixgbe_adapter *ad =3D dev->data->dev_private; > > int timeout =3D MAX_TIMEOUT; > > > > while (rte_atomic32_read(&ad->link_thread_running) && timeout) = { > > msec_delay(DELAY_INTERVAL); > > timeout--; > > } > > > > > > return !!timeout; > > } > > > > BR, > > Zhu, Tao > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Friday, April 10, 2020 8:10 PM > > > To: Zhu, TaoX ; Lu, Wenzhuo > > > ; Ye, Xiaolong > > > Cc: dev@dpdk.org; martin.weiser@allegro-packets.com; stable@dpdk.org > > > Subject: RE: [PATCH v2] net/ixgbe: fix resource leak after thread exi= ts > > > normally > > > > > > Hi Tao, > > > > > > > From: Zhu Tao > > > > > > > > When the thread exits normally, pthread_join() is not called, which= can > > > > result in a resource leak. Therefore, the thread is set to separati= on > > > > mode using function pthread_detach(), so that no program call > > > > pthread_join() is required to recycle, and when the thread exits, > > > > the system automatically reclaims resources. > > > > > > > > Fixes: 819d0d1d57f1 ("net/ixgbe: fix blocking system events") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Zhu Tao > > > > --- > > > > drivers/net/ixgbe/ixgbe_ethdev.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > v2 changes: > > > > commit log. > > > > > > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > > > b/drivers/net/ixgbe/ixgbe_ethdev.c > > > > index 2c57976..f141ae4 100644 > > > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > > > @@ -4165,11 +4165,9 @@ static int > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > > > > ixgbe_dev_cancel_link_thread(struct rte_eth_dev *dev) > > > > { > > > > struct ixgbe_adapter *ad =3D dev->data->dev_private; > > > > -void *retval; > > > > > > > > if (!ixgbe_dev_wait_setup_link_complete(dev)) { > > > > pthread_cancel(ad->link_thread_tid); > > > > -pthread_join(ad->link_thread_tid, &retval); > > > > > > As you already waiting for link thread termination in > > > ixgbe_dev_wait_setup_link_complete(), do you still need > > > pthread_cancel() here? > > > > > > > rte_atomic32_clear(&ad->link_thread_running); > > > > PMD_DRV_LOG(ERR, "Link thread not complete, cancel it!"); > > > > } > > > > @@ -4186,6 +4184,7 @@ static int > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev, > > > > u32 speed; > > > > bool autoneg =3D false; > > > > > > > > +pthread_detach(pthread_self()); > > > > speed =3D hw->phy.autoneg_advertised; > > > > if (!speed) > > > > ixgbe_get_link_capabilities(hw, &speed, &autoneg); > > > > -- > > > > 1.8.3.1 > > > > >