Message ID | 20211012100204.5569-1-cyeaa@connect.ust.hk (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Raslan Darawsheh |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 65D71A0C47; Tue, 12 Oct 2021 12:02:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 00F394111B; Tue, 12 Oct 2021 12:02:18 +0200 (CEST) Received: from JPN01-OS2-obe.outbound.protection.outlook.com (mail-eopbgr1410131.outbound.protection.outlook.com [40.107.141.131]) by mails.dpdk.org (Postfix) with ESMTP id 5EE2A410E4; Tue, 12 Oct 2021 12:02:16 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eRxBUdUFIClsaby6jBBOeFGPHnA39Rt2HJ2I/ofOsyboYwzhQq5xFVd/OFf8hSq1cBqskakP2Vkd+3wJZXR4Qp4qlloAS0LcrqqFwUM6/6XPccci6nCkXCa//d01bfjFkGcTOEMkyRjpTs1uvbfSyYQq3SYh+JG0oEBCgwlp3t5qn2rHfN9r6ZVxSHxRD735UUzQ/sAMKLjG8ZlQN5yuoHBYtRjyy9YJ5VCTw7Gq8y5qrMpfF6VFBEuO287oB/pP18KPbh7Iim0waZINvkhvBb3U31vtm4FoRI82fYjly0TKi1hLlpGBixQ9hvsC9SriEqM8OD6HXR7NQUHhV+cwXw== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=aCMYJTl6wBN+8s7GJaLiwCsJJqImvFsfG+mKp6of7uo=; b=O64UVYIbDw971/6RmBwZ6WvkuFN1BWA/68WWA0K6CpWi6ZzjoiDDehKI1ePBPwVteUzNCBgqyLlzD20Rx2Hdk+2YP0CDVGGL8gybzxY7yUy7MRkN9VzAMtWEjvdzVx2nBrb6lWUMHn9JoOecTTEoSwQ1V8hKWdQRDfZiKtVpjwOYESCXFcILqXxf6BoXCnd27NPUa7wNfluvBQ72ZRtKKFvRBOuH91LaOH/1CQF+441kkvqy2FytBTVAXg4dBuLSEejlWLW7IhOQXjnkAxocNY2AdFikgofdTLHrfDdWkbS/L1UtCJZBDw0SOjSbq5qbB4borIhmLp8PST7yTsalJQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=connect.ust.hk; dmarc=pass action=none header.from=connect.ust.hk; dkim=pass header.d=connect.ust.hk; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=connect.ust.hk; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=aCMYJTl6wBN+8s7GJaLiwCsJJqImvFsfG+mKp6of7uo=; b=FoffkPSmVXeRPAvayWj/urMCoyAF8xnPig03J98l9f3NFt31W1eYLsnolKKfERyAW9m/pAFP2KV+nQCxSIv66nv0rVVUWjmSuEWvrf1UnVGVX+wc99RPcOwDvKfxE8OdpOmx0mUfcAkkLHCgwBHSvhqzCfjyy75X3QbrZ/v4AWE= Authentication-Results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=connect.ust.hk; Received: from TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM (2603:1096:400:b7::8) by TYCP286MB1281.JPNP286.PROD.OUTLOOK.COM (2603:1096:400:b8::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4587.18; Tue, 12 Oct 2021 10:02:13 +0000 Received: from TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM ([fe80::c0af:a534:cead:3a04]) by TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM ([fe80::c0af:a534:cead:3a04%7]) with mapi id 15.20.4587.026; Tue, 12 Oct 2021 10:02:13 +0000 From: Chengfeng Ye <cyeaa@connect.ust.hk> To: david.marchand@redhat.com, viacheslavo@nvidia.com, shahafs@nvidia.com, matan@nvidia.com Cc: dev@dpdk.org, Chengfeng Ye <cyeaa@connect.ust.hk>, stable@dpdk.org Date: Tue, 12 Oct 2021 03:02:04 -0700 Message-Id: <20211012100204.5569-1-cyeaa@connect.ust.hk> X-Mailer: git-send-email 2.17.1 Content-Type: text/plain X-ClientProxiedBy: HK0PR03CA0103.apcprd03.prod.outlook.com (2603:1096:203:b0::19) To TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM (2603:1096:400:b7::8) MIME-Version: 1.0 Received: from ubuntu.localdomain (175.159.124.155) by HK0PR03CA0103.apcprd03.prod.outlook.com (2603:1096:203:b0::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4587.25 via Frontend Transport; Tue, 12 Oct 2021 10:02:12 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: c941ac54-dd4d-4557-7599-08d98d675d08 X-MS-TrafficTypeDiagnostic: TYCP286MB1281: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: <TYCP286MB1281288A71F2053F8B492B078AB69@TYCP286MB1281.JPNP286.PROD.OUTLOOK.COM> X-MS-Oob-TLC-OOBClassifiers: OLM:185; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: w3L+hFp45e61WOdpMkQMjHPQSl8Lnn0iDuQC1+jYRGCLpHXNx8T52SlxTVZen6+sOYoc2YzmqsJ1LqjC8c4CizCrPyrvCWp+EenqEOtbgmB7/bh3FYttLuME+nbXpuF2tothmaPDi/wjfSxq0k2eTUvbEebYmRcmdND5kh2mzSKaI4WwwUn75qf15RvEBkWLk25dVQuK854+Qdhj4dKWxXRv1g9SSZCej2h4UzvXFSO41w2V0n+5RMGb3UJmDgvl7xEfPaT2ccBQNFemrPd65NvF4W8ETwxmvV4nboNl9f4y93NTA4hi/L/DHI1E2rR/ddVmdeMOnEDlHPT37z/PxQiAkTNuL3Sblj5eFlHrHvtbH/M1iPXo50q8u1V/ALdAcNd4qj9Yz7PB0gq8j2decZtBN7znoT2nuO3fHI5YrMEyrkV5GUsXwS2aGQ6u56ksbSllD1LDq2Sr7cXpsF9fEF93R2anoNSUMggrHIyf5GIHKm95ovuILqxKXd3h6V7Xcg9mMlAKTJ/8CS5PdEBmBqOIDR1xmLGXmUsvJ8PRTBjV0L6Rr3E4Ak1fjUD4quKnqVoayxm22EntCQRKstNO0xFnf2wWatBUqteIxDhWACe16H/giQH+Zg+bDa6Jtkan1lWaQ7Dvj3M12/pEUyLDv/CwQVc9HsFSx2p2vlSjjJWF+I0d/lBo84Y7Dt4vS/8Ecyrmsa3WTA5X+z4HzfH8pQ== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM; PTR:; CAT:NONE; SFS:(4636009)(366004)(316002)(8676002)(2906002)(786003)(8936002)(36756003)(86362001)(66946007)(26005)(66556008)(4326008)(38350700002)(1076003)(6486002)(2616005)(956004)(66476007)(4744005)(5660300002)(52116002)(38100700002)(6512007)(186003)(6666004)(83380400001)(508600001)(6506007); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: m57V85i69rVVXwkZryQ/Q8429ElTY1NndCimk+S0OkdEJIEjQbFHCJoLex8mgXEG0GMQ5Ng8p02Fg7ES0DvqrtUgNIAYwCv2A+nRzNYDQwwij6YVa9ABDrMJ7E7LfiJWM5lJbwJQVHWdWBDSALnGj0ZA9gWZ9pGpe2SCehgJ3NAdXcJXgVjKweJ46p/WuVVApdfeg7UzHJikfCFC5iJ/Fod+7Y3JvXDESeOl4y7A02oQQPDOODbFaTJJUFeGIARgPTBaDqEuUNlxfZDFFLbUDs8FVIPKvmgevSxvOKgxezkBN/rgHAbDRPdSP5SrRtO/bC/Bg/KfMXFUb8zi0um0JJW6c9odAPWbqDwTL97KaTYTQBBVn0+HDfkmIDFkpjhyZPJ80ZTovGzR0sMoAxlN7cCC2qL4seVo//opfrsyZCJOnNdlQhWYA5Vdv1EolAg0TT2eCGLf8SdqrzrxtWnu3FnjWeHKB/C+MVKOEsRPcmOu2oNqnmIXrbem1V1I5bSVAKZs+rNXHPjongXJloM+4Tur3Wfs10q5LpaDiPL/4pqCFE7NDUwCOI1XSyFiZPplJc2bb6wZiWVFJbjw454MWvoDds0HGz/nBaY5pn8sH9VoQmF7u/3BZlCoUQTACVQ9x2ixhp/c0Fj8CsW7+fOfiowclXy1IIDiQm9ADoDP5hMNvFhbE5/LcBcJ/0EQtShFGsii04hDZyuIrQzEoQ+gTfu47/XlTwAdQobrYs+vUmOC7wG+n2lD7JkxtvnwZ4nofMSE/slakaWN11RBhp+wicbT4/tA9wBEJz9LOED4yJmldWvfOzy0BdToou9lGrSNdp7dZxFZZpDKJQiGSpStXJhDS2tWez06MuJM/KLVBDTrgtCFRKileE2aaIPkxXyVICEsdpo8K2Oo15DRPIV6FC0nZRGQ6z18RERcVFcfLJZzo77KnVocG1yUJnvX7xEPetFjSb+rqD6BevTSNDGIuGc5iEdaw3zcGUhGuoPIfAQstRvYeMghXVt4NVHjdgebM4uB8ZsDTVwt72mpvQoetK93R+IhPJjSO6svri9XzfQOowf6krgRnUkCHtHK7z+zwxRlNFqP8PTGzMLRkjNv/Nm/86gPiv1Zn9/QMxGnVNv+s8m2wX/jTWp6tFRK1XdWZDwm6iWuVdSfwsVo8GDMECnAz+a5wGVEpE8Y1vXrBzGQ2+Sy3DLnJIhJs3/8rA/kbwIMGPpDSfGtx4axFZL1jPNPvB1ko5iA9tlIMv/opM7ssbe3HrljrWjNG4vBzEHMOcLyqPRPwvHb5extMkz4Z/MV7j2+yZUU8CTpSFVik58TnSgdCP5YyCaKOMk+8QdQ X-OriginatorOrg: connect.ust.hk X-MS-Exchange-CrossTenant-Network-Message-Id: c941ac54-dd4d-4557-7599-08d98d675d08 X-MS-Exchange-CrossTenant-AuthSource: TYCP286MB1188.JPNP286.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Oct 2021 10:02:13.3281 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 6c1d4152-39d0-44ca-88d9-b8d6ddca0708 X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: IygeQDwbMBfc4lM3SY4yNs/Pv8ur0w+rVxF4EQoQZQ5aDgl452ZC2vjxT7BySq6y4glHzr2EzuB2VlIkPR9FoA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYCP286MB1281 Subject: [dpdk-dev] [PATCH v5] net/mlx5: fix mutex unlock in txpp cleanup X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
[v5] net/mlx5: fix mutex unlock in txpp cleanup
|
|
Checks
Context | Check | Description |
---|---|---|
ci/checkpatch | warning | coding style issues |
ci/github-robot: build | success | github build: passed |
ci/Intel-compilation | success | Compilation OK |
ci/iol-broadcom-Functional | success | Functional Testing PASS |
ci/iol-broadcom-Performance | success | Performance Testing PASS |
ci/iol-x86_64-compile-testing | success | Testing PASS |
ci/iol-x86_64-unit-testing | success | Testing PASS |
ci/iol-mellanox-Performance | success | Performance Testing PASS |
ci/iol-aarch64-unit-testing | success | Testing PASS |
ci/intel-Testing | success | Testing PASS |
ci/iol-intel-Performance | success | Performance Testing PASS |
ci/iol-intel-Functional | success | Functional Testing PASS |
ci/iol-aarch64-compile-testing | success | Testing PASS |
Commit Message
YE Chengfeng
Oct. 12, 2021, 10:02 a.m. UTC
The lock sh->txpp.mutex was not correctly released on one path
of cleanup function return, potentially causing the deadlock.
Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing")
Cc: stable@dpdk.org
Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk>
---
drivers/net/mlx5/mlx5_txpp.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Comments
> -----Original Message----- > From: Chengfeng Ye <cyeaa@connect.ust.hk> > Sent: Tuesday, October 12, 2021 13:02 > To: david.marchand@redhat.com; Slava Ovsiienko > <viacheslavo@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Matan > Azrad <matan@nvidia.com> > Cc: dev@dpdk.org; Chengfeng Ye <cyeaa@connect.ust.hk>; > stable@dpdk.org > Subject: [PATCH v5] net/mlx5: fix mutex unlock in txpp cleanup > > The lock sh->txpp.mutex was not correctly released on one path of cleanup > function return, potentially causing the deadlock. > > Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing") > Cc: stable@dpdk.org > > Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Hi, > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Chengfeng Ye > Sent: Tuesday, October 12, 2021 1:02 PM > To: david.marchand@redhat.com; Slava Ovsiienko > <viacheslavo@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>; Matan > Azrad <matan@nvidia.com> > Cc: dev@dpdk.org; Chengfeng Ye <cyeaa@connect.ust.hk>; > stable@dpdk.org > Subject: [dpdk-dev] [PATCH v5] net/mlx5: fix mutex unlock in txpp cleanup > > The lock sh->txpp.mutex was not correctly released on one path of cleanup > function return, potentially causing the deadlock. > > Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing") > Cc: stable@dpdk.org > > Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> Patch applied to next-net-mlx, Kindest regards Raslan Darawsheh
On 10/12/2021 11:02 AM, Chengfeng Ye wrote: > The lock sh->txpp.mutex was not correctly released on one path > of cleanup function return, potentially causing the deadlock. > > Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing") > Cc: stable@dpdk.org > > Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> > --- > drivers/net/mlx5/mlx5_txpp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5_txpp.c b/drivers/net/mlx5/mlx5_txpp.c > index 4f6da9f2d1..0ece788a84 100644 > --- a/drivers/net/mlx5/mlx5_txpp.c > +++ b/drivers/net/mlx5/mlx5_txpp.c > @@ -961,8 +961,12 @@ mlx5_txpp_stop(struct rte_eth_dev *dev) > MLX5_ASSERT(!ret); > RTE_SET_USED(ret); > MLX5_ASSERT(sh->txpp.refcnt); > - if (!sh->txpp.refcnt || --sh->txpp.refcnt) > + if (!sh->txpp.refcnt || --sh->txpp.refcnt) { > + ret = pthread_mutex_unlock(&sh->txpp.mutex); > + MLX5_ASSERT(!ret); > + RTE_SET_USED(ret); Is this 'RTE_SET_USED()' need to be used multiple times for same variable? This usage looks ugly, I can see why it is used but I wonder if this can be solved differently, what about something like following: #ifdef RTE_LIBRTE_MLX5_DEBUG #define MLX5_ASSERT(exp) RTE_VERIFY(exp) #else #ifdef RTE_ENABLE_ASSERT #define MLX5_ASSERT(exp) RTE_ASSERT(exp) #else #define MLX5_ASSERT(exp) RTE_SET_USED(exp) #endif #endif > return; > + } > /* No references any more, do actual destroy. */ > mlx5_txpp_destroy(sh); > ret = pthread_mutex_unlock(&sh->txpp.mutex); >
Hi, Ferruh > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: Wednesday, November 10, 2021 18:57 > To: Chengfeng Ye <cyeaa@connect.ust.hk>; david.marchand@redhat.com; > Slava Ovsiienko <viacheslavo@nvidia.com>; Shahaf Shuler > <shahafs@nvidia.com>; Matan Azrad <matan@nvidia.com> > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5] net/mlx5: fix mutex unlock in txpp > cleanup > > On 10/12/2021 11:02 AM, Chengfeng Ye wrote: > > The lock sh->txpp.mutex was not correctly released on one path of > > cleanup function return, potentially causing the deadlock. > > > > Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing") > > Cc: stable@dpdk.org > > > > Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> > > --- > > drivers/net/mlx5/mlx5_txpp.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/mlx5/mlx5_txpp.c > > b/drivers/net/mlx5/mlx5_txpp.c index 4f6da9f2d1..0ece788a84 100644 > > --- a/drivers/net/mlx5/mlx5_txpp.c > > +++ b/drivers/net/mlx5/mlx5_txpp.c > > @@ -961,8 +961,12 @@ mlx5_txpp_stop(struct rte_eth_dev *dev) > > MLX5_ASSERT(!ret); > > RTE_SET_USED(ret); > > MLX5_ASSERT(sh->txpp.refcnt); > > - if (!sh->txpp.refcnt || --sh->txpp.refcnt) > > + if (!sh->txpp.refcnt || --sh->txpp.refcnt) { > > + ret = pthread_mutex_unlock(&sh->txpp.mutex); > > + MLX5_ASSERT(!ret); > > + RTE_SET_USED(ret); > > Is this 'RTE_SET_USED()' need to be used multiple times for same variable? mmm, It seems "claim_zero()" macro would be better here: claim_zero(pthread_mutex_lock(&sh->txpp.mutex)); I will provide the cleanup patch, thank you for noticing that > This usage looks ugly, I can see why it is used but I wonder if this can be > solved differently, what about something like following: > > #ifdef RTE_LIBRTE_MLX5_DEBUG > #define MLX5_ASSERT(exp) RTE_VERIFY(exp) > #else > #ifdef RTE_ENABLE_ASSERT > #define MLX5_ASSERT(exp) RTE_ASSERT(exp) > #else > #define MLX5_ASSERT(exp) RTE_SET_USED(exp) > #endif > #endif It would directly replace MLX5_ASSERT(exp) with RTE_SET_USED(exp) if there is neither RTE_ENABLE_ASSERT nor RTE_LIBRTE_MLX5_DEBUG. We would not like to drop the "not used" check functionality at all , right? With best regards, Slava
On 11/11/2021 7:06 AM, Slava Ovsiienko wrote: > Hi, Ferruh > >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yigit@intel.com> >> Sent: Wednesday, November 10, 2021 18:57 >> To: Chengfeng Ye <cyeaa@connect.ust.hk>; david.marchand@redhat.com; >> Slava Ovsiienko <viacheslavo@nvidia.com>; Shahaf Shuler >> <shahafs@nvidia.com>; Matan Azrad <matan@nvidia.com> >> Cc: dev@dpdk.org; stable@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v5] net/mlx5: fix mutex unlock in txpp >> cleanup >> >> On 10/12/2021 11:02 AM, Chengfeng Ye wrote: >>> The lock sh->txpp.mutex was not correctly released on one path of >>> cleanup function return, potentially causing the deadlock. >>> >>> Fixes: d133f4cdb7 ("net/mlx5: create clock queue for packet pacing") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Chengfeng Ye <cyeaa@connect.ust.hk> >>> --- >>> drivers/net/mlx5/mlx5_txpp.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/mlx5/mlx5_txpp.c >>> b/drivers/net/mlx5/mlx5_txpp.c index 4f6da9f2d1..0ece788a84 100644 >>> --- a/drivers/net/mlx5/mlx5_txpp.c >>> +++ b/drivers/net/mlx5/mlx5_txpp.c >>> @@ -961,8 +961,12 @@ mlx5_txpp_stop(struct rte_eth_dev *dev) >>> MLX5_ASSERT(!ret); >>> RTE_SET_USED(ret); >>> MLX5_ASSERT(sh->txpp.refcnt); >>> - if (!sh->txpp.refcnt || --sh->txpp.refcnt) >>> + if (!sh->txpp.refcnt || --sh->txpp.refcnt) { >>> + ret = pthread_mutex_unlock(&sh->txpp.mutex); >>> + MLX5_ASSERT(!ret); >>> + RTE_SET_USED(ret); >> >> Is this 'RTE_SET_USED()' need to be used multiple times for same variable? > mmm, It seems "claim_zero()" macro would be better here: > > claim_zero(pthread_mutex_lock(&sh->txpp.mutex)); > > I will provide the cleanup patch, thank you for noticing that > >> This usage looks ugly, I can see why it is used but I wonder if this can be >> solved differently, what about something like following: >> >> #ifdef RTE_LIBRTE_MLX5_DEBUG >> #define MLX5_ASSERT(exp) RTE_VERIFY(exp) >> #else >> #ifdef RTE_ENABLE_ASSERT >> #define MLX5_ASSERT(exp) RTE_ASSERT(exp) >> #else >> #define MLX5_ASSERT(exp) RTE_SET_USED(exp) >> #endif >> #endif > It would directly replace MLX5_ASSERT(exp) with RTE_SET_USED(exp) > if there is neither RTE_ENABLE_ASSERT nor RTE_LIBRTE_MLX5_DEBUG. > We would not like to drop the "not used" check functionality at all , right? > The suggestion was to prevent following kind of usage: MLX5_ASSERT(!ret); RTE_SET_USED(ret); I assume you need above usage when a variable is used only in the 'MLX5_ASSERT', if there is a way to prevent warning in that case without 'RTE_SET_USED' that may be better.
diff --git a/drivers/net/mlx5/mlx5_txpp.c b/drivers/net/mlx5/mlx5_txpp.c index 4f6da9f2d1..0ece788a84 100644 --- a/drivers/net/mlx5/mlx5_txpp.c +++ b/drivers/net/mlx5/mlx5_txpp.c @@ -961,8 +961,12 @@ mlx5_txpp_stop(struct rte_eth_dev *dev) MLX5_ASSERT(!ret); RTE_SET_USED(ret); MLX5_ASSERT(sh->txpp.refcnt); - if (!sh->txpp.refcnt || --sh->txpp.refcnt) + if (!sh->txpp.refcnt || --sh->txpp.refcnt) { + ret = pthread_mutex_unlock(&sh->txpp.mutex); + MLX5_ASSERT(!ret); + RTE_SET_USED(ret); return; + } /* No references any more, do actual destroy. */ mlx5_txpp_destroy(sh); ret = pthread_mutex_unlock(&sh->txpp.mutex);