From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A672143437; Tue, 5 Dec 2023 04:13:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 389D9402C8; Tue, 5 Dec 2023 04:13:26 +0100 (CET) Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-vi1eur04on2051.outbound.protection.outlook.com [40.107.8.51]) by mails.dpdk.org (Postfix) with ESMTP id D37F240291 for ; Tue, 5 Dec 2023 04:13:24 +0100 (CET) ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=UW15U4uC9caNkl52ktVd8Bp1jcJp8BKJHyUHYXfM6z2EeIhwoUYdyTRNIx+307POzQspgBPheAOepEFl26CW6cPZ4YwYI3D1I//E5cFxF+dcM0LTSSaoHuYGlGG7MZf4CT2z2fl8brtOBfi7AzrtRvOgFtuXGvr4tlmo0fuEHRREdtjc0ukALeJTtFF6JJW/LKuRh3vCYOZET6FA9ySrIR+QWKgut1IesUCvHO42OExhVwZQ4zTnKj+KIHVXSYT1ujnS647lTZI1QuEtO1Uxk0pz/uMLc8MD+Lq2X4jRb759NmorwBGw5l2qw3TzgiP+SxaS/KVMITYZ4TX88o1C9A== ARC-Message-Signature: i=2; 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=UgjYZIxxvyVFUdZTcqVNriCRjrAshLzw53myfL9+UQY=; b=IxHReFCdqq+X5yCFRcrXZKursQI3UnFme8eK51cTk2iablAMR7UgRDCTrqlzhjBoRwJo7u1yYOgSfnt3F+7knFHXrOhaAd+B9+VTq6AZHbFozhkJ0drJA2ahSrNkSDInweuhmdHH/j+lgGPh9ILP1jJviNLvOKTIbFOgygNTqRU716/3e9Gg+T7wGKHwns9nmhWxDOX5ZAEQA66VZ3cxzyECiV+5/7Zcld09+O6O8nLCLQHcmtuH37BTRzkYjtXKzLmZ1FhMLPHY2R95kenfusPD9IAuI1XJK8/jOfT3NyB2UiplUTheSZJcP/f7weRfDzaB5YDAWAdz378IJaQXhA== ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=dpdk.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com]) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=UgjYZIxxvyVFUdZTcqVNriCRjrAshLzw53myfL9+UQY=; b=SNO6MuoGiPeyfr3PzWbGyLj+s0OcRswILHn2S7dHkUEFpQrQzvoJLTB5DlyxeWBZPNzliKvtWpVyBTnhAkfBVZ4SseZsfgJc4AyxXLpDMnGDpHAPLqIBXAYCi2sxPvh8IoPVXyCEd/ZQudU52yTHlsRlXLqVtUWMOKQmM3W57nE= Received: from AS8P250CA0022.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:330::27) by DB4PR08MB9238.eurprd08.prod.outlook.com (2603:10a6:10:3fa::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7046.34; Tue, 5 Dec 2023 03:13:21 +0000 Received: from AM2PEPF0001C70F.eurprd05.prod.outlook.com (2603:10a6:20b:330:cafe::e0) by AS8P250CA0022.outlook.office365.com (2603:10a6:20b:330::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7046.34 via Frontend Transport; Tue, 5 Dec 2023 03:13:21 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM2PEPF0001C70F.mail.protection.outlook.com (10.167.16.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7068.20 via Frontend Transport; Tue, 5 Dec 2023 03:13:21 +0000 Received: ("Tessian outbound 5d213238733f:v228"); Tue, 05 Dec 2023 03:13:21 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 17b387919fcf34fa X-CR-MTA-TID: 64aa7808 Received: from c78eb406997b.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 39395F98-75BD-45E1-B4FE-EC945377E306.1; Tue, 05 Dec 2023 03:13:15 +0000 Received: from EUR02-DB5-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id c78eb406997b.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 05 Dec 2023 03:13:15 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FUvNzfS0LEzF/AnMQYt7PvDJWAqJr0OD6jpZ+LvLmeQXyJ6QGsLHdQZJ86ykkhApv77g4cHsLQxdEVGI6n9RU3umQHhDdw0eNpg7hr7ZbiLTBvxHKdWhL5iTBP9pm+JC1306SKIcFkjTTb31PPXRndiBj3Nx65qrpcvte4O1rylXZxzRusVCWcypj6vCs7eXhQPqhG7NE/kTd4Q7Ikh1Z7mD0DwDvfnzcSyZ/q2yrv/6eIHuTeXrCs+pGXj6TGFnJE/J6fdyjJ9t5dCi34642DGdDE9gBIXQr4mTzE2J8/sv57FVmiyGIwGq2tWVJQThhMAz7/c3/hdJNq5ITuLtnQ== 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=UgjYZIxxvyVFUdZTcqVNriCRjrAshLzw53myfL9+UQY=; b=EeVqcqXDAR1lwkUU7VSumNAs4ZQfSY5rn+UCaflbZr1PzWJAfBJ3fcwbMVtsGtvlSvE/nh/MoE4lFlZvLNW9CcK8XvwU1wAIeOWr/Tx/2Gh8paIDtLS8tnC/iPd8wBafcBjzC9vQ4x4Jlq4iNsrz+d4GF7gnGJkyloPRPmsCCDayUot8NyYBEB8Bj0FC+B4a4LoROS5aalSyg24vgxfYTgr34y1IjbYtLAtyCxGUaChp1lLxCsuvDRrx9Hm4DaJcIeE8w6sT5aMKUOIbd6jjas2YuMf8s+IRYnRt74rNnlDWvC3CJlOzqlJJt7gaq7ElhgvF6AHY15F1M+h+KlBLFg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=UgjYZIxxvyVFUdZTcqVNriCRjrAshLzw53myfL9+UQY=; b=SNO6MuoGiPeyfr3PzWbGyLj+s0OcRswILHn2S7dHkUEFpQrQzvoJLTB5DlyxeWBZPNzliKvtWpVyBTnhAkfBVZ4SseZsfgJc4AyxXLpDMnGDpHAPLqIBXAYCi2sxPvh8IoPVXyCEd/ZQudU52yTHlsRlXLqVtUWMOKQmM3W57nE= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB7718.eurprd08.prod.outlook.com (2603:10a6:20b:50a::22) by PAWPR08MB8936.eurprd08.prod.outlook.com (2603:10a6:102:340::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7046.33; Tue, 5 Dec 2023 03:13:10 +0000 Received: from AS8PR08MB7718.eurprd08.prod.outlook.com ([fe80::4e1f:19b7:3948:6d14]) by AS8PR08MB7718.eurprd08.prod.outlook.com ([fe80::4e1f:19b7:3948:6d14%7]) with mapi id 15.20.7046.034; Tue, 5 Dec 2023 03:13:10 +0000 Message-ID: <19ea4729-bdfa-4ee3-83d6-5922c1b42c0a@arm.com> Date: Tue, 5 Dec 2023 11:13:04 +0800 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] mbuf: remove the redundant code for mbuf prefree To: =?UTF-8?Q?Morten_Br=C3=B8rup?= Cc: dev@dpdk.org, nd@arm.com, Ruifeng Wang References: <20231204023910.1714667-1-feifei.wang2@arm.com> <98CBD80474FA8B44BF855DF32C47DC35E9F081@smartserver.smartshare.dk> From: Feifei Wang In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F081@smartserver.smartshare.dk> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: SG2PR02CA0008.apcprd02.prod.outlook.com (2603:1096:3:17::20) To AS8PR08MB7718.eurprd08.prod.outlook.com (2603:10a6:20b:50a::22) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: AS8PR08MB7718:EE_|PAWPR08MB8936:EE_|AM2PEPF0001C70F:EE_|DB4PR08MB9238:EE_ X-MS-Office365-Filtering-Correlation-Id: 3b967751-be48-4cbf-e9d2-08dbf54022bd x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: 8q1j4NMZb47vMX/KUTpkAAz7a1rSW1JUj5LSckVzi8aXODRz/L2yKcTnq7F9M5P5itQCleVwkNfumUvpFdaizFofA5DDu672ZeW34TAqtGyg7fF4ixVnP4TTobvdgqCAyiv/oUehGFW9SBndWk8myRZk9aIbE838JMlTh/9t74wjDHKH6Mfsx+Eqk+xw7wU1W5UlbUlzE4c8ZQSMBnft1u3fc5WydYwAog9S2cViX3JMetE1CfImVJhEZq0N3Yv7hO5Mr1K4pzAlq+4GQJRjfj0BqR4VKXRqR/xSyO93hV2vc4n4oOxp0/+tw8HrscMGNuNV24/idZYNy6YwFveZCKIYTPJew3ulBhvP8z3RPL0s3SaxOgxTSymyplGXWVSNwNeaihC/FoywWu2X2hMQSevRZW6rOcav+MlGX+ePdcRjLQwGQfbmrXVHfM4p4I4CBdqEeEcACPlIDTv3odudTFthifQrpeBMMP9gg2EUsRJ18s8Qj35uo4lnaxBr6VUmoc4iPIu9FH/NS7oPwdRa7x0V+ZiezFWCEk+lm0Uvqnjsx2y0yZCmn2zDxh0+phUAcHNbToAY70ngtPeiYDK9ENioLMbXzJ7K9cYHI21/dc1+TbNy+s/pMu1qptiKRxwmeoLpadW6mUCCFqmtspSFfA== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AS8PR08MB7718.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(346002)(39860400002)(376002)(136003)(396003)(366004)(230922051799003)(1800799012)(451199024)(64100799003)(186009)(66476007)(66946007)(66556008)(316002)(6916009)(83380400001)(38100700002)(36756003)(8936002)(8676002)(4326008)(31696002)(86362001)(5660300002)(41300700001)(2906002)(31686004)(66574015)(2616005)(478600001)(26005)(6506007)(6666004)(6512007)(966005)(6486002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB8936 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM2PEPF0001C70F.eurprd05.prod.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 9c90f29d-e5fc-4dde-2089-08dbf5401c03 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 4yi7EenaBuu2Wy4s+OSnLkjH+oIssDvHc3X9SiXOy2hcGu933XT+iiTvaRMJj/40IinC+lWt/1b63sy9XaeNr8nef8kERX7HZIZT6zaz1fCrVnQ8ArTyCcdQ1yPzmuCH4xktwnYuYsJ/UMbwAQKgjs1N3iz2QVDfg/A5wr2n14WPYxR38ZF8zz5DvBvD1m0JhcJgvZaSDyGZISEgQjYECjsShIX/xuVgUXWU4xrU93WXSp7/yozGpqgtFWI7APE0VSDfjgn3HuxZ91hDKQZ8oUXK21Ghz/OQI4VM1J32mSzThqPFl8XeVScaRN+Em3WU1GK8HoWwFyTfv/PPUTfrcbr6Qu5uO10P4woCMVyj8R21wEOCHr6WqWGQpPHe8oLL4z0e4EwliA9HVcAXKpmOCjX6swkS/f5qc9WkzFwKMsQ+4V/RrU82Nq3mqG7jUz9uaXdukRWZ6J/CxmSo43XND9S1wR41tavKcImqMgn2d7SaV7yatVG0kpn2yaWu2YI0C5mhzZoBmgHWHcZzjjFM4aW28W97NVqxV+kScseaOhVWmBNQlU1z98mb/hjbnjujl5gol+Et7BKulxdvbmGYqj/EQBm+XqjaWXSWh1SdjoRkLA5Q1WYlL0UVPWg/jtg2iPNlPYuWQ4zp2mOvDx/Ssci3gVDHF2LK7GXzLUCXo/ufAzcE5jO6LOljLOE0f1qRoVIEJ0kDCe4rY8hguAqUv+blQDFoOi2ac6gtovsNbYTGHqyp3mQeodfn4s9dQH0aSCEHwSAzqjoZN5WA7eCuhA== X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFS:(13230031)(4636009)(39850400004)(396003)(346002)(136003)(376002)(230922051799003)(64100799003)(451199024)(1800799012)(82310400011)(186009)(40470700004)(36840700001)(46966006)(86362001)(966005)(6486002)(40460700003)(478600001)(6506007)(6512007)(6666004)(70206006)(70586007)(316002)(8936002)(4326008)(6862004)(8676002)(31696002)(81166007)(47076005)(356005)(36860700001)(83380400001)(82740400003)(2906002)(40480700001)(31686004)(36756003)(5660300002)(41300700001)(2616005)(66574015)(336012)(26005)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Dec 2023 03:13:21.2015 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3b967751-be48-4cbf-e9d2-08dbf54022bd X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM2PEPF0001C70F.eurprd05.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR08MB9238 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 2023/12/4 15:41, Morten Brørup 写道: >> From: Feifei Wang [mailto:feifei.wang2@arm.com] >> Sent: Monday, 4 December 2023 03.39 >> >> For 'rte_pktmbuf_prefree_seg' function, 'rte_mbuf_refcnt_read(m) == 1' >> and '__rte_mbuf_refcnt_update(m, -1) == 0' are the same cases where >> mbuf's refcnt value should be 1. Thus we can simplify the code and >> remove the redundant part. >> >> Furthermore, according to [1], when the mbuf is stored inside the >> mempool, the m->refcnt value should be 1. And then it is detached >> from its parent for an indirect mbuf. Thus change the description of >> 'rte_pktmbuf_prefree_seg' function. >> >> [1] https://patches.dpdk.org/project/dpdk/patch/20170404162807.20157-4- >> olivier.matz@6wind.com/ >> >> Suggested-by: Ruifeng Wang >> Signed-off-by: Feifei Wang >> --- >> lib/mbuf/rte_mbuf.h | 22 +++------------------- >> 1 file changed, 3 insertions(+), 19 deletions(-) >> >> diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h >> index 286b32b788..42e9b50d51 100644 >> --- a/lib/mbuf/rte_mbuf.h >> +++ b/lib/mbuf/rte_mbuf.h >> @@ -1328,7 +1328,7 @@ static inline int >> __rte_pktmbuf_pinned_extbuf_decref(struct rte_mbuf *m) >> * >> * This function does the same than a free, except that it does not >> * return the segment to its pool. >> - * It decreases the reference counter, and if it reaches 0, it is >> + * It decreases the reference counter, and if it reaches 1, it is > No, the original description is correct. > However, the reference counter is set to 1 when put back in the pool, as a shortcut so it isn't needed to be set back to 1 when allocated from the pool. Ok. for 'else if (__rte_mbuf_refcnt_update(m, -1) == 0)' case, it is easy to understand. but for '(likely(rte_mbuf_refcnt_read(m) == 1))' case, I think this will create misleading. dpdk users maybe difficult to understand why the code can not match the function description. Maybe we need some explanation here? >> * detached from its parent for an indirect mbuf. >> * >> * @param m >> @@ -1358,25 +1358,9 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m) > The preceding "if (likely(rte_mbuf_refcnt_read(m) == 1)) {" is only a shortcut for the likely case. Ok. >> m->nb_segs = 1; >> >> return m; >> - >> - } else if (__rte_mbuf_refcnt_update(m, -1) == 0) { >> - >> - if (!RTE_MBUF_DIRECT(m)) { >> - rte_pktmbuf_detach(m); >> - if (RTE_MBUF_HAS_EXTBUF(m) && >> - RTE_MBUF_HAS_PINNED_EXTBUF(m) && >> - __rte_pktmbuf_pinned_extbuf_decref(m)) >> - return NULL; >> - } >> - >> - if (m->next != NULL) >> - m->next = NULL; >> - if (m->nb_segs != 1) >> - m->nb_segs = 1; >> - rte_mbuf_refcnt_set(m, 1); >> - >> - return m; >> } >> + >> + __rte_mbuf_refcnt_update(m, -1); >> return NULL; >> } >> >> -- >> 2.25.1 > NAK. > > This patch is not race safe. With the patch: > > This thread: > if (likely(rte_mbuf_refcnt_read(m) == 1)) { // Assume it's 2. > > The other thread: > if (likely(rte_mbuf_refcnt_read(m) == 1)) { // It's 2. > __rte_mbuf_refcnt_update(m, -1); // Now it's 1. > return NULL; > > This thread: > __rte_mbuf_refcnt_update(m, -1); // Now it's 0. > return NULL; > > None of the threads have done the "prefree" work. > Agree. After we see the failture of unit_test, we realize that we ignored the mutiple thread case. Also maybe we need to add extra descripion to avoid misleading？