[1/3] eal: add 128-bit cmpset (x86-64 only)
Checks
Commit Message
This operation can be used for non-blocking algorithms, such as a
non-blocking stack or ring.
Signed-off-by: Gage Eads <gage.eads@intel.com>
---
.../common/include/arch/x86/rte_atomic_64.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
Comments
On 1/10/19 11:55 PM, Gage Eads wrote:
> This operation can be used for non-blocking algorithms, such as a
> non-blocking stack or ring.
>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> ---
> .../common/include/arch/x86/rte_atomic_64.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
> index fd2ec9c53..34c2addf8 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
> @@ -34,6 +34,7 @@
> /*
> * Inspired from FreeBSD src/sys/amd64/include/atomic.h
> * Copyright (c) 1998 Doug Rabson
> + * Copyright (c) 2019 Intel Corporation
> * All rights reserved.
> */
>
> @@ -208,4 +209,25 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
> }
> #endif
>
> +static inline int
> +rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t *src)
> +{
> + uint8_t res;
> +
> + asm volatile (
> + MPLOCKED
> + "cmpxchg16b %[dst];"
> + " sete %[res]"
> + : [dst] "=m" (*dst),
> + [res] "=r" (res)
> + : "c" (src[1]),
> + "b" (src[0]),
> + "m" (*dst),
> + "d" (exp[1]),
> + "a" (exp[0])
> + : "memory");
> +
> + return res;
> +}
> +
> #endif /* _RTE_ATOMIC_X86_64_
Is it OK to add it to rte_atomic_64.h header which is for 64-bit integer
ops?
Andrew.
> H_ */
Hi Gage,
snipped
> > @@ -208,4 +209,25 @@ static inline void
> rte_atomic64_clear(rte_atomic64_t *v)
> > }
> > #endif
> >
> > +static inline int
> > +rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t
> > +*src) {
> > + uint8_t res;
> > +
> > + asm volatile (
> > + MPLOCKED
> > + "cmpxchg16b %[dst];"
> > + " sete %[res]"
> > + : [dst] "=m" (*dst),
> > + [res] "=r" (res)
> > + : "c" (src[1]),
> > + "b" (src[0]),
> > + "m" (*dst),
> > + "d" (exp[1]),
> > + "a" (exp[0])
> > + : "memory");
Since update depends upon on the 'set|unset' value of ZF, should we first set ZF to 0?
Apologies in advance if it is internally taken care by 'sete'.
> > +
> > + return res;
> > +}
> > +
> > #endif /* _RTE_ATOMIC_X86_64_
>
> Is it OK to add it to rte_atomic_64.h header which is for 64-bit integer ops?
>
> Andrew.
> > H_ */
From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Sunday, January 13, 2019 6:19 AM
To: Eads, Gage <gage.eads@intel.com>; dev@dpdk.org
Cc: olivier.matz@6wind.com; Richardson, Bruce <bruce.richardson@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
Subject: Re: [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)
On 1/10/19 11:55 PM, Gage Eads wrote:
This operation can be used for non-blocking algorithms, such as a
non-blocking stack or ring.
Signed-off-by: Gage Eads <gage.eads@intel.com><mailto:gage.eads@intel.com>
---
.../common/include/arch/x86/rte_atomic_64.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
index fd2ec9c53..34c2addf8 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_64.h
@@ -34,6 +34,7 @@
/*
* Inspired from FreeBSD src/sys/amd64/include/atomic.h
* Copyright (c) 1998 Doug Rabson
+ * Copyright (c) 2019 Intel Corporation
* All rights reserved.
*/
@@ -208,4 +209,25 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
}
#endif
+static inline int
+rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t *src)
+{
+ uint8_t res;
+
+ asm volatile (
+ MPLOCKED
+ "cmpxchg16b %[dst];"
+ " sete %[res]"
+ : [dst] "=m" (*dst),
+ [res] "=r" (res)
+ : "c" (src[1]),
+ "b" (src[0]),
+ "m" (*dst),
+ "d" (exp[1]),
+ "a" (exp[0])
+ : "memory");
+
+ return res;
+}
+
#endif /* _RTE_ATOMIC_X86_64_
Is it OK to add it to rte_atomic_64.h header which is for 64-bit integer ops?
Andrew.
I believe this file is for atomic operations specific to x86_64 builds, but not necessarily limited to 64-bit operations (note that rte_atomic_32.h contains 64-bit operations specific to 32-bit builds). At least, that’s how I interpreted it.
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Sunday, January 13, 2019 10:29 PM
> To: Andrew Rybchenko <arybchenko@solarflare.com>; Eads, Gage
> <gage.eads@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)
>
> Hi Gage,
>
> snipped
> > > @@ -208,4 +209,25 @@ static inline void
> > rte_atomic64_clear(rte_atomic64_t *v)
> > > }
> > > #endif
> > >
> > > +static inline int
> > > +rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t
> > > +*src) {
> > > + uint8_t res;
> > > +
> > > + asm volatile (
> > > + MPLOCKED
> > > + "cmpxchg16b %[dst];"
> > > + " sete %[res]"
> > > + : [dst] "=m" (*dst),
> > > + [res] "=r" (res)
> > > + : "c" (src[1]),
> > > + "b" (src[0]),
> > > + "m" (*dst),
> > > + "d" (exp[1]),
> > > + "a" (exp[0])
> > > + : "memory");
> Since update depends upon on the 'set|unset' value of ZF, should we first set ZF
> to 0?
>
> Apologies in advance if it is internally taken care by 'sete'.
cmpxchg16b will set the ZF if the compared values are equal, else it will clear the ZF, so there's no need to initialize the ZF.
Source: https://www.felixcloutier.com/x86/cmpxchg8b:cmpxchg16b
Thanks,
Gage
Thanks Gage for clarifying and correcting. Appreciate the same.
> -----Original Message-----
> From: Eads, Gage
> Sent: Monday, January 14, 2019 9:17 PM
> To: Varghese, Vipin <vipin.varghese@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com; Richardson, Bruce <bruce.richardson@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64 only)
>
>
>
> > -----Original Message-----
> > From: Varghese, Vipin
> > Sent: Sunday, January 13, 2019 10:29 PM
> > To: Andrew Rybchenko <arybchenko@solarflare.com>; Eads, Gage
> > <gage.eads@intel.com>; dev@dpdk.org
> > Cc: olivier.matz@6wind.com; Richardson, Bruce
> > <bruce.richardson@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH 1/3] eal: add 128-bit cmpset (x86-64
> > only)
> >
> > Hi Gage,
> >
> > snipped
> > > > @@ -208,4 +209,25 @@ static inline void
> > > rte_atomic64_clear(rte_atomic64_t *v)
> > > > }
> > > > #endif
> > > >
> > > > +static inline int
> > > > +rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp,
> > > > +uint64_t
> > > > +*src) {
> > > > + uint8_t res;
> > > > +
> > > > + asm volatile (
> > > > + MPLOCKED
> > > > + "cmpxchg16b %[dst];"
> > > > + " sete %[res]"
> > > > + : [dst] "=m" (*dst),
> > > > + [res] "=r" (res)
> > > > + : "c" (src[1]),
> > > > + "b" (src[0]),
> > > > + "m" (*dst),
> > > > + "d" (exp[1]),
> > > > + "a" (exp[0])
> > > > + : "memory");
> > Since update depends upon on the 'set|unset' value of ZF, should we
> > first set ZF to 0?
> >
> > Apologies in advance if it is internally taken care by 'sete'.
>
> cmpxchg16b will set the ZF if the compared values are equal, else it will clear the
> ZF, so there's no need to initialize the ZF.
>
> Source: https://www.felixcloutier.com/x86/cmpxchg8b:cmpxchg16b
>
> Thanks,
> Gage
@@ -34,6 +34,7 @@
/*
* Inspired from FreeBSD src/sys/amd64/include/atomic.h
* Copyright (c) 1998 Doug Rabson
+ * Copyright (c) 2019 Intel Corporation
* All rights reserved.
*/
@@ -208,4 +209,25 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
}
#endif
+static inline int
+rte_atomic128_cmpset(volatile uint64_t *dst, uint64_t *exp, uint64_t *src)
+{
+ uint8_t res;
+
+ asm volatile (
+ MPLOCKED
+ "cmpxchg16b %[dst];"
+ " sete %[res]"
+ : [dst] "=m" (*dst),
+ [res] "=r" (res)
+ : "c" (src[1]),
+ "b" (src[0]),
+ "m" (*dst),
+ "d" (exp[1]),
+ "a" (exp[0])
+ : "memory");
+
+ return res;
+}
+
#endif /* _RTE_ATOMIC_X86_64_H_ */