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 40B9041C49; Thu, 9 Feb 2023 09:34:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2DFA940DF8; Thu, 9 Feb 2023 09:34:07 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 940914067B; Thu, 9 Feb 2023 09:34:06 +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: Thu, 9 Feb 2023 09:34:03 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87719@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] eal: introduce atomics abstraction Thread-Index: AQHZNcVOKaGjm++r5EuVscptDtirzq65Kg2ggAF2NYCAAAyiAIAJnq4AgAB4YgCAAIctgIAAZAUAgAChXAA= 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> <98CBD80474FA8B44BF855DF32C47DC35D8770D@smartserver.smartshare.dk> <20230208163521.GB5117@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Honnappa Nagarahalli" , "Tyler Retzlaff" Cc: , , , , , , , "nd" , , "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: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com] > Sent: Thursday, 9 February 2023 01.17 >=20 > >=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. > > > > > > > > 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. > > > > > > > > > > > > > > > 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. > > > > > > > > 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. > > > > > > > > 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. > > > > > > > > > > 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. > > > > i consider it a mandatory requirement. i don't see practically how = we > could > > withdraw existing use and even if we had clean way i don't see why = we > would > > want to. so this item is defintely settled if you were concerned. > I think I agree here. >=20 > > > > > > > > So either we upgrade the DPDK build requirements to support C11 > (including > > the optional stdatomics), or we provide our own DPDK atomics. > > > > i think the issue of requiring a toolchain conformant to a specific > standard is a > > separate matter because any adoption of C11 standard atomics is a > potential > > abi break from the current use of intrinsics. > I am not sure why you are calling it as ABI break. Referring to [1], I > just see wrappers around intrinsics (though [2] does not use the > intrinsics). >=20 > [1] https://github.com/gcc- > mirror/gcc/blob/master/gcc/ginclude/stdatomic.h > [2] https://github.com/llvm- > mirror/clang/blob/master/lib/Headers/stdatomic.h Good input, Honnappa. This means that the ABI break is purely academic, and there is no ABI = breakage in reality. Since the underlying implementation is the same, it is perfectly OK to = mix C11 and intrinsic atomics, even when the DPDK and the application = are built in different environments (with and without C11 atomics, or = vice versa). This eliminates my only remaining practical concern about this approach. >=20 > > > > the abstraction (whatever namespace it resides) allows the existing > > toolchain/platform combinations to maintain compatibility by > defaulting to > > current non-standard intrinsics. > How about using the intrinsics (__atomic_xxx) name space for > abstraction? This covers the GCC and Clang compilers. > If there is another platform that uses the same name space for > something else, I think DPDK should not be supporting that platform. > What problems do you see? >=20 > > > > once in place it provides an opportunity to introduce new > toolchain/platform > > combinations and enables an opt-in capability to use stdatomics on > existing > > toolchain/platform combinations subject to community discussion on > > how/if/when. > > > > it would be good to get more participants into the discussion so = i'll > cc techboard > > for some attention. i feel like the only area that isn't decided is > to do or not do > > this in rte_ namespace. > > > > i'm strongly in favor of rte_ namespace after discussion, mainly due > to to > > disadvantages of trying to overlap with the standard namespace while > not > > providing a compatible api/abi and because it provides clear > disambiguation of > > that difference in semantics and compatibility with the standard = api. > > > > so far i've noted the following > > > > * we will not provide the non-explicit apis. > +1 >=20 > > * we will make no attempt to support operate on struct/union atomics > > with our apis. > +1 >=20 > > * we will mirror the standard api potentially in the rte_ namespace > to > > - reference the standard api documentation. > > - assume compatible semantics (sans exceptions from first 2 > points). > > > > my vote is to remove 'potentially' from the last point above for > reasons > > previously discussed in postings to the mail thread. > > > > thanks all for the discussion, i'll send up a patch removing non- > explicit apis for > > viewing. > > > > ty