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 2057B41C54; Thu, 9 Feb 2023 18:30:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1019B4113D; Thu, 9 Feb 2023 18:30:21 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id D5519410F9; Thu, 9 Feb 2023 18:30:18 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 0ACDF20C8AEC; Thu, 9 Feb 2023 09:30:18 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 0ACDF20C8AEC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1675963818; bh=dneFkze5ojeDFWnc6pBhHCjtVmZWKh8U/LOdi7cgWVg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=P+PpxtewmiBbompWYavrVjhETlK8+PidYFZhvTyrXCqDFQQGAHPbkrJ7l+YjK5XMp a7UtZn0lSb4tIURSHwolBxe8MuZt0d24wXp4f78jOm6HWm1WFWwD4l6eJV/p6Yz2kG pP2CKp2tVnJNmtxECHNBdAmEsEklF6dtbJPHoVeQ= Date: Thu, 9 Feb 2023 09:30:17 -0800 From: Tyler Retzlaff To: Honnappa Nagarahalli Cc: Morten =?iso-8859-1?Q?Br=F8rup?= , "thomas@monjalon.net" , "dev@dpdk.org" , "bruce.richardson@intel.com" , "david.marchand@redhat.com" , "jerinj@marvell.com" , "konstantin.ananyev@huawei.com" , "ferruh.yigit@amd.com" , nd , "techboard@dpdk.org" Subject: Re: [PATCH] eal: introduce atomics abstraction Message-ID: <20230209173017.GA21854@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote: > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > 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). > > [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 it's a potential abi break because atomic types are not the same types as their corresponding integer types etc.. (or at least are not guaranteed to be by all implementations of c as an abstract language). ISO/IEC 9899:2011 6.2.5 (27) Further, there is the _Atomic qualifier. The presence of the _Atomic qualifier designates an atomic type. The size, representation, and alignment of an atomic type need not be the same as those of the corresponding unqualified type. 7.17.6 (3) NOTE The representation of atomic integer types need not have the same size as their corresponding regular types. They should have the same size whenever possible, as it eases effort required to port existing code. i use the term `potential abi break' with intent because for me to assert in absolute terms i would have to evaluate the implementation of every current and potential future compilers atomic vs non-atomic types. this as i'm sure you understand is not practical, it would also defeat the purpose of moving to a standard. therefore i rely on the specification prescribed by the standard not the detail of a specific implementation. > > 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. the namespace starting with `__` is also reserved for the implementation. this is why compilers gcc/clang/msvc place name their intrinsic and builtin functions starting with __ to explicitly avoid collision with the application namespace. ISO/IEC 9899:2011 7.1.3 (1) All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use. ... > If there is another platform that uses the same name space for something else, I think DPDK should not be supporting that platform. that's effectively a statement excluding windows platform and all non-gcc compilers from ever supporting dpdk. > What problems do you see? i'm fairly certain at least one other compiler uses the __atomic namespace but it would take me time to check, the most notable potential issue that comes to mind is if such an intrinsic with the same name is provided in a different implementation and has either regressive code generation or different semantics it would be bad because it is intrinsic you can't just hack around it with #undef __atomic to shim in a semantically correct version. how about this, is there another possible namespace you might suggest that conforms or doesn't conflict with the the rules defined in ISO/IEC 9899:2011 7.1.3 i think if there were that would satisfy all of my concerns related to namespaces. keep in mind the point of moving to a standard is to achieve portability so if we do things that will regress us back to being dependent on an implementation we haven't succeeded. that's all i'm trying to guarantee here. i feel like we are really close on this discussion, if we can just iron this issue out we can probably get going on the actual changes. thanks for the consideration. > > > > > 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 > > > * we will make no attempt to support operate on struct/union atomics > > with our apis. > +1 > > > * 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