DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Bruce Richardson" <bruce.richardson@intel.com>,
	"Tyler Retzlaff" <roretzla@linux.microsoft.com>
Cc: <dev@dpdk.org>
Subject: RE: RFC abstracting atomics
Date: Tue, 10 Jan 2023 12:45:05 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87648@smartserver.smartshare.dk> (raw)
In-Reply-To: <Y70tALGqxUHqYBup@bricha3-MOBL.ger.corp.intel.com>

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Tuesday, 10 January 2023 10.17
> 
> On Mon, Jan 09, 2023 at 02:56:04PM -0800, Tyler Retzlaff wrote:
> > hi folks,
> >
> > i would like to introduce a layer of abstraction that would allow
> > optional use of standard C11 atomics when the platform / toolchain
> > combination has them available.

It would be great if that abstraction layer could expose the exact same (or very similar) interface as standard C11 atomics, so the API is given in advance, and developers don't have to learn the API of yet another shim. It would also make it easier getting rid of the abstraction layer at a later time.

> >
> > making the option usable would be a phased approach intended to focus
> > review and minimize dealing with churn on such a broad change.
> >
> > 1. provide an initial series to add the abstraction and the ability
> >    control enablement with a meson option enable_stdatomics=false
> will
> >    be the default.
> >
> >    for all existing platform / toolchain combinations the default
> would
> >    remain false. i.e. i have no plans to enable it for existing
> platforms
> >    toolchain combinations but leaves a change of default open to the
> >    community as a future discussion if it is desired.
> >
> > 2. once the initial abstraction is integrated a series will be
> introduced to
> >    port the tree to the abstraction with enable_stdatomics=false. the
> goal
> >    being low or no change to the current use of gcc builtin C++11
> memory
> >    model atomics.
> >
> > 3. once the tree is ported a final series will be introduced to
> introduce
> >    the remaining change to allow the use of enable_stdatomics=true.
> >
> > would appreciate any assistance / suggestions you can provide to
> > introduce the abstraction smoothly.
> >
> 
> Plan generally sounds ok. However, beyond point #3, would there then be
> plans to remove the option and always use stdatomics in future?

We are already trying to migrate atomics in DPDK from using __sync built-ins (used in rte_atomic) to using __atomic built-ins; e.g. new contributions are recommended to use the latter (and not rte_atomic). When DPDK migrates to C11, I assume stdatomics will become the new standard for atomics in DPDK, and the process for migrating to stdatomics will resemble the currently ongoing process for migrating from rte_atomic to __atomic built-ins, which seems to be relatively uncomplicated (but slow).

Another thing: The proposed layer of abstraction should only live until DPDK supports C11, which is hopefully relatively soon. So is such a (hopefully) short-lived abstraction layer worth the effort?

> 
> To slightly expand the scope of the discussion - would it be worthwhile
> putting these abstractions in a new library in DPDK other than EAL, to
> start the process of splitting out some of the lower-level material
> from
> that library?

While I applaud this idea, I think we first need to discuss in detail how to split up EAL, and especially what new structure we want to end up with instead. E.g. we already have architecture specific code in various libraries and drivers, which proves that architecture specific stuff does not necessarily need to go into the EAL. We also need to discuss challenges/barriers to separating stuff from the EAL, and how we can solve those challenges; e.g. some parts are inside the EAL only due to startup sequence requirements, which might be solvable by offering better startup sequencing support for libraries in the EAL. Perhaps the EAL has even outlived itself, and could be replaced by set of DPDK Core libraries instead - some could be optional (at build time). But let's make this a separate discussion.


  reply	other threads:[~2023-01-10 11:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09 22:56 Tyler Retzlaff
2023-01-10  9:16 ` Bruce Richardson
2023-01-10 11:45   ` Morten Brørup [this message]
2023-01-10 20:31     ` Tyler Retzlaff
2023-01-11  7:45       ` Morten Brørup
2023-01-10 20:10   ` Tyler Retzlaff
2023-01-11 10:10     ` Bruce Richardson
2023-01-11 10:23       ` Morten Brørup
2023-01-11 11:56         ` Bruce Richardson
2023-01-11 12:46           ` Morten Brørup
2023-01-11 14:18             ` Bruce Richardson
2023-01-11 15:07               ` Morten Brørup
2023-01-13 14:18                 ` Ben Magistro
2023-01-13 16:10                   ` Jerin Jacob
2023-01-13 17:17                     ` 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=98CBD80474FA8B44BF855DF32C47DC35D87648@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=roretzla@linux.microsoft.com \
    /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).