From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Tyler Retzlaff" <roretzla@linux.microsoft.com>
Cc: "Honnappa Nagarahalli" <Honnappa.Nagarahalli@arm.com>,
<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" <nd@arm.com>
Subject: RE: [PATCH] eal: introduce atomics abstraction
Date: Thu, 2 Feb 2023 21:44:52 +0100 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D876F7@smartserver.smartshare.dk> (raw)
In-Reply-To: <20230202190023.GA32597@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 2 February 2023 20.00
>
> On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Wednesday, 1 February 2023 22.41
> > >
> > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli
> wrote:
> > > >
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > >
> > > > > 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.
> > >
> > > >
> > > > 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.
> >
> > Perhaps we can use something already existing, such as this:
> > https://android.googlesource.com/platform/bionic/+/lollipop-
> release/libc/include/stdatomic.h
> >
> > >
> > > 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
> > > manual that explain the justification for these reservations and if
> > > if we think about ODR and ABI compatibility we can conceive of
> others.
> >
> > I we are going to move to C11 soon, I consider the shim interim, and
> am inclined to ignore these warning factors.
> >
> > If we are not moving to C11 soon, I would consider these
> disadvantages more seriously.
>
> I think it's reasonable to assume that we are talking years here.
>
> We've had a few discussions about minimum C standard. I think my first
> mailing list exchanges about C99 was almost 2 years ago. Given that we
> still aren't on C99 now (though i know Bruce has a series up) indicates
> that progression to C11 isn't going to happen any time soon and even if
> it was the baseline we still can't just use it (reasons described
> later).
>
> Also, i'll point out that we seem to have accepted moving to C99 with
> one of the holdback compilers technically being non-conformant but it
> isn't blocking us because it provides the subset of C99 features
> without
> being conforming that we happen to be using.
>
> >
> > >
> > > 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.
> >
> > This is a very important remark, which should be considered
> carefully! Tyler has firsthand experience with DPDK portability. If he
> thinks porting to Windows is going to be a headache if we expose the
> stdatomic.h API, we must listen! So, what is your gut feeling here,
> Tyler?
>
> I think this is even more of a concern with language standard than it
> is
> with a platform standard. Because the language standard is used across
> platforms.
>
> On the surface it looks appealing to just go through all the dpdk code
> one last time and #include <stdatomic.h> and directly depend on names
> that "look" standard. In practice though we aren't depending on the
> toolchain / libc surface we are binding ourselves to the shim and the
> implementation it provides.
>
> This is aside from the mechanics of making it work in the different
> contexts we now have to care about. Here is a story of how things
> become tricky.
>
> When i #include <stdatomic.h> which one gets used if the implementation
> provides one? Let's force our stdatomic.h
>
> Now i need to force the build system to prefer my shim header? Keeping
> in mind that the presence of a libc stdatomic.h does not mean that the
> toolchain in fact supports standard atomics. Okay, that's under our
> control by editing some meson.build files maybe it isn't so bad but...
>
> It seems my application also has to do the same in their build system
> now because...
>
> The type definitions (size, alignment) and code generated from the
> body of inline functions as seen by the application built translation
> units may differ from those in the dpdk translation units if they don't
> use our header. The potential for ABI compat problems is increasing but
> maybe it is managable? it can be worse...
>
> We can't limit our scope to thinking that there is just an
> application (a single binary) and dpdk. Complex applications will
> invariably depend on other libraries and if the application needs to
> interface with those compatibily at the ABI level using standard
> atomics
> then we've made it very difficult since the application has to choose
> to
> use our conflicting named atomic types which may not be compatible or
> the real standard atomics. They can of course produce horrible shims
> of their own to interoperate.
>
> We need consistency across the entire binary at runtime and i don't
> think it's practical to say that anyone who uses dpdk has to compile
> their whole world with our shim. So dealing with all this complexity
> for the sake of asthetics "looking" like the standard api seems kind
> of not worth it. Sure it saves having to deprecate later and one last
> session of shotgun surgery but that's kind of all we get.
>
> Don't think i'm being biased in favor of windows/msvc here. From the
> perspective of the windows/msvc combination i intend to use only the
> standard C ABI provided by the implementation. I have no intention of
> trying to introduce support for the current ABI that doesn't use the
> standard atomic types. my discouraging of this approach is about
> avoiding
> subtle to detect but very painful problems on
> {linux,unix}/compiler<version>
> combinations that already have a shipped/stable ABI.
>
> > >
> > > let's discuss this from here. if there's still overwhelming desire
> to
> > > go
> > > this route then we'll just do our best.
> > >
> > > ty
> >
> > I have a preference for exposing the stdatomic.h API. Tyler listed
> the disadvantages above. (I also have a preference for moving to C11
> soon.)
>
> I am eager to see this happen, but as explained in my original proposal
> it doesn't eliminate the need for an abstraction. Unless we are willing
> to break our compatibility promises and potentially take a performance
> hit on some platform/compiler combinations which as i understand is not
> acceptable.
>
> >
> > Exposing a 1:1 similar API with RTE prefixes would also be acceptable
> for me. The disadvantage is that the names are different than the C11
> names, which might lead to some confusion. And from an ABI stability
> perspective, such an important API should not be marked experimental.
> This means that years will pass before we can get rid of it again, due
> to ABI stability policies.
>
> I think the key to success with rte_ prefixed names is making
> absolutely
> sure we mirror the semantics and types in the standard.
>
> I will point out one bit of fine print here is that we will not support
> atomic operations on struct/union types (something the standard
> supports).
> With the rte_ namespace i think this becomes less ambiguous, if we
> present
> standard C names though what's to avoid the confusion? Aside from it
> fails
> to compile with one compiler vs another.
>
> I agree that this may be around for years. But how many years depends a
> lot on how long we have to maintain compatibility for the existing
> platform/compiler combinations that can't (and aren't enabled) to use
> the standard.
>
> Even if we introduced standard names we still have to undergo some kind
> of mutant deprecation process to get the world to recompile everything
> against the actual standard, so it doesn't give us forward
> compatibility.
>
> Let me know what folks would like to do, i guess i'm firmly leaned
> toward no-shim and just rte_ explicit. But as a community i'll pursue
> whatever you decide.
>
> Thanks!
Tyler is making a very strong case here.
I have changed my mind, and now support Tyler's approach.
-Morten
next prev parent reply other threads:[~2023-02-02 20:44 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-12 21:26 [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-01-12 21:26 ` [PATCH] eal: introduce atomics abstraction Tyler Retzlaff
2023-01-31 22:42 ` Thomas Monjalon
2023-02-01 1:07 ` Honnappa Nagarahalli
2023-02-01 8:09 ` Morten Brørup
2023-02-01 21:41 ` Tyler Retzlaff
2023-02-02 8:43 ` Morten Brørup
2023-02-02 19:00 ` Tyler Retzlaff
2023-02-02 20:44 ` Morten Brørup [this message]
2023-02-03 13:56 ` Bruce Richardson
2023-02-03 14:25 ` Morten Brørup
2023-02-03 12:19 ` Bruce Richardson
2023-02-03 20:49 ` Tyler Retzlaff
2023-02-07 15:16 ` Morten Brørup
2023-02-07 21:58 ` Tyler Retzlaff
2023-02-07 23:34 ` Honnappa Nagarahalli
2023-02-08 1:20 ` Tyler Retzlaff
2023-02-08 8:31 ` Morten Brørup
2023-02-08 16:35 ` Tyler Retzlaff
2023-02-09 0:16 ` Honnappa Nagarahalli
2023-02-09 8:34 ` Morten Brørup
2023-02-09 17:30 ` Tyler Retzlaff
2023-02-10 5:30 ` Honnappa Nagarahalli
2023-02-10 20:30 ` Tyler Retzlaff
2023-02-13 5:04 ` Honnappa Nagarahalli
2023-02-13 15:28 ` Ben Magistro
2023-02-13 15:55 ` Bruce Richardson
2023-02-13 16:46 ` Ben Magistro
2023-02-13 17:49 ` Morten Brørup
2023-02-13 23:18 ` Tyler Retzlaff
2023-01-31 21:33 ` [PATCH] eal: abstract compiler atomics Tyler Retzlaff
2023-02-08 21:43 ` [PATCH v2] " Tyler Retzlaff
2023-02-08 21:43 ` [PATCH v2] eal: introduce atomics abstraction Tyler Retzlaff
2023-02-09 8:05 ` Morten Brørup
2023-02-09 18:15 ` Tyler Retzlaff
2023-02-09 19:19 ` Morten Brørup
2023-02-09 22:04 ` Tyler Retzlaff
2023-04-03 21:17 ` Mattias Rönnblom
2023-02-09 9:04 ` Bruce Richardson
2023-02-09 12:53 ` Ferruh Yigit
2023-02-09 17:40 ` Tyler Retzlaff
2023-02-09 22:13 ` Ferruh Yigit
2023-02-10 0:36 ` Tyler Retzlaff
2023-02-09 17:38 ` Tyler Retzlaff
2023-04-03 21:32 ` Mattias Rönnblom
2023-04-03 21:11 ` Mattias Rönnblom
2023-04-03 21:25 ` Honnappa Nagarahalli
2023-04-04 2:24 ` Tyler Retzlaff
2023-02-22 18:09 ` [PATCH v2] eal: abstract compiler atomics Tyler Retzlaff
2023-02-22 20:07 ` Honnappa Nagarahalli
2023-02-23 19:11 ` Tyler Retzlaff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=98CBD80474FA8B44BF855DF32C47DC35D876F7@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=jerinj@marvell.com \
--cc=konstantin.ananyev@huawei.com \
--cc=nd@arm.com \
--cc=roretzla@linux.microsoft.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).