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 A0B1841C3C; Wed, 8 Feb 2023 09:32:04 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 482F542D56; Wed, 8 Feb 2023 09:31:37 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 6E77240DFD for ; Wed, 8 Feb 2023 09:31:36 +0100 (CET) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] eal: introduce atomics abstraction Date: Wed, 8 Feb 2023 09:31:32 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8770D@smartserver.smartshare.dk> In-Reply-To: <20230208012040.GA22219@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] eal: introduce atomics abstraction Thread-Index: Adk7W5OnXOgM7tCORR+Kc94woi3JmgANUd+A References: <1673558785-24992-1-git-send-email-roretzla@linux.microsoft.com> <1673558785-24992-2-git-send-email-roretzla@linux.microsoft.com> <1844463.CQOukoFCf9@thomas> <20230201214111.GA30564@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230208012040.GA22219@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Honnappa Nagarahalli" Cc: , , , , , , , "nd" 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: Wednesday, 8 February 2023 02.21 >=20 > On Tue, Feb 07, 2023 at 11:34:14PM +0000, Honnappa Nagarahalli wrote: > > > > > > > > > > > > > > Honnappa, please could you give your view on the future of > atomics in > > > DPDK? > > > > Thanks Thomas, apologies it has taken me a while to get to this > discussion. > > > > > > > > IMO, we do not need DPDK's own abstractions. APIs from > stdatomic.h > > > (stdatomics as is called here) already serve the purpose. These > APIs are well > > > understood and documented. > > > > > > i agree that whatever atomics APIs we advocate for should align > with the > > > standard C atomics for the reasons you state including implied > semantics. > > Another point I want to make is, we need 'xxx_explicit' APIs only, = as > we want memory ordering explicitly provided at each call site. (This > can be discussed later). >=20 > i don't have any issue with removing the non-explicit versions. = they're > just just convenience for seq_cst anyway. if people don't want them we > don't have to have them. I agree with Honnappa on this point. The non-explicit versions are for lazy (or not so experienced) = developers, and might impact performance if used instead of the correct = explicit versions. I'm working on porting some of our application code from DPDK's = rte_atomic32 operations to modern atomics, and I'm temporarily using = acq_rel with a FIXME comment on each operation until I have the overview = to determine if another memory order is better for each operation. And = if I don't get around to fixing the memory order, it is still a step in = the right direct direction to get rid of the old __sync based atomics; = and the FIXME's remain to be fixed in a later release. So here's an idea: Alternatively to omitting the non-explicit versions, = we could include them for application developers, but document them as = placeholders for "memory order to be determined later" and emit a = warning when used. It might speed up the transition away from old atomic = operations. Alternatively, we risk thoughtless use of seq_cst with the = explicit versions, which might be difficult to detect in code reviews. =20 Either way, with or without non-explicit versions, is fine with me. >=20 > > > > > > > > > > > > > For environments where stdatomics are not supported, we could > have a > > > stdatomic.h in DPDK implementing the same APIs (we have to support > only > > > _explicit APIs). This allows the code to use stdatomics APIs and > when we move > > > to minimum supported standard C11, we just need to get rid of the > file in DPDK > > > repo. > > > > > > my concern with this is that if we provide a stdatomic.h or > introduce names > > > from stdatomic.h it's a violation of the C standard. > > > > > > references: > > > * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3. > > > * GNU libc manual > > > https://www.gnu.org/software/libc/manual/html_node/Reserved- > > > Names.html > > > > > > in effect the header, the names and in some instances namespaces > introduced > > > are reserved by the implementation. there are several reasons in > the GNU libc > > Wouldn't this apply only after the particular APIs were introduced? > i.e. it should not apply if the compiler does not support stdatomics. >=20 > yeah, i agree they're being a bit wishy washy in the wording, but i'm > not convinced glibc folks are documenting this as permissive guidance > against. >=20 > > > > > manual that explain the justification for these reservations and = if > if we think > > > about ODR and ABI compatibility we can conceive of others. > > > > > > i'll also remark that the inter-mingling of names from the POSIX > standard > > > implicitly exposed as a part of the EAL public API has been > problematic for > > > portability. > > These should be exposed as EAL APIs only when compiled with a > compiler that does not support stdatomics. >=20 > you don't necessarily compile dpdk, the application or its other > dynamically linked dependencies with the same compiler at the same > time. > i.e. basically the model of any dpdk-dev package on any linux > distribution. >=20 > if dpdk is built without real stdatomic types but the application has > to > interoperate with a different kit or library that does they would be > forced > to dance around dpdk with their own version of a shim to hide our > faked up stdatomics. >=20 So basically, if we want a binary DPDK distribution to be compatible = with a separate application build environment, they both have to = implement atomics the same way, i.e. agree on the ABI for atomics. Summing up, this leaves us with only two realistic options: 1. Go all in on C11 stdatomics, also requiring the application build = environment to support C11 stdatomics. 2. Provide our own DPDK atomics library. (As mentioned by Tyler, the third option - using C11 stdatomics inside = DPDK, and requiring a build environment without C11 stdatomics to = implement a shim - is not realistic!) I strongly want atomics to be available for use across inline and = compiled code; i.e. it must be possible for both compiled DPDK functions = and inline functions to perform atomic transactions on the same atomic = variable. So either we upgrade the DPDK build requirements to support C11 = (including the optional stdatomics), or we provide our own DPDK atomics. > > > > > > > > let's discuss this from here. if there's still overwhelming desire > to go this route > > > then we'll just do our best. > > > > > > ty