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 41E2742395; Tue, 10 Jan 2023 21:31:27 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DC50040E25; Tue, 10 Jan 2023 21:31:26 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id D3CEB4021E for ; Tue, 10 Jan 2023 21:31:24 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 1C92D20B92AE; Tue, 10 Jan 2023 12:31:24 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1C92D20B92AE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1673382684; bh=ad8L5q7DQan1z0+EybCDxoVtpjC0rbUURu9StpLYops=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=G6lKF/+NvlEfTxHwtY8RL92m5N9KUrz68ZO1+J0Vm42+C/KrY7pJKvItARL+7ALwc MaO+Ro+zcNMhcRxqCFqx3KRFIpcRcWp0oSsujM4GmF20gJtObU8IXLkA5fKCyh1YpL us2V+XQvwv+XQXtXV2G/iQjc1YE6Lmg11LrBMboY= Date: Tue, 10 Jan 2023 12:31:24 -0800 From: Tyler Retzlaff To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Bruce Richardson , dev@dpdk.org Subject: Re: RFC abstracting atomics Message-ID: <20230110203124.GD21476@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20230109225604.GA25566@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <98CBD80474FA8B44BF855DF32C47DC35D87648@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87648@smartserver.smartshare.dk> 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 Tue, Jan 10, 2023 at 12:45:05PM +0100, Morten Brørup wrote: > > 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. the patch series i have prototype exactly mirrors the stdandard atomics from C11. it requires a few fixups to fill convenience builtins that the standard doesn't provide e.g. `__atomic_add_fetch(&o, v, order)` becomes `__atomic_fetch_add(&o, v, order) + v;` sort of stuff, mostly minor. > > > > > > > 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). the few __sync left in use are easily converted. there might be some argument against converting __sync -> __atomic because of sub-optimal codegen on older gcc for the semantically equivalent replacements. the only comment i have on current/existing rte_atomic is they're annoyingly stealing a little bit of the rte_ namespace that would allow convenient alignment with the standard names. also some of them provide only signed or unsigned variations which forces kind of non-standardy things to be done in the code but are perfectly acceptable when using the gcc implementation. with the set of macros i'm introducing migrating to direct use of C11 atomics would likely be just a big search and replace (or that is my goal). > 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? i kept hoping we would just baseline on C11 but after prototyping the abstraction i realize there may be some valid arguments for not moving to stdatomics now or possibly ever for some platform/toolchain combinations (so long as they are declared supported). given the drawn out discussion about RHEL-7 has shown that the migration path may be long term. more possible headwinds were listed in my reply to Bruce. > > > > > 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. i gave some detail about this to my reply with Bruce. i think when i post the series we can discuss it more. but for atomics i expect it isn't worth separating out.