diff mbox

[dpdk-dev,v6,01/14] eal: introduce atomic exchange operation

Message ID 20180121185956.32970-1-ferruh.yigit@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Ferruh Yigit Jan. 21, 2018, 6:59 p.m. UTC 
  From: Stephen Hemminger <stephen@networkplumber.org>

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>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v6:
*fix build error caused by rte_atomic64_t_cmpset
---
 .../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 ++++++++++++++++++++++
 4 files changed, 126 insertions(+)

Comments

Ferruh Yigit Jan. 21, 2018, 7:25 p.m. UTC | #1 
On 1/21/2018 6:59 PM, Ferruh Yigit wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> 
> 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>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Series applied to dpdk-next-net/master, thanks.
Thomas Monjalon Jan. 21, 2018, 11:50 p.m. UTC | #2 
21/01/2018 20:25, Ferruh Yigit:
> On 1/21/2018 6:59 PM, Ferruh Yigit wrote:
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > 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>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Series applied to dpdk-next-net/master, thanks.

I need to drop this series when pulling in master,
because PPC is not supported.

Chao, please could you help on this feature?
Thanks
Stephen Hemminger Jan. 22, 2018, 4:56 p.m. UTC | #3 
On Mon, 22 Jan 2018 00:50:55 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 21/01/2018 20:25, Ferruh Yigit:
> > On 1/21/2018 6:59 PM, Ferruh Yigit wrote:  
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > 
> > > 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>
> > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>  
> > Series applied to dpdk-next-net/master, thanks.  
> 
> I need to drop this series when pulling in master,
> because PPC is not supported.
> 
> Chao, please could you help on this feature?
> Thanks

The generic code should work for PPC.
Thomas Monjalon Jan. 22, 2018, 9:03 p.m. UTC | #4 
22/01/2018 17:56, Stephen Hemminger:
> On Mon, 22 Jan 2018 00:50:55 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 21/01/2018 20:25, Ferruh Yigit:
> > > On 1/21/2018 6:59 PM, Ferruh Yigit wrote:  
> > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > 
> > > > 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>
> > > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>  
> > > Series applied to dpdk-next-net/master, thanks.  
> > 
> > I need to drop this series when pulling in master,
> > because PPC is not supported.
> > 
> > Chao, please could you help on this feature?
> > Thanks
> 
> The generic code should work for PPC.

No, the generic code is inside #ifdef RTE_FORCE_INTRINSICS.
PPC does not define RTE_FORCE_INTRINSICS,
and ARM could disable it I guess.
Stephen Hemminger Jan. 22, 2018, 9:48 p.m. UTC | #5 
On Mon, 22 Jan 2018 22:03:02 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:

> 22/01/2018 17:56, Stephen Hemminger:
> > On Mon, 22 Jan 2018 00:50:55 +0100
> > Thomas Monjalon <thomas@monjalon.net> wrote:
> >   
> > > 21/01/2018 20:25, Ferruh Yigit:  
> > > > On 1/21/2018 6:59 PM, Ferruh Yigit wrote:    
> > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > 
> > > > > 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>
> > > > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>    
> > > > Series applied to dpdk-next-net/master, thanks.    
> > > 
> > > I need to drop this series when pulling in master,
> > > because PPC is not supported.
> > > 
> > > Chao, please could you help on this feature?
> > > Thanks  
> > 
> > The generic code should work for PPC.  
> 
> No, the generic code is inside #ifdef RTE_FORCE_INTRINSICS.
> PPC does not define RTE_FORCE_INTRINSICS,
> and ARM could disable it I guess.

I will add a non intrinsic generic version using cmpset.
Thomas Monjalon Jan. 22, 2018, 10:09 p.m. UTC | #6 
22/01/2018 22:48, Stephen Hemminger:
> On Mon, 22 Jan 2018 22:03:02 +0100
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 22/01/2018 17:56, Stephen Hemminger:
> > > On Mon, 22 Jan 2018 00:50:55 +0100
> > > Thomas Monjalon <thomas@monjalon.net> wrote:
> > >   
> > > > 21/01/2018 20:25, Ferruh Yigit:  
> > > > > On 1/21/2018 6:59 PM, Ferruh Yigit wrote:    
> > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > > 
> > > > > > 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>
> > > > > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>    
> > > > > Series applied to dpdk-next-net/master, thanks.    
> > > > 
> > > > I need to drop this series when pulling in master,
> > > > because PPC is not supported.
> > > > 
> > > > Chao, please could you help on this feature?
> > > > Thanks  
> > > 
> > > The generic code should work for PPC.  
> > 
> > No, the generic code is inside #ifdef RTE_FORCE_INTRINSICS.
> > PPC does not define RTE_FORCE_INTRINSICS,
> > and ARM could disable it I guess.
> 
> I will add a non intrinsic generic version using cmpset.

OK thanks
diff mbox

Patch

diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
index 36cfabc38..55bfc3903 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
@@ -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);
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
index fb3abf187..8d711b6f6 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic_32.h
@@ -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)
 {
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 1a53a766b..fd2ec9c53 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
@@ -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)
 {
diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
index 3ba7245a3..d3348343c 100644
--- a/lib/librte_eal/common/include/generic/rte_atomic.h
+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
@@ -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_4(dst, val, __ATOMIC_SEQ_CST);
+}
+#endif
+
 /**
  * The atomic counter structure.
  */