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 C71C443688; Wed, 6 Dec 2023 11:12:18 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 417D5402CF; Wed, 6 Dec 2023 11:12:18 +0100 (CET) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.12]) by mails.dpdk.org (Postfix) with ESMTP id 72148402C6 for ; Wed, 6 Dec 2023 11:12:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1701857537; x=1733393537; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=uurMX5POiu9mW22O/mOrXMPhH9D5yZ0nceAK1XCnuhY=; b=fnHxVgqSFSphpOzl4k5OO8TcwkOJg97mto2yJ7H/ksAAwC67cXT2KFCn 47sWurD32ZTHbEkesyV9xtIwBZwMzEdDED6HBo2x/uopsQJs7NeLrdP+D fPpHUgViY0EhED8SE6r0ICgFON0q28tLrbz2b5tVtAIMHqNzSBDNuVwW6 lu66v+0YwGhe5ufNbbFHaGVAD3yAWESy8IGY4DfBiZsV+FOWkgSy9o2wp U/A1W2cB5twUPabZ1sfq5qQ1OVtbx6P1wumyzzfRgJX467J357GHsN4l4 ZTc2jgMlOdZy29q5tu3+rISlYJjjaochL+DTM6CLpsnIvde2gxaISoRNN Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10915"; a="1105426" X-IronPort-AV: E=Sophos;i="6.04,254,1695711600"; d="scan'208";a="1105426" Received: from orviesa001.jf.intel.com ([10.64.159.141]) by orvoesa104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Dec 2023 02:12:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.04,254,1695711600"; d="scan'208";a="19278517" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orviesa001.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 06 Dec 2023 02:12:15 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Wed, 6 Dec 2023 02:12:14 -0800 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Wed, 6 Dec 2023 02:12:14 -0800 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.101) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Wed, 6 Dec 2023 02:12:14 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DPQprCy1zcAT6heieqN4hsw9VfNW8Qijj0F2kEXGynVqpBx7Ud84PWNhFQzgykODoiLSk+hKAz7KAIvqWnNdffmPZnvEgo4Wk4gOnUd34I1ddZwAF9nHFAR8fK1Jb5JV4QMhK1wn02jZ66mLOybYtgqOazXP5ZILNBTytgU7mwIf57dVEVUCZzKKehltdEQxWQHmMr7MuTQAwDyt6Dm63sYEDetUGis/OHLa+SVbTKV5n/SyJ6Xmo3xAe/c27mQDLa1kOjVr5Qh8TEve7HQyqTUFvWKBsrRsZalB44D5+jcdNzvouxFKe1gegFk8C6DAklBinjS4sCV8TA00B1lLrw== 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=Sy0cKqM8hsSFP++/RKOpcdQD9YXysjNhMR5yNNy6wfI=; b=X0vuqpP5jjyl/ZcgDv7vBnhqB3szp/2wn+LcMIBYHoap/1yM2aOPCTAn8lZpbek4BOUpN7BfPsnMCSrMFzMiC9N9kvSGmTW3+u8gTwSwBfh6xdF5nJg0RRHzLo36i4ANRQtcEp7wrNhvfcEkU0+ZBH9VLdyZ2SW4nRB55cHacmVSWd/Xz81z7F5sOB6Y1vXpqv1h1zgxT0G+XDKR1yKFUBMYywa/ttj9FUD32NOWv1afHP4/mvcXlSLWbBX9n9IkG/xZj20dlNPQ67L9Xaf67veq4h6CHSxPsq0yig+xoqca43MJt8XTSb7yx4x7xvRrwAQLAMOmvlw5xdsMZeMCxQ== 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 Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by PH7PR11MB8551.namprd11.prod.outlook.com (2603:10b6:510:30d::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7046.34; Wed, 6 Dec 2023 10:12:10 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::4782:d54a:209d:cb49]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::4782:d54a:209d:cb49%7]) with mapi id 15.20.7046.034; Wed, 6 Dec 2023 10:12:10 +0000 Date: Wed, 6 Dec 2023 10:12:05 +0000 From: Bruce Richardson To: Stephen Hemminger CC: Konstantin Ananyev , Morten =?iso-8859-1?Q?Br=F8rup?= , Feifei Wang , "dev@dpdk.org" , "nd@arm.com" , Ruifeng Wang Subject: Re: [PATCH v1] mbuf: remove the redundant code for mbuf prefree Message-ID: References: <20231204023910.1714667-1-feifei.wang2@arm.com> <98CBD80474FA8B44BF855DF32C47DC35E9F081@smartserver.smartshare.dk> <20231205105032.4b81e3da@hermes.local> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20231205105032.4b81e3da@hermes.local> X-ClientProxiedBy: DU2P251CA0022.EURP251.PROD.OUTLOOK.COM (2603:10a6:10:230::20) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|PH7PR11MB8551:EE_ X-MS-Office365-Filtering-Correlation-Id: 3a18adea-1cb4-4594-c969-08dbf643cf58 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fKSnazh+y4qxPpEGP4QNXs5ZsHbU8GekLR2WkgtcZKabnLIiapSTihItgmelM/7A+6ENvPGMrEkLngFCS/Gftw1Ow89d/gNXvbHRtJs3zI0d/rUz/ww8PcJZyPsBVCdeQY2oHC8a6IxU6J9Worr1bxvKoUFUGqVYG+sPbHDt/SpJwQba8BIPXZqRT35PVgdnz7j2c4Ml1piiv5wDcEz6WOGrNcjAq8T92hER/AsECLUrQ2n8x1ctuIEuZ2RqDGMnR5cmJckJZSplPIIaauGyWYIYUeVJ9Y2C7UK75BZBXKN2+mPBnNbSAT2qXZb4vbgtL22vH3ByReEzC+yquTNSXVsQkIOjpwgLGLAlm5zwZeJe689Jg+rYq/4swcMqKO3XIj+dELHdDGLSwQfF6boRj7ovSY/v+BGJ86OYmHijUaCAdLhpkpaafOglOdPSqXzR2QkWBLO9IggqgtyPIHUT+FmPqoOwoy/+U13ynSRyjGdpd5v7mrxQKCl7QEkPfh7+9YER4KnvonCEq+BiqSyUYdaqsKMkKfnjFsoS083GP4qZfsP2s1rnrUz5TGwsXOer X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230031)(366004)(39860400002)(136003)(346002)(396003)(376002)(230922051799003)(186009)(451199024)(1800799012)(64100799003)(2906002)(5660300002)(86362001)(41300700001)(38100700002)(82960400001)(6512007)(26005)(6506007)(6666004)(478600001)(6486002)(44832011)(54906003)(66946007)(83380400001)(66556008)(4326008)(316002)(66476007)(6916009)(8936002)(8676002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?r7GYnzxSz7o8BNlHZALKgWAbP8tX7hBJWHmHqIXTwZCXBOH1pACpsn+sTPuU?= =?us-ascii?Q?iDC+5yWVv73kr2CScp47zXG6GRNtdLzlVA7qhpZctbPGn19dahZMnEBFARQG?= =?us-ascii?Q?1TgzVKAmoq2f7YFYqaRhBPcFle/a66v3PjG85J+joEf63S2oVBXRyIrZY39p?= =?us-ascii?Q?HWqOFru02Qh0Nazwab6yPDlO2bmnKK3CMHYrOV9b4mq2nmDhd4X8QqOigplA?= =?us-ascii?Q?LubsNk7mEpWAxJ9PTRAI6u4jDWF49uOkSIbK0JB46DmW+Xe05wg1rhYfz/9E?= =?us-ascii?Q?HOEitqKEptGP8JiHfoJEsObMiIaFPU4C+TWqw3Nu7YJiuBVIows6x69DtNbE?= =?us-ascii?Q?g09+27nrplqJvUiNjYnRrWZxqjGpdI2N8USAJ1aw4ODQzslUcL84AWXnypi4?= =?us-ascii?Q?/QHkcUac1Gy1odT15Bp7XhPv8xjkBRAB3kZbi9gB1zsYjztMbJHEt29L81M7?= =?us-ascii?Q?fkZfyY6GXeb1pge4yP2jK/fitj3fJE+wnCOQesw/FcU4+B+TJ8Sh24NjY2QS?= =?us-ascii?Q?dDZsaiwThESssrfF0xNb3rELZ5Ibl0KB0I5yYFLWK45QJORwvRL9oixn8CnQ?= =?us-ascii?Q?ETJnwmoL0TYKWZ1D0slbNRC+NXlro/8OMahf0t0oZfsMMlk+BPEmhgtmcRwC?= =?us-ascii?Q?AN9CB8QNin/2FKTO318shPRPfB9a2GRz8lltPxjhp2+e2SbYQR0ykLL55y3w?= =?us-ascii?Q?UqYwTV2SK2OOZ/K++jAGaVKOARYYJpnupko/cZqgR/Wa3pfUCFe9U5cq+o5e?= =?us-ascii?Q?XnSsCiA15uhlONKYe4DO5BSuJ9SQce9OT0JP+wwL3rCBx+b305XXR0BxH4SJ?= =?us-ascii?Q?/P7h2TcalbJFR32KClsKHHUBZQ63rBKCqRJ7IQC78kYrfJlGgD11gZeOL2ED?= =?us-ascii?Q?lzWdEmzzOrs8pXqjyJcZnvGo6myMwBiNVopIEm7H+fp2v5EzMwly8RE5NdOU?= =?us-ascii?Q?xlYQWRuv/D1of4Qp66vyWBaedHjdWMZQEwsP1+55SFW6yAdkUuIfBBeLf1eN?= =?us-ascii?Q?uO1XorVH7Fts9DfEiXYF94QgYbcsg30pfpzlN+bgUKLZBEb6Se/45mKlA+S6?= =?us-ascii?Q?HxpkLw0k9uZ2zTgOeUssMmTDLSeCpvmZ695f7T3f58mKQOp0hq7xmGPuKB9N?= =?us-ascii?Q?mToVVOWlNRPal+fQfetCKdUYNS66SZSRYGfYoHYai77rneuYFzsBonFSn5hq?= =?us-ascii?Q?ijF8GwIQEaL4wnM/g0OgUqggLZjSo4hdB9qc8cHEJkBZ1OIW0jfbwuqSVkTQ?= =?us-ascii?Q?RxJyEFdK9Ul09xm4FmEkvpFNTSIMUKKUNrXKXWFlWtfYL5C31LhuVN4NBNvf?= =?us-ascii?Q?9SN2fEME33RTWKxpdgFcJDXEEkJnHCTAoXBvldPpYPnQu0SDoI6LXo48TdAp?= =?us-ascii?Q?3GKJn9bSoQNtrP2wcJgmlOcTr3z/DFY9KhWScOuDqmX4xZGQm0gqjSO/zxYt?= =?us-ascii?Q?PMLJAcVzzRtB/qIDK2QaGLISG1/zKK1f/RidZpzK6H98Mwg8hR/dFaHHaHoS?= =?us-ascii?Q?E7wuGCv4+GjSmAcUcNHUXJQqKU9ZlV6NZqCy1wK5HaPTwhUlkM6aPvvnzjhU?= =?us-ascii?Q?RdTUsmQ1RHgRJqKiPYYpZ9SApb4smL8ukdcThI/VQALqqyiaHT66M9pJuMq2?= =?us-ascii?Q?2g=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 3a18adea-1cb4-4594-c969-08dbf643cf58 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Dec 2023 10:12:10.7036 (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: XNUdQk075174PHgRUB6nUmoRHDSYX1MpyE5VoHVLZRvMk+rbztUfUeWyLyEkjm0doTbzpeAMu0DUCHsIO3RMcO8nu0vKTfj3Q5HEJc50aQA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB8551 X-OriginatorOrg: intel.com 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 On Tue, Dec 05, 2023 at 10:50:32AM -0800, Stephen Hemminger wrote: > On Mon, 4 Dec 2023 11:07:08 +0000 > Konstantin Ananyev wrote: > > > > > 2.25.1 > > > > > > NAK. > > > > > > This patch is not race safe. > > > > +1, It is a bad idea. > > The patch does raise a couple of issues that could be addressed by > rearranging. There is duplicate code, and there are no comments > to explain the rationale. > > Maybe something like the following (untested): > > diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h > index 286b32b788a5..b43c055fbe3f 100644 > --- a/lib/mbuf/rte_mbuf.h > +++ b/lib/mbuf/rte_mbuf.h > @@ -1342,42 +1342,32 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m) > { > __rte_mbuf_sanity_check(m, 0); > > - if (likely(rte_mbuf_refcnt_read(m) == 1)) { > - > - 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; > - > - 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; > + if (likely(rte_mbuf_refcnt_read(m) != 1) ) { == 1 > + /* If this is the only reference to the mbuf it can just > + * be setup for reuse without modifying reference count. > + */ > + } else if (unlikely(__rte_mbuf_refcnt_update(m, -1) == 0)) { > + /* This was last reference reset to 1 for recycling/free. */ > rte_mbuf_refcnt_set(m, 1); > + } else { > + /* mbuf is still in use */ > + return NULL; > + } > This seems much clearer with good comments. > - return m; > + 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; > } > - return NULL; > + > + if (m->next != NULL) > + m->next = NULL; > + if (m->nb_segs != 1) > + m->nb_segs = 1; > + return m; > } > > /**