[dpdk-dev,v8,01/15] eal: introduce atomic exchange operation
Checks
Commit Message
To handle atomic update of link status (64 bit), every driver
was doing its own version using cmpset.
Atomic exchange is a useful primitive in its own right;
therefore make it a EAL routine.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
.../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
.../common/include/arch/x86/rte_atomic.h | 24 +++++++
.../common/include/arch/x86/rte_atomic_32.h | 12 ++++
.../common/include/arch/x86/rte_atomic_64.h | 12 ++++
lib/librte_eal/common/include/generic/rte_atomic.h | 78 ++++++++++++++++++++++
5 files changed, 146 insertions(+), 1 deletion(-)
Comments
26/01/2018 03:01, Stephen Hemminger:
> To handle atomic update of link status (64 bit), every driver
> was doing its own version using cmpset.
> Atomic exchange is a useful primitive in its own right;
> therefore make it a EAL routine.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> .../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
> .../common/include/arch/x86/rte_atomic.h | 24 +++++++
> .../common/include/arch/x86/rte_atomic_32.h | 12 ++++
> .../common/include/arch/x86/rte_atomic_64.h | 12 ++++
> lib/librte_eal/common/include/generic/rte_atomic.h | 78 ++++++++++++++++++++++
> 5 files changed, 146 insertions(+), 1 deletion(-)
Looks good, thanks.
It probably deserves a review by PPC experts.
Adding Chao, maintainer of this part.
+ Bruce and Konstantin for x86 part.
On Fri, Jan 26, 2018 at 06:14:01PM +0100, Thomas Monjalon wrote:
> 26/01/2018 03:01, Stephen Hemminger:
> > To handle atomic update of link status (64 bit), every driver was
> > doing its own version using cmpset. Atomic exchange is a useful
> > primitive in its own right; therefore make it a EAL routine.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> ---
> > .../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
> > .../common/include/arch/x86/rte_atomic.h | 24 +++++++
> > .../common/include/arch/x86/rte_atomic_32.h | 12 ++++
> > .../common/include/arch/x86/rte_atomic_64.h | 12 ++++
> > lib/librte_eal/common/include/generic/rte_atomic.h | 78
> > ++++++++++++++++++++++ 5 files changed, 146 insertions(+), 1
> > deletion(-)
>
> Looks good, thanks.
>
> It probably deserves a review by PPC experts. Adding Chao, maintainer
> of this part. + Bruce and Konstantin for x86 part.
>
Would it not be simpler to use __sync_bool_compare_and_swap compiler
built-in on all supported platforms? Do we really need the per-platform
optimization of this?
/Bruce
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Friday, January 26, 2018 2:02 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH v8 01/15] eal: introduce atomic exchange operation
>
> To handle atomic update of link status (64 bit), every driver
> was doing its own version using cmpset.
> Atomic exchange is a useful primitive in its own right;
> therefore make it a EAL routine.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> .../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
> .../common/include/arch/x86/rte_atomic.h | 24 +++++++
> .../common/include/arch/x86/rte_atomic_32.h | 12 ++++
> .../common/include/arch/x86/rte_atomic_64.h | 12 ++++
> lib/librte_eal/common/include/generic/rte_atomic.h | 78 ++++++++++++++++++++++
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
On Fri, 26 Jan 2018 17:24:40 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:
> On Fri, Jan 26, 2018 at 06:14:01PM +0100, Thomas Monjalon wrote:
> > 26/01/2018 03:01, Stephen Hemminger:
> > > To handle atomic update of link status (64 bit), every driver was
> > > doing its own version using cmpset. Atomic exchange is a useful
> > > primitive in its own right; therefore make it a EAL routine.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> ---
> > > .../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
> > > .../common/include/arch/x86/rte_atomic.h | 24 +++++++
> > > .../common/include/arch/x86/rte_atomic_32.h | 12 ++++
> > > .../common/include/arch/x86/rte_atomic_64.h | 12 ++++
> > > lib/librte_eal/common/include/generic/rte_atomic.h | 78
> > > ++++++++++++++++++++++ 5 files changed, 146 insertions(+), 1
> > > deletion(-)
> >
> > Looks good, thanks.
> >
> > It probably deserves a review by PPC experts. Adding Chao, maintainer
> > of this part. + Bruce and Konstantin for x86 part.
> >
> Would it not be simpler to use __sync_bool_compare_and_swap compiler
> built-in on all supported platforms? Do we really need the per-platform
> optimization of this?
>
> /Bruce
Exchange is different than compare and swap. The is nice atomic intrinsic
in GCC. The x86 part is in Linux and BSD already.
On Fri, Jan 26, 2018 at 01:54:23PM -0800, Stephen Hemminger wrote:
> On Fri, 26 Jan 2018 17:24:40 +0000
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > On Fri, Jan 26, 2018 at 06:14:01PM +0100, Thomas Monjalon wrote:
> > > 26/01/2018 03:01, Stephen Hemminger:
> > > > To handle atomic update of link status (64 bit), every driver was
> > > > doing its own version using cmpset. Atomic exchange is a useful
> > > > primitive in its own right; therefore make it a EAL routine.
> > > >
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> ---
> > > > .../common/include/arch/ppc_64/rte_atomic.h | 21 +++++-
> > > > .../common/include/arch/x86/rte_atomic.h | 24 +++++++
> > > > .../common/include/arch/x86/rte_atomic_32.h | 12 ++++
> > > > .../common/include/arch/x86/rte_atomic_64.h | 12 ++++
> > > > lib/librte_eal/common/include/generic/rte_atomic.h | 78
> > > > ++++++++++++++++++++++ 5 files changed, 146 insertions(+), 1
> > > > deletion(-)
> > >
> > > Looks good, thanks.
> > >
> > > It probably deserves a review by PPC experts. Adding Chao, maintainer
> > > of this part. + Bruce and Konstantin for x86 part.
> > >
> > Would it not be simpler to use __sync_bool_compare_and_swap compiler
> > built-in on all supported platforms? Do we really need the per-platform
> > optimization of this?
> >
> > /Bruce
>
> Exchange is different than compare and swap. The is nice atomic intrinsic
> in GCC. The x86 part is in Linux and BSD already.
Yes, sorry, I picked the wrong builtin. I should have referenced
instead "__sync_lock_test_and_set", which is an xchg op according to the
docs [https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html].
@@ -132,6 +132,12 @@ static inline int rte_atomic16_dec_and_test(rte_atomic16_t *v)
return __atomic_sub_fetch(&v->cnt, 1, __ATOMIC_ACQUIRE) == 0;
}
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+ return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
+}
+
/*------------------------- 32 bit atomic operations -------------------------*/
static inline int
@@ -233,6 +239,13 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
return ret == 0;
}
+
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+ return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+
/*------------------------- 64 bit atomic operations -------------------------*/
static inline int
@@ -427,7 +440,6 @@ static inline int rte_atomic64_test_and_set(rte_atomic64_t *v)
{
return rte_atomic64_cmpset((volatile uint64_t *)&v->cnt, 0, 1);
}
-
/**
* Atomically set a 64-bit counter to 0.
*
@@ -438,6 +450,13 @@ static inline void rte_atomic64_clear(rte_atomic64_t *v)
{
v->cnt = 0;
}
+
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+ return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+
#endif
#ifdef __cplusplus
@@ -60,6 +60,18 @@ rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
return res;
}
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+ asm volatile(
+ MPLOCKED
+ "xchgw %0, %1;"
+ : "=r" (val), "=m" (*dst)
+ : "0" (val), "m" (*dst)
+ : "memory"); /* no-clobber list */
+ return val;
+}
+
static inline int rte_atomic16_test_and_set(rte_atomic16_t *v)
{
return rte_atomic16_cmpset((volatile uint16_t *)&v->cnt, 0, 1);
@@ -134,6 +146,18 @@ rte_atomic32_cmpset(volatile uint32_t *dst, uint32_t exp, uint32_t src)
return res;
}
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+ asm volatile(
+ MPLOCKED
+ "xchgl %0, %1;"
+ : "=r" (val), "=m" (*dst)
+ : "0" (val), "m" (*dst)
+ : "memory"); /* no-clobber list */
+ return val;
+}
+
static inline int rte_atomic32_test_and_set(rte_atomic32_t *v)
{
return rte_atomic32_cmpset((volatile uint32_t *)&v->cnt, 0, 1);
@@ -98,6 +98,18 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
return res;
}
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dest, uint64_t val)
+{
+ uint64_t old;
+
+ do {
+ old = *dest;
+ } while (rte_atomic64_cmpset(dest, old, val));
+
+ return old;
+}
+
static inline void
rte_atomic64_init(rte_atomic64_t *v)
{
@@ -71,6 +71,18 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
return res;
}
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+ asm volatile(
+ MPLOCKED
+ "xchgq %0, %1;"
+ : "=r" (val), "=m" (*dst)
+ : "0" (val), "m" (*dst)
+ : "memory"); /* no-clobber list */
+ return val;
+}
+
static inline void
rte_atomic64_init(rte_atomic64_t *v)
{
@@ -139,6 +139,32 @@ rte_atomic16_cmpset(volatile uint16_t *dst, uint16_t exp, uint16_t src)
}
#endif
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ * ret = *dst
+ * *dst = val;
+ * return ret;
+ *
+ * @param dst
+ * The destination location into which the value will be written.
+ * @param val
+ * The new value.
+ * @return
+ * The original value at that location
+ */
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint16_t
+rte_atomic16_exchange(volatile uint16_t *dst, uint16_t val)
+{
+ return __atomic_exchange_2(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
/**
* The atomic counter structure.
*/
@@ -392,6 +418,32 @@ rte_atomic32_cmpset(volatile uint32_t *dst, uint32_t exp, uint32_t src)
}
#endif
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ * ret = *dst
+ * *dst = val;
+ * return ret;
+ *
+ * @param dst
+ * The destination location into which the value will be written.
+ * @param val
+ * The new value.
+ * @return
+ * The original value at that location
+ */
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint32_t
+rte_atomic32_exchange(volatile uint32_t *dst, uint32_t val)
+{
+ return __atomic_exchange_4(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
/**
* The atomic counter structure.
*/
@@ -644,6 +696,32 @@ rte_atomic64_cmpset(volatile uint64_t *dst, uint64_t exp, uint64_t src)
}
#endif
+/**
+ * Atomic exchange.
+ *
+ * (atomic) equivalent to:
+ * ret = *dst
+ * *dst = val;
+ * return ret;
+ *
+ * @param dst
+ * The destination location into which the value will be written.
+ * @param val
+ * The new value.
+ * @return
+ * The original value at that location
+ */
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val);
+
+#ifdef RTE_FORCE_INTRINSICS
+static inline uint64_t
+rte_atomic64_exchange(volatile uint64_t *dst, uint64_t val)
+{
+ return __atomic_exchange_8(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
/**
* The atomic counter structure.
*/