From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v4 1/1] eal: add 128-bit compare exchange (x86-64 only) Date: Wed, 03 Apr 2019 21:27:48 +0200 Message-ID: <1850366.0NedYqJ5gm@xps> References: <20190304205133.2248-1-gage.eads@intel.com> <2974113.GYloSpOT6o@xps> <9184057F7FC11744A2107296B6B8EB1E542106DC@FMSMSX108.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: "dev@dpdk.org" , "olivier.matz@6wind.com" , "arybchenko@solarflare.com" , "Richardson, Bruce" , "Ananyev, Konstantin" , "gavin.hu@arm.com" , "Honnappa.Nagarahalli@arm.com" , "nd@arm.com" , "chaozhu@linux.vnet.ibm.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" To: "Eads, Gage" Return-path: Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 2E2276C9B for ; Wed, 3 Apr 2019 21:27:53 +0200 (CEST) In-Reply-To: <9184057F7FC11744A2107296B6B8EB1E542106DC@FMSMSX108.amr.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 03/04/2019 21:21, Eads, Gage: > > 03/04/2019 19:34, Gage Eads: > > > This operation can be used for non-blocking algorithms, such as a > > > non-blocking stack or ring. > > > > > > Signed-off-by: Gage Eads > > > Reviewed-by: Honnappa Nagarahalli > > > --- > > > --- a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h > > > +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h > > > +/** > > > + * An atomic compare and set function used by the mutex functions. > > > + * (Atomically) Equivalent to: > > > + * if (*dst == *exp) > > > + * *dst = *src > > > + * else > > > + * *exp = *dst > > > + * > > > + * @note The success and failure arguments must be one of the > > > +__ATOMIC_* values > > > + * defined in the C++11 standard. For details on their behavior, > > > +refer to the > > > + * standard. > > > + * > > > + * @param dst > > > + * The destination into which the value will be written. > > > + * @param exp > > > + * Pointer to the expected value. If the operation fails, this memory is > > > + * updated with the actual value. > > > + * @param src > > > + * Pointer to the new value. > > > + * @param weak > > > + * A value of true allows the comparison to spuriously fail and allows the > > > + * 'exp' update to occur non-atomically (i.e. a torn read may occur). > > > + * Implementations may ignore this argument and only implement the > > strong > > > + * variant. > > > + * @param success > > > + * If successful, the operation's memory behavior conforms to this (or a > > > + * stronger) model. > > > + * @param failure > > > + * If unsuccessful, the operation's memory behavior conforms to this (or > > a > > > + * stronger) model. This argument cannot be __ATOMIC_RELEASE, > > > + * __ATOMIC_ACQ_REL, or a stronger model than success. > > > + * @return > > > + * Non-zero on success; 0 on failure. > > > + */ > > > +static inline int __rte_experimental > > > +rte_atomic128_cmp_exchange(rte_int128_t *dst, > > > + rte_int128_t *exp, > > > + const rte_int128_t *src, > > > + unsigned int weak, > > > + int success, > > > + int failure) > > > > I was thinking about keeping the doxygen in the generic file. > > Is it possible? > > > > We'd need to include the definition of rte_int128_t, so we'd also need either an ifdef on RTE_ARCH_64 or RTE_ARCH_X86_64 to protect 32-bit builds. That macro would prevent doxygen from parsing that section, unless we add a workaround like, for example: > > #if defined(RTE_ARCH_64) || defined(__DOXYGEN__) > > So the patch would look like the v3, with the declaration in generic/rte_atomic.h, but with that preprocessor change. If we change RTE_ARCH_X86_64 to RTE_ARCH_64, I'd add a note clarifying that it's only implemented for x86-64. What do you think? I would like to see the doc in the generic file and the implementation in the x86 file. I tried forward declaration of the typedef: struct rte_int128; typedef struct rte_int128 rte_int128_t; I don't why it does not work. So I'm trying to protect the declaration with #ifdef __DOXYGEN__