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 C7EFB439D6; Fri, 26 Jan 2024 22:35:22 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 529F9402AB; Fri, 26 Jan 2024 22:35:22 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 3E94440289 for ; Fri, 26 Jan 2024 22:35:21 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 79F1420E61A1; Fri, 26 Jan 2024 13:35:20 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 79F1420E61A1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1706304920; bh=si4jaUYGKkUtCwQ/sPM6nL/GA4fEBFJakZTOOx2XphI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=WFnIsazMllYR7RKtc0Orl+tpJPmOWaq1JO0JaMRz2afHyA7kzhwdb6Y3qQHtqWEal Tzn2oHSIhwZU8V/wNkGnqNwxP0zA5zxyx5ABK6MoaPDS+X6oQX07kXi6+ud7Eggbtw QTnAcuna5ARdkuNh+QUp2RK8Gzi7HKbX1kRhq6hg= Date: Fri, 26 Jan 2024 13:35:20 -0800 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Mattias =?iso-8859-1?Q?R=F6nnblom?= , dev@dpdk.org, Tyler Retzlaff , konstantin.v.ananyev@yandex.ru, Mattias =?iso-8859-1?Q?R=F6nnblom?= , honnappa.nagarahalli@arm.com Subject: Re: rte_atomic_*_explicit Message-ID: <20240126213520.GB22822@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <0e268757-8368-456f-ba2f-10a1969c498f@lysator.liu.se> <98CBD80474FA8B44BF855DF32C47DC35E9F1A2@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35E9F1A8@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F1A8@smartserver.smartshare.dk> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Fri, Jan 26, 2024 at 11:52:11AM +0100, Morten Brørup wrote: > > From: Mattias Rönnblom [mailto:hofors@lysator.liu.se] > > Sent: Friday, 26 January 2024 09.07 > > > > On 2024-01-25 23:10, Morten Brørup wrote: > > >> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se] > > >> Sent: Thursday, 25 January 2024 19.54 > > >> > > >> Why do rte_stdatomic.h functions have the suffix "_explicit"? > > >> Especially > > >> since there aren't any wrappers for the implicit variants. > > >> > > >> More to type, more to read. > > > > > > They have the "_explicit" suffix to make their names similar to those > > in stdatomic.h. > > > > > > > OK, so to avoid a situation where someone accidentally misinterpret > > rte_atomic_fetch_add(&counter, 1, rte_memory_order_relaxed); > > as what, exactly? > > > > If you have a wrapper, why not take the opportunity and reap some > > benefits from this and fix/extend the standard API, making it a better > > fit for your application. Like removing the redundant "_explicit", > > "fetch"-less add/sub, maybe and a function for single-writer atomic add > > (non-atomic read + non-atomic add + atomic store), etc. > > > > Prohibiting implicit operations was done already, so it's already now > > not a straight-off copy of the standard API. > > > > > You might consider their existence somewhat temporary until C11 > > stdatomics can be fully phased in, so there's another argument for > > similar names. (This probably does not happen as long as compilers > > generate slower code for C11 stdatomics than with their atomic built- > > ins.) > > > > > > > To me, it seems reasonable a wrapper API should stay as long as it > > provide a benefit over the standard API. One could imagine that being a > > long time. > > > > I imagine some DPDK developers being tired of migrating from one > > atomics > > API to another. -> GCC built-ins (-> attempted C11 > > stdatomics?) -> . Now you are adding a future "-> CXY > > atomics" move as well, and with that it also seems natural to add a > > change back to a wrapper or complementary API, when CXY didn't turned > > out good enough for some particular platform, or when some non- > > complaint > > compiler comes along. > > Yes, more migrations seem to be on the roadmap. > > We can take the opportunity to change direction now, and decide to keep the API long term. > Then it would need more documentation (basically copying function descriptions from ), and the functions could have the "_explicit" suffix removed (macros with the suffix could be added for backwards compatibility), and more functions - like the ones you suggested above - could be added. > > What do people think? > 1. Keep considering a temporary wrapper for until compilers reach some undefined level of maturity, or > 2. Consider stable, clean it up (remove "_explicit" suffix), add documentation to the macros, and extend it. > > Living in a DPDK-only environment, I would prefer option 2; but if mixing DPDK code with non-DPDK code (that uses ) it might be weird. rte_stdatomic.h should be considered temporary, but how long temporary is depends on when we can deprecate support for distributions and the older toolchains they are tied to. the macros were introduced to allow a path to gradually moving to standard c11 atomics. gcc versions available on distributions we promise support for is currently the biggest barrier to direct adoption. * older versions of gcc generate sub-optimal code when using c11 atomic generics in some instances. * gcc c++23 support is incomplete, in particular at the time we evaluated using the c11 atomics directly but gcc c++23 held us back because the stdatomic.h it shipped didn't interoperate. * the macros allow developers to easily evaluate/compare with and without standard C atomics with a single build-time option -Denable_stdatomic=true * eventually we will want to move directly to the names from the standard, arguably just search and replace of rte_atomic with atomic is the most mechanically trivial - hence there is some value in keeping _explicit suffix. > > > > I suggested fixing the original API, or at least have a > > wrapper API, already at the point DPDK moved to direct GCC built-in > > calls. Then we wouldn't have had this atomics API ping-pong. > > The decision back then might have been too hasty, and based on incomplete assumptions. > Please shout louder next time you think a mistake is in the making. > > > > > >> > > >> When was this API introduced? Shouldn't it say "experimental" > > >> somewhere? > > > > > > They were introduced as part of the migration to C11. > > > I suppose they were not marked experimental because they replaced > > something we didn't want anymore (the compiler built-ins for atomics, > > e.g. __atomic_load_n()). I don't recall if we discussed experimental > > marking or not. > > In hindsight, they should probably have been marked "experimental". i'm not sure i feel strongly that they need to be marked experimental and by marked i assume we're only talking about placing a comment in a file rather than __rte_experimental which has no application here. typically we do that when we may want to change or remove the api without going through the normal deprecation and removal process. for these macros it is difficult to imagine why we would change them as that would only cause them to deviate from the signature or behavior of the standard C generics... why would we do that if our intention is to eventually fully migrate to direct use of the standard C names? ty