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 45D2042809; Wed, 22 Mar 2023 12:28:49 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CFD5F40E09; Wed, 22 Mar 2023 12:28:48 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id AF6BB40A84 for ; Wed, 22 Mar 2023 12:28:47 +0100 (CET) Content-class: urn:content-classes:message Subject: RE: [PATCH 0/7] replace rte atomics with GCC builtin atomics MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Date: Wed, 22 Mar 2023 12:28:44 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D877E1@smartserver.smartshare.dk> X-MimeOLE: Produced By Microsoft Exchange V6.5 In-Reply-To: <20230317214910.GA31884@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 0/7] replace rte atomics with GCC builtin atomics Thread-Index: AdlZGlIB9Ye3x89tRn2jpkXytiKx9QDlawiw References: <1679084388-19267-1-git-send-email-roretzla@linux.microsoft.com> <20230317144226.2f26bad1@hermes.local> <20230317214910.GA31884@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Stephen Hemminger" Cc: , , , 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 > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Friday, 17 March 2023 22.49 >=20 > 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. >=20 > i agree. >=20 > 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. >=20 > 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. Also, in a couple of the drivers, you are using int64_t for packet = counters. These cannot be negative and should be uint64_t. And AFAIK, = such counters can use RELAXED memory order.