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 E1F4C42A39; Tue, 2 May 2023 06:31:41 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8073E410FB; Tue, 2 May 2023 06:31:41 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2050.outbound.protection.outlook.com [40.107.21.50]) by mails.dpdk.org (Postfix) with ESMTP id A3CA540E2D for ; Tue, 2 May 2023 06:31:39 +0200 (CEST) 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=qjIa1vwguVEOjlHaui4V8XssVDysCcv3SqraeiZLDnY=; b=Y3yWd5c81p+AGtRil3GzWRLp0ec1XEWeY1M4buaLq5jxNMKfZmef6u7NktixBwBcqrcIgV+0hzwbqe5+WDYhO+l1RI1BcAIHBkJYti23NMVrrCgtRVhFFIS7Y9mtajZXz1m70VRA3/6p4ajMKp88TvvnWCuO7jeex9oTbS3bmF8= Received: from DB7PR05CA0054.eurprd05.prod.outlook.com (2603:10a6:10:2e::31) by PAWPR08MB8789.eurprd08.prod.outlook.com (2603:10a6:102:332::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.30; Tue, 2 May 2023 04:31:37 +0000 Received: from DBAEUR03FT051.eop-EUR03.prod.protection.outlook.com (2603:10a6:10:2e:cafe::c9) by DB7PR05CA0054.outlook.office365.com (2603:10a6:10:2e::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.31 via Frontend Transport; Tue, 2 May 2023 04:31:37 +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 DBAEUR03FT051.mail.protection.outlook.com (100.127.142.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.20 via Frontend Transport; Tue, 2 May 2023 04:31:37 +0000 Received: ("Tessian outbound e13c2446394c:v136"); Tue, 02 May 2023 04:31:36 +0000 X-CR-MTA-TID: 64aa7808 Received: from 614d9071c658.3 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 05005DD1-208A-47F7-83A5-6301F442442A.1; Tue, 02 May 2023 04:31:30 +0000 Received: from EUR02-DB5-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 614d9071c658.3 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Tue, 02 May 2023 04:31:30 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FzNdd1cWaTWoR1MW8Odkz7oDHWs79TMdt81huHjEgY+KD3+/niMRZxbQtbtSSoKmyHUOg/KtaPda0BuN/FGRlgqVqrXPwFOVAW4aR/9TOziTxZXjGdTRaX7dp+rJRoEC+89jGk6ki+yrDJtdkCD8j5WhS5wOQWadqRINpRAVQfHsOMDW2UKZ2jSyebGLlRsDU8nq6XI4d1KeIOJU3ReCn+0gPBBNRlGMavY1c7hboAtYuCYWKW2GF4koNUlFmnq8o7Gi3ifE15MMEyH91H0bG42WffLmdPiApHAhLdrFk2DzxomyiL/7ILySmmjOuBHzJ9q3TzossqRfTeMbX0FKMg== 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=qjIa1vwguVEOjlHaui4V8XssVDysCcv3SqraeiZLDnY=; b=i2DcDTjCSj0wvP+dxq9mS7dH2OZcKcktTWs1HD4YX2A6i8gvL5eGirRoq1/9XgoJiRcaets1trSytUh8dQa58OfWuoFVfXofcOmlA0nuSCFlI2o1pwuhUv35gZBJLwRS7Ga7+yG3Ani3S3YYdl2UTVaTBdigtInojhaLHviSBvpe+iAEuims4eF9mYiut5tWc7JL8XJaHFqH6a2l8Ph0wlch81rooJRaUe7DI689u3czLAQZK+F302TVIsJCvaSMMWJzHjbSlgGsxiVreKDO8mm84IcDr0usbg42esZZGzLZBaUsbDTdBccrIVS73tCyh6qkCu03QFAx8J0ar5dbCQ== 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=qjIa1vwguVEOjlHaui4V8XssVDysCcv3SqraeiZLDnY=; b=Y3yWd5c81p+AGtRil3GzWRLp0ec1XEWeY1M4buaLq5jxNMKfZmef6u7NktixBwBcqrcIgV+0hzwbqe5+WDYhO+l1RI1BcAIHBkJYti23NMVrrCgtRVhFFIS7Y9mtajZXz1m70VRA3/6p4ajMKp88TvvnWCuO7jeex9oTbS3bmF8= Received: from DBAPR08MB5814.eurprd08.prod.outlook.com (2603:10a6:10:1b1::6) by AS8PR08MB6136.eurprd08.prod.outlook.com (2603:10a6:20b:292::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.31; Tue, 2 May 2023 04:31:28 +0000 Received: from DBAPR08MB5814.eurprd08.prod.outlook.com ([fe80::621c:838a:cb11:19b7]) by DBAPR08MB5814.eurprd08.prod.outlook.com ([fe80::621c:838a:cb11:19b7%7]) with mapi id 15.20.6340.031; Tue, 2 May 2023 04:31:28 +0000 From: Honnappa Nagarahalli To: Tyler Retzlaff CC: =?iso-8859-1?Q?Morten_Br=F8rup?= , Stephen Hemminger , "dev@dpdk.org" , Ruifeng Wang , "thomas@monjalon.net" , nd , Wathsala Wathawana Vithanage , nd Subject: RE: [PATCH 0/7] replace rte atomics with GCC builtin atomics Thread-Topic: [PATCH 0/7] replace rte atomics with GCC builtin atomics Thread-Index: Adlc0x2jp4hZ/JHqTt+4XE4Z0dKiCQAAqo6gAAGcVLAAAOhc4AAAnqUAAAGpMQAH757JgAABwqCA Date: Tue, 2 May 2023 04:31:27 +0000 Message-ID: References: <20230317214910.GA31884@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <98CBD80474FA8B44BF855DF32C47DC35D877E1@smartserver.smartshare.dk> <20230322142134.GA29057@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <98CBD80474FA8B44BF855DF32C47DC35D877E6@smartserver.smartshare.dk> <20230322152932.GB29057@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <98CBD80474FA8B44BF855DF32C47DC35D877E8@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D877E9@smartserver.smartshare.dk> <20230322180608.GA28785@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230502033745.GA28467@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> In-Reply-To: <20230502033745.GA28467@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: FADCF6FD0E267D48BAB94C15C7BBE595.0 x-checkrecipientchecked: true Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; x-ms-traffictypediagnostic: DBAPR08MB5814:EE_|AS8PR08MB6136:EE_|DBAEUR03FT051:EE_|PAWPR08MB8789:EE_ X-MS-Office365-Filtering-Correlation-Id: 7dc2eacb-c97e-45dc-909c-08db4ac61e08 x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr 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: I7SnSX1iydANwZtAD9ucBezeKYimyCatelzWLSX8oSVUeqW3HPHbdAV+pEbdDQH/y3P4c+FWACtlzIaiLk8QNKcl8aU3D0sxt3DdDoxtNWZe3LdLaWc2uLeVAe/SE5cGq8/2P+blxHFXaDPhDe/T4HcySkhyyI5fF0MgSLzLJuX7dag/XkguZXSHHwX8d1OCaxSL/B/557NuPjSdsB9Vs2xuniHVdaxnQw0Yyf5eVvwRuXRUfF9r3pGBFpJGvKxx2USgiKge11g1JOwANG8lBN657BUXbb2vUTG6T/aQg5f38WOrJPJsQ/Y6m+52lAcMXkPdrZb2qC5LpJppECDTJUpbyKDVISWt1P7XQ76bPr30KKYkisyH8yeOejVxC9nRePv3r44wQai1WaDoN/Om7bS2BXMslF4JJx/EqgQlcRD9Du9WD5B5dIebjrJqhOlIfYq8+LJn94KSJxBDBPuB/XICuA3k3x0ZiUvtaMipSHemrBK7Y3VsF20Pl3MqJQ2u73riLPodoxPkz81q53N+fzEh8V3mDMldgKoSSxbWy2qVitufLVUVVblcepup85KaiqHb4jMEzTUUN0PcRId6FCanlj/CEEzrgba1PNOVeAEyzTKna7PeV5zECFEBRr9Y X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DBAPR08MB5814.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230028)(4636009)(376002)(396003)(366004)(346002)(136003)(39860400002)(451199021)(38100700002)(33656002)(8676002)(8936002)(2906002)(5660300002)(52536014)(76116006)(316002)(64756008)(66446008)(66476007)(66556008)(66946007)(6916009)(4326008)(122000001)(41300700001)(38070700005)(30864003)(55016003)(86362001)(71200400001)(7696005)(478600001)(6506007)(53546011)(9686003)(186003)(966005)(66574015)(83380400001)(54906003)(21314003); DIR:OUT; SFP:1101; Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB6136 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: DBAEUR03FT051.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 338c8974-36f9-4791-df92-08db4ac6184d X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: cTxjIkGymgVoWohYopIvMwdf9cFeiTrMLKng274pHMWZAVVYjGhZc8QwvAL0moTskOweJagyvjkPHndSiI7X3phHxns8z+ZetGqc3rYH3AIvQoecSxf/iAERhI2Zcy9E7MPKRCUqn/28bFCIHxWY5U60E9Sx5jaPDrJ+ba3aKwIR5s4ue0pdpjph91jOsX87PX/gH2H/Y4IVyPbWgJLRyIf3zZY5TrJHpmV2S9b2s0H4xml0E/04jDTR57xtztnE8sxtT3KXWWGeG4Pl4hLzJuFXgBgL43Rn6fIOI+tz6VGdpc1o8rD4xJH1D5xMMnJFJEZ9S+Pe9Phi8U5NwTjYcqErRayoV09XGl9LEGIDo6GuD624fpKSM4PZpiLEuDxJVrDkj6INgTSGBh2cVDQ+TNG0oC9key8J89m9+/ufyaW/6GhWGZYLKZaP0kyPFblF/fqvymm/GnyoPINs/WjKKidsf7boj+ASBg+SQyZR0Qtjp6RBQqsMV93SrDPNK75vRAPZ3bknVcwFR93cbln+Q8jDtrCN1dpRJ13KpWnDYsE4QLo3NacD6QzEf/wgOwcXkvOg5Xetm++wblrzH38V/rPTZlzOYYEJUhGfA8rW46IN9u2eRarhvMwCLNnpWXKiSCPeTzI9+tBDENomgorhoCs/AlROuweOT1ylGP+rlfB3oUQ9jmCDu0a4wkOlPp75lZqj0C3ub2lzEke17eEa1rLrz5FLIySJdDcbKLwqOYuVaogE9arEtHbV4pSdMxqc 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:(13230028)(4636009)(396003)(136003)(376002)(39860400002)(346002)(451199021)(46966006)(36840700001)(40470700004)(41300700001)(6862004)(8676002)(8936002)(5660300002)(52536014)(86362001)(33656002)(66574015)(83380400001)(47076005)(36860700001)(34020700004)(356005)(81166007)(82740400003)(40460700003)(55016003)(26005)(966005)(316002)(40480700001)(478600001)(336012)(4326008)(54906003)(70586007)(70206006)(2906002)(30864003)(7696005)(186003)(6506007)(53546011)(9686003)(82310400005)(21314003); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2023 04:31:37.0914 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 7dc2eacb-c97e-45dc-909c-08db4ac61e08 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: DBAEUR03FT051.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB8789 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 > -----Original Message----- > From: Tyler Retzlaff > Sent: Monday, May 1, 2023 10:38 PM > To: Honnappa Nagarahalli > Cc: Morten Br=F8rup ; Stephen Hemminger > ; dev@dpdk.org; Ruifeng Wang > ; thomas@monjalon.net; nd > Subject: Re: [PATCH 0/7] replace rte atomics with GCC builtin atomics >=20 >=20 > On Wed, Mar 22, 2023 at 11:06:08AM -0700, Tyler Retzlaff wrote: > > On Wed, Mar 22, 2023 at 05:38:12PM +0000, Honnappa Nagarahalli wrote: > > > > > > > > > > -----Original Message----- > > > > From: Morten Br=EF=BF=BDrup > > > > Sent: Wednesday, March 22, 2023 12:08 PM > > > > To: Honnappa Nagarahalli ; Tyler > > > > Retzlaff > > > > Cc: Stephen Hemminger ; > dev@dpdk.org; > > > > Ruifeng Wang ; thomas@monjalon.net; nd > > > > ; nd > > > > Subject: RE: [PATCH 0/7] replace rte atomics with GCC builtin > > > > atomics > > > > > > > > > From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com] > > > > > Sent: Wednesday, 22 March 2023 17.40 > > > > > > > > > > > From: Morten Br=EF=BF=BDrup > > > > > > Sent: Wednesday, March 22, 2023 11:14 AM > > > > > > > > > > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > > > > > > > Sent: Wednesday, 22 March 2023 16.30 > > > > > > > > > > > > > > On Wed, Mar 22, 2023 at 03:58:07PM +0100, Morten Br=EF=BF=BDr= up > wrote: > > > > > > > > > From: Tyler Retzlaff > > > > > > > > > [mailto:roretzla@linux.microsoft.com] > > > > > > > > > Sent: Wednesday, 22 March 2023 15.22 > > > > > > > > > > > > > > > > > > On Wed, Mar 22, 2023 at 12:28:44PM +0100, Morten Br=EF=BF= =BDrup > wrote: > > > > > > > > > > > From: Tyler Retzlaff > > > > > > > > > > > [mailto:roretzla@linux.microsoft.com] > > > > > > > > > > > Sent: Friday, 17 March 2023 22.49 > > > > > > > > > > > > > > > > > > > > > > On Fri, Mar 17, 2023 at 02:42:26PM -0700, Stephen > > > > > > > > > > > Hemminger > > > > > > wrote: > > > > > > > > > > > > On Fri, 17 Mar 2023 13:19:41 -0700 Tyler Retzlaff > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Replace the use of rte_atomic.h types and > > > > > > > > > > > > > functions, instead use > > > > > > > GCC > > > > > > > > > > > > > supplied C++11 memory model builtins. > > > > > > > > > > > > > > > > > > > > > > > > > > This series covers the libraries and drivers > > > > > > > > > > > > > that are built on > > > > > > > > > Windows. > > > > > > > > > > > > > > > > > > > > > > > > > > The code has be converted to use the __atomic > > > > > > > > > > > > > builtins > > > > > but > > > > > > > > > > > > > there > > > > > > > are > > > > > > > > > > > > > additional during conversion i notice that there > > > > > > > > > > > > > may be some > > > > > > > issues > > > > > > > > > > > > > that need to be addressed. > > > > > > > > > > > > > > > > > > > > > > > > I don't think all these cmpset need to use SEQ_CST. > > > > > > > > > > > > Especially for the places where it is used a loop, > > > > > > > > > > > > might > > > > > be > > > > > > > > > > > > more efficient with some of the other memory models= . > > > > > > > > > > > > > > > > > > > > > > i agree. > > > > > > > > > > > > > > > > > > > > > > however, i'm not trying to improve the code with > > > > > > > > > > > this > > > > > change, > > > > > > > > > > > just decouple it from rte_atomics.h so trying my > > > > > > > > > > > best to > > > > > avoid > > > > > > > > > > > any unnecessary semantic change. > > > > > > > > > > > > > > > > > > > > > > certainly if the maintainers of this code wish to > > > > > > > > > > > weaken the ordering where appropriate after the > > > > > > > > > > > change is merged they > > > > > can > > > > > > > > > > > do so and > > > > > > > handily > > > > > > > > > > > this change has enabled them to do so easily > > > > > > > > > > > allowing them > > > > > to > > > > > > > > > > > test > > > > > > > just > > > > > > > > > > > their change in isolation. > > > > > > > > > > > > > > > > > > > > I agree with the two-step approach, where this first > > > > > > > > > > step is a simple > > > > > > > > > search-and-replacement; but I insist that you add a > > > > > > > > > FIXME or similar note where you have blindly used > > > > > > > > > SEQ_CST, indicating > > > > > that > > > > > > > > > the memory order > > > > > > > needs to > > > > > > > > > be reviewed and potentially corrected. > > > > > > > > > > > > > > > > > > i think the maintainers need to take some > > > > > > > > > responsibility, if > > > > > they > > > > > > > > > see optimizations they missed when previously writing > > > > > > > > > the code they need to follow up with a patch themselves. > > > > > > > > > i can't do everything for them and marking things i'm > > > > > > > > > not sure about will only lead to me having to churn > > > > > > > > > patch series to remove the > > > > > unwanted > > > > > > comments later. > > > > > > > > > > > > > > > > The previous atomic functions didn't have the "memory order= " > > > > > > > > parameter, so > > > > > > > the maintainers didn't have to think about it - and thus > > > > > > > they didn't miss any optimizations when accepting the code. > > > > > > > > > > > > > > > > I also agree 100 % that it is not your responsibility to > > > > > > > > consider > > > > > or > > > > > > > determine which memory order is appropriate! > > > > > > > > > > > > > > > > But I think you should mark the locations where you are > > > > > > > > changing from the > > > > > > > old rte_atomic functions (where no memory order optimization > > > > > > > was > > > > > > > available) to the new functions - to highlight where the > > > > > > > option of memory ordering has been introduced and knowingly > ignored (by you). > > > > > > > > > > > > > > > > > > > > > > first, i have to apologize i confused myself about which of > > > > > > > the many patch series i have up right now that you were comme= nting > on. > > > > > > > > > > > > No worries... you are rushing through quite an effort for > > > > > > this, so a > > > > > little > > > > > > confusion is perfectly understandable. Especially when I'm > > > > > > replying to > > > > > an ageing > > > > > > email. :-) > > > > > > > > > > > > > > > > > > > > let me ask for clarification in relation to this series. > > > > > > > > > > > > > > isn't that every single usage of the rte_atomic APIs? > > > > > > > > > > > > Probably, yes. > > > > > > > > > > > > > i mean are you > > > > > > > literally asking for the entire patch series to look like > > > > > > > the following patch snippet with the expectation that > > > > > > > maintainers will come along and clean up/review after this se= ries is > merged? > > > > > > > > > > > > > > -rte_atomic_add32(&o, v); > > > > > > > +//FIXME: opportunity for relaxing ordering constraint, > > > > > > > +please > > > > > review > > > > > > > +__atomic_fetch_add(&o, v, order); > > > > > > > > > > > > Exactly. And something similar for the rte_atomicXX_t > > > > > > variables > > > > > changed to > > > > > > intXX_t, such as the packet counters. > > > > > > > > > > > > Realistically, I don't expect the maintainers to clean them up > > > > > > anytime > > > > > soon. The > > > > > > purpose is to make the FIXMEs stick until someone eventually > > > > > > cleans > > > > > them up, so > > > > > > they are not forgotten as time passes. > > > > > Cleaning up the rte_atomic APIs is a different effort. There is > > > > > already lot of effort that has gone into this and there is more > > > > > effort happening (rte_ring being a painful one) > > > > > > > > > > Instead of having FIXME, why not just send a separate patch with > > > > > SEQ_CST (still a search and replace)? We can leave the tougher > > > > > ones like rte_ring as they are being worked on. > > > > > > > > The FIXME makes it possible in the future to differentiate between > > > > the instances that still need review and the instances that have > > > > been reviewed where SEQ_CST was the correct choice. (Similarly for > > > > the choice of type for variables previously rte_atomicNN_t.) > > > Apologies, relooked at the heading of this patch, got confused with o= ther > patches. > > > > yeah, i did the same thing this morning :) > > > > > > > > The changes Arm had done for rte_atomic_ to __atomic_xxx were not dir= ect > replacements. The algorithms were studied, relaxed where required, race > conditions fixed, performance benchmarked. IMO, we need to go through the > same steps here. > > > > > > I looked at the series, we should just review the patch and make sugg= ested > changes. Are we constrained by any deadlines for this work? > > > > i'm going to say yes but i'll qualify. the use of the rte_atomic_xxx > > APIs drags in extra work when creating a series that performs the > > actual conversions to the standard atomics. > > > > if i don't decouple ring from rte_atomic_xxx that means i have to go > > convert all the rte_atomic.h to standard atomics and working around > > some of the implementation detail to do it is very time consuming. > > which then has further flow on effects because then i have to go fix > > every single driver that is still using rte_atomic.h. > > > > incidentally i have a work in progress to decouple everything from > > rte_atomic.h (including all drivers) but it would really negatively > > impact getting standard atomics introduced if we had to serialize the > > introduction behind a total removal of rte_atomic or had to make > > changes to every consumer of the old rte_atomic APIs. > > > > if we can get by with a comment on the rte_atomic_xxx lines in this > > series it would be helpful. when we bring the next series for standard > > atomics i'm not adverse to introducing changes to the ordering in that > > series if requested so long as i can get the series up 'soon' so there > > is lots of review time runway for 23.11. > > > > > > > > I would suggest to drop 1/7. Arm is working on removing the non-C11 > algorithm for rte_ring (not sure if we will be successful). I think it is= better to > explore this approach rather than the changes in patch 1/7. > > > > i think my answer here is timing. i'd rather take the work from arm > > but if it isn't coming for a while then it becomes a blocker. > > > > we're waiting for the 23.07 start before this series can be merged. > > how about we re-evaluate where arm is at when the merge window opens. > > we can then decide to drop 1/7 or not at that time? >=20 > ping? >=20 > any update if there is going to be a series from arm as an acceptable > replacement for patch 1/7? otherwise i think we should take the patch as = is. it > isn't altering the semantics of the code and is fairly low line count cha= nge so > shouldn't distrupt any out of tree work as a result of the churn. Yes, we are working on a patch. There is a RFC [1], but we are still workin= g on proving if the algorithm is correct. But, the plan is to find a soluti= on (or present alternatives if there are no solutions) in 23.07 release. [1] https://patchwork.dpdk.org/project/dpdk/patch/20230421191642.217011-1-w= athsala.vithanage@arm.com/ >=20 > please update asap, this is one of the two series that is preventing subm= ission of > the first series converting to standard atomics for review. >=20 > thanks! >=20 > > > > ty