[dpdk-dev] [PATCH v8 0/3] support c11 memory model barrier in librte_ring

[dpdk-dev] [PATCH v8 0/3] support c11 memory model barrier in librte_ring

[Jia He]

To support C11 memory model barrier, 2 options are suggested by Jerin:
1. use rte_smp_rmb
2. use load_acquire/store_release
CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
on any architectures so far.

The reason why providing 2 options is due to the performance benchmark
difference in different arm machines.

Already fuctionally tested on the machines as follows:
- on X86
- on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
- on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=n

---
Changelog:
V8: Change the lincense to SPDX tag. Change USE_C11_MEM_MODEL to "n" on
    any architectures by default
V7: fix check-git-log warnings which is suggested by Jerin
V6: minor change in subject and log
V5: split it into 2 patchset due to the milestone concerns, this is the 2st
    one. Also fix checkpatch.pl warnings
V4: split into small patches
V3: arch specific implementation for enqueue/dequeue barrier
V2: let users choose whether using load_acquire/store_release
V1: rte_smp_rmb() between 2 loads

Jia He (3):
  eal/arm64: remove the braces {} for dmb() and dsb()
  ring: introduce new header file to include common functions
  ring: introduce new header file to support C11 memory model

 config/common_linuxapp                             |   2 +
 .../common/include/arch/arm/rte_atomic_64.h        |   4 +-
 lib/librte_eventdev/rte_event_ring.h               |   6 +-
 lib/librte_ring/Makefile                           |   4 +-
 lib/librte_ring/rte_ring.h                         | 173 ++----------------
 lib/librte_ring/rte_ring_c11_mem.h                 | 193 ++++++++++++++++++++
 lib/librte_ring/rte_ring_generic.h                 | 202 +++++++++++++++++++++
 7 files changed, 420 insertions(+), 164 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_c11_mem.h
 create mode 100644 lib/librte_ring/rte_ring_generic.h

-- 
2.7.4

[dpdk-dev] [PATCH v8 1/3] eal/arm64: remove the braces {} for dmb() and dsb()

[Jia He]

for the code as follows:
if (condition)
	rte_smp_rmb();
else
	rte_smp_wmb();
Without this patch, compiler will report this error:
error: 'else' without a previous 'if'

Fixes: 84733fd0d75e ("eal/arm64: fix memory barrier definition")
Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
index b6bbd0b..10ccf14 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
@@ -15,8 +15,8 @@ extern "C" {
 
 #include "generic/rte_atomic.h"
 
-#define dsb(opt)  { asm volatile("dsb " #opt : : : "memory"); }
-#define dmb(opt)  { asm volatile("dmb " #opt : : : "memory"); }
+#define dsb(opt) asm volatile("dsb " #opt : : : "memory")
+#define dmb(opt) asm volatile("dmb " #opt : : : "memory")
 
 #define rte_mb() dsb(sy)
 
-- 
2.7.4

[dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Jia He]

Move the common part of rte_ring.h into rte_ring_generic.h.
Move the memory barrier part into update_tail().

No functional changes here.

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eventdev/rte_event_ring.h |   6 +-
 lib/librte_ring/Makefile             |   3 +-
 lib/librte_ring/rte_ring.h           | 161 +---------------------------
 lib/librte_ring/rte_ring_generic.h   | 202 +++++++++++++++++++++++++++++++++++
 4 files changed, 210 insertions(+), 162 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_generic.h

diff --git a/lib/librte_eventdev/rte_event_ring.h b/lib/librte_eventdev/rte_event_ring.h
index 480d1d0..29d4228 100644
--- a/lib/librte_eventdev/rte_event_ring.h
+++ b/lib/librte_eventdev/rte_event_ring.h
@@ -98,9 +98,8 @@ rte_event_ring_enqueue_burst(struct rte_event_ring *r,
 		goto end;
 
 	ENQUEUE_PTRS(&r->r, &r[1], prod_head, events, n, struct rte_event);
-	rte_smp_wmb();
 
-	update_tail(&r->r.prod, prod_head, prod_next, 1);
+	update_tail(&r->r.prod, prod_head, prod_next, 1, 1);
 end:
 	if (free_space != NULL)
 		*free_space = free_entries - n;
@@ -140,9 +139,8 @@ rte_event_ring_dequeue_burst(struct rte_event_ring *r,
 		goto end;
 
 	DEQUEUE_PTRS(&r->r, &r[1], cons_head, events, n, struct rte_event);
-	rte_smp_rmb();
 
-	update_tail(&r->r.cons, cons_head, cons_next, 1);
+	update_tail(&r->r.cons, cons_head, cons_next, 1, 0);
 
 end:
 	if (available != NULL)
diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
index f5f0d35..281ffb7 100644
--- a/lib/librte_ring/Makefile
+++ b/lib/librte_ring/Makefile
@@ -17,6 +17,7 @@ LIBABIVER := 1
 SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
 
 # install includes
-SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h \
+					rte_ring_generic.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index e924438..519614c 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -356,91 +356,8 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r);
 	} \
 } while (0)
 
-static __rte_always_inline void
-update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
-		uint32_t single)
-{
-	/*
-	 * If there are other enqueues/dequeues in progress that preceded us,
-	 * we need to wait for them to complete
-	 */
-	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
-
-	ht->tail = new_val;
-}
-
-/**
- * @internal This function updates the producer head for enqueue
- *
- * @param r
- *   A pointer to the ring structure
- * @param is_sp
- *   Indicates whether multi-producer path is needed or not
- * @param n
- *   The number of elements we will want to enqueue, i.e. how far should the
- *   head be moved
- * @param behavior
- *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
- *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
- * @param old_head
- *   Returns head value as it was before the move, i.e. where enqueue starts
- * @param new_head
- *   Returns the current/new head value i.e. where enqueue finishes
- * @param free_entries
- *   Returns the amount of free space in the ring BEFORE head was moved
- * @return
- *   Actual number of objects enqueued.
- *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
- */
-static __rte_always_inline unsigned int
-__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
-		unsigned int n, enum rte_ring_queue_behavior behavior,
-		uint32_t *old_head, uint32_t *new_head,
-		uint32_t *free_entries)
-{
-	const uint32_t capacity = r->capacity;
-	unsigned int max = n;
-	int success;
-
-	do {
-		/* Reset n to the initial burst count */
-		n = max;
-
-		*old_head = r->prod.head;
-
-		/* add rmb barrier to avoid load/load reorder in weak
-		 * memory model. It is noop on x86
-		 */
-		rte_smp_rmb();
-
-		const uint32_t cons_tail = r->cons.tail;
-		/*
-		 *  The subtraction is done between two unsigned 32bits value
-		 * (the result is always modulo 32 bits even if we have
-		 * *old_head > cons_tail). So 'free_entries' is always between 0
-		 * and capacity (which is < size).
-		 */
-		*free_entries = (capacity + cons_tail - *old_head);
-
-		/* check that we have enough room in ring */
-		if (unlikely(n > *free_entries))
-			n = (behavior == RTE_RING_QUEUE_FIXED) ?
-					0 : *free_entries;
-
-		if (n == 0)
-			return 0;
-
-		*new_head = *old_head + n;
-		if (is_sp)
-			r->prod.head = *new_head, success = 1;
-		else
-			success = rte_atomic32_cmpset(&r->prod.head,
-					*old_head, *new_head);
-	} while (unlikely(success == 0));
-	return n;
-}
+/* Move common functions to generic file */
+#include "rte_ring_generic.h"
 
 /**
  * @internal Enqueue several objects on the ring
@@ -476,9 +393,8 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table,
 		goto end;
 
 	ENQUEUE_PTRS(r, &r[1], prod_head, obj_table, n, void *);
-	rte_smp_wmb();
 
-	update_tail(&r->prod, prod_head, prod_next, is_sp);
+	update_tail(&r->prod, prod_head, prod_next, is_sp, 1);
 end:
 	if (free_space != NULL)
 		*free_space = free_entries - n;
@@ -486,74 +402,6 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table,
 }
 
 /**
- * @internal This function updates the consumer head for dequeue
- *
- * @param r
- *   A pointer to the ring structure
- * @param is_sc
- *   Indicates whether multi-consumer path is needed or not
- * @param n
- *   The number of elements we will want to enqueue, i.e. how far should the
- *   head be moved
- * @param behavior
- *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
- *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
- * @param old_head
- *   Returns head value as it was before the move, i.e. where dequeue starts
- * @param new_head
- *   Returns the current/new head value i.e. where dequeue finishes
- * @param entries
- *   Returns the number of entries in the ring BEFORE head was moved
- * @return
- *   - Actual number of objects dequeued.
- *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
- */
-static __rte_always_inline unsigned int
-__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
-		unsigned int n, enum rte_ring_queue_behavior behavior,
-		uint32_t *old_head, uint32_t *new_head,
-		uint32_t *entries)
-{
-	unsigned int max = n;
-	int success;
-
-	/* move cons.head atomically */
-	do {
-		/* Restore n as it may change every loop */
-		n = max;
-
-		*old_head = r->cons.head;
-
-		/* add rmb barrier to avoid load/load reorder in weak
-		 * memory model. It is noop on x86
-		 */
-		rte_smp_rmb();
-
-		const uint32_t prod_tail = r->prod.tail;
-		/* The subtraction is done between two unsigned 32bits value
-		 * (the result is always modulo 32 bits even if we have
-		 * cons_head > prod_tail). So 'entries' is always between 0
-		 * and size(ring)-1. */
-		*entries = (prod_tail - *old_head);
-
-		/* Set the actual entries for dequeue */
-		if (n > *entries)
-			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
-
-		if (unlikely(n == 0))
-			return 0;
-
-		*new_head = *old_head + n;
-		if (is_sc)
-			r->cons.head = *new_head, success = 1;
-		else
-			success = rte_atomic32_cmpset(&r->cons.head, *old_head,
-					*new_head);
-	} while (unlikely(success == 0));
-	return n;
-}
-
-/**
  * @internal Dequeue several objects from the ring
  *
  * @param r
@@ -587,9 +435,8 @@ __rte_ring_do_dequeue(struct rte_ring *r, void **obj_table,
 		goto end;
 
 	DEQUEUE_PTRS(r, &r[1], cons_head, obj_table, n, void *);
-	rte_smp_rmb();
 
-	update_tail(&r->cons, cons_head, cons_next, is_sc);
+	update_tail(&r->cons, cons_head, cons_next, is_sc, 0);
 
 end:
 	if (available != NULL)
diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
new file mode 100644
index 0000000..01f2cae
--- /dev/null
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -0,0 +1,202 @@
+/*-
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/*
+ * Derived from FreeBSD's bufring.h
+ *
+ **************************************************************************
+ *
+ * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *
+ * 2. The name of Kip Macy nor the names of other
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ***************************************************************************/
+
+#ifndef _RTE_RING_GENERIC_H_
+#define _RTE_RING_GENERIC_H_
+
+static __rte_always_inline void
+update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
+		uint32_t single, uint32_t enqueue)
+{
+	if (enqueue)
+		rte_smp_wmb();
+	else
+		rte_smp_rmb();
+	/*
+	 * If there are other enqueues/dequeues in progress that preceded us,
+	 * we need to wait for them to complete
+	 */
+	if (!single)
+		while (unlikely(ht->tail != old_val))
+			rte_pause();
+
+	ht->tail = new_val;
+}
+
+/**
+ * @internal This function updates the producer head for enqueue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sp
+ *   Indicates whether multi-producer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where enqueue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where enqueue finishes
+ * @param free_entries
+ *   Returns the amount of free space in the ring BEFORE head was moved
+ * @return
+ *   Actual number of objects enqueued.
+ *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *free_entries)
+{
+	const uint32_t capacity = r->capacity;
+	unsigned int max = n;
+	int success;
+
+	do {
+		/* Reset n to the initial burst count */
+		n = max;
+
+		*old_head = r->prod.head;
+
+		/* add rmb barrier to avoid load/load reorder in weak
+		 * memory model. It is noop on x86
+		 */
+		rte_smp_rmb();
+
+		const uint32_t cons_tail = r->cons.tail;
+		/*
+		 *  The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * *old_head > cons_tail). So 'free_entries' is always between 0
+		 * and capacity (which is < size).
+		 */
+		*free_entries = (capacity + cons_tail - *old_head);
+
+		/* check that we have enough room in ring */
+		if (unlikely(n > *free_entries))
+			n = (behavior == RTE_RING_QUEUE_FIXED) ?
+					0 : *free_entries;
+
+		if (n == 0)
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sp)
+			r->prod.head = *new_head, success = 1;
+		else
+			success = rte_atomic32_cmpset(&r->prod.head,
+					*old_head, *new_head);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+/**
+ * @internal This function updates the consumer head for dequeue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sc
+ *   Indicates whether multi-consumer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where dequeue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where dequeue finishes
+ * @param entries
+ *   Returns the number of entries in the ring BEFORE head was moved
+ * @return
+ *   - Actual number of objects dequeued.
+ *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *entries)
+{
+	unsigned int max = n;
+	int success;
+
+	/* move cons.head atomically */
+	do {
+		/* Restore n as it may change every loop */
+		n = max;
+
+		*old_head = r->cons.head;
+
+		/* add rmb barrier to avoid load/load reorder in weak
+		 * memory model. It is noop on x86
+		 */
+		rte_smp_rmb();
+
+		const uint32_t prod_tail = r->prod.tail;
+		/* The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * cons_head > prod_tail). So 'entries' is always between 0
+		 * and size(ring)-1.
+		 */
+		*entries = (prod_tail - *old_head);
+
+		/* Set the actual entries for dequeue */
+		if (n > *entries)
+			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
+
+		if (unlikely(n == 0))
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sc)
+			r->cons.head = *new_head, success = 1;
+		else
+			success = rte_atomic32_cmpset(&r->cons.head, *old_head,
+					*new_head);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+#endif /* _RTE_RING_GENERIC_H_ */
-- 
2.7.4

[dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Jia He]

To support C11 memory model barrier, 2 options are suggested by Jerin:
1. use rte_smp_rmb
2. use load_acquire/store_release(refer to [1]).
CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
on any architectures so far.

The reason why providing 2 options is due to the performance benchmark
difference in different arm machines, refer to [2].

We haven't tested on ppc64. If anyone verifies it, he can add
CONFIG_RTE_RING_USE_C11_MEM_MODEL=y to ppc64 config files.

[1] https://github.com/freebsd/freebsd/blob/master/sys/sys/buf_ring.h#L170
[2] http://dpdk.org/ml/archives/dev/2017-October/080861.html

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Jianbo Liu <jianbo.liu@arm.com>
---
 config/common_linuxapp             |   2 +
 lib/librte_ring/Makefile           |   3 +-
 lib/librte_ring/rte_ring.h         |  14 ++-
 lib/librte_ring/rte_ring_c11_mem.h | 193 +++++++++++++++++++++++++++++++++++++
 4 files changed, 210 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_c11_mem.h

diff --git a/config/common_linuxapp b/config/common_linuxapp
index 74c7d64..999fbaa 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -50,3 +50,5 @@ CONFIG_RTE_LIBRTE_AVP_PMD=y
 CONFIG_RTE_LIBRTE_NFP_PMD=y
 CONFIG_RTE_LIBRTE_POWER=y
 CONFIG_RTE_VIRTIO_USER=y
+
+CONFIG_RTE_RING_USE_C11_MEM_MODEL=n
diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
index 281ffb7..bde8907 100644
--- a/lib/librte_ring/Makefile
+++ b/lib/librte_ring/Makefile
@@ -18,6 +18,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
 
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h \
-					rte_ring_generic.h
+					rte_ring_generic.h \
+					rte_ring_c11_mem.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index 519614c..3343eba 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -356,8 +356,20 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r);
 	} \
 } while (0)
 
-/* Move common functions to generic file */
+/* Between load and load. there might be cpu reorder in weak model
+ * (powerpc/arm).
+ * There are 2 choices for the users
+ * 1.use rmb() memory barrier
+ * 2.use one-direcion load_acquire/store_release barrier,defined by
+ * CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
+ * It depends on performance test results.
+ * By default, move common functions to rte_ring_generic.h
+ */
+#ifdef RTE_RING_USE_C11_MEM_MODEL
+#include "rte_ring_c11_mem.h"
+#else
 #include "rte_ring_generic.h"
+#endif
 
 /**
  * @internal Enqueue several objects on the ring
diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
new file mode 100644
index 0000000..20b7ff6
--- /dev/null
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -0,0 +1,193 @@
+/*-
+ *   Copyright(c) 2017 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/*
+ * Derived from FreeBSD's bufring.h
+ *
+ **************************************************************************
+ *
+ * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *
+ * 2. The name of Kip Macy nor the names of other
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ***************************************************************************/
+
+#ifndef _RTE_RING_C11_MEM_H_
+#define _RTE_RING_C11_MEM_H_
+
+static __rte_always_inline void
+update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
+		uint32_t single, uint32_t enqueue)
+{
+	RTE_SET_USED(enqueue);
+
+	/*
+	 * If there are other enqueues/dequeues in progress that preceded us,
+	 * we need to wait for them to complete
+	 */
+	if (!single)
+		while (unlikely(ht->tail != old_val))
+			rte_pause();
+
+	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
+}
+
+/**
+ * @internal This function updates the producer head for enqueue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sp
+ *   Indicates whether multi-producer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where enqueue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where enqueue finishes
+ * @param free_entries
+ *   Returns the amount of free space in the ring BEFORE head was moved
+ * @return
+ *   Actual number of objects enqueued.
+ *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *free_entries)
+{
+	const uint32_t capacity = r->capacity;
+	unsigned int max = n;
+	int success;
+
+	do {
+		/* Reset n to the initial burst count */
+		n = max;
+
+		*old_head = __atomic_load_n(&r->prod.head,
+					__ATOMIC_ACQUIRE);
+		const uint32_t cons_tail = r->cons.tail;
+		/*
+		 *  The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * *old_head > cons_tail). So 'free_entries' is always between 0
+		 * and capacity (which is < size).
+		 */
+		*free_entries = (capacity + cons_tail - *old_head);
+
+		/* check that we have enough room in ring */
+		if (unlikely(n > *free_entries))
+			n = (behavior == RTE_RING_QUEUE_FIXED) ?
+					0 : *free_entries;
+
+		if (n == 0)
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sp)
+			r->prod.head = *new_head, success = 1;
+		else
+			success = __atomic_compare_exchange_n(&r->prod.head,
+					old_head, *new_head,
+					0, __ATOMIC_ACQUIRE,
+					__ATOMIC_RELAXED);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+/**
+ * @internal This function updates the consumer head for dequeue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sc
+ *   Indicates whether multi-consumer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where dequeue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where dequeue finishes
+ * @param entries
+ *   Returns the number of entries in the ring BEFORE head was moved
+ * @return
+ *   - Actual number of objects dequeued.
+ *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *entries)
+{
+	unsigned int max = n;
+	int success;
+
+	/* move cons.head atomically */
+	do {
+		/* Restore n as it may change every loop */
+		n = max;
+		*old_head = __atomic_load_n(&r->cons.head,
+					__ATOMIC_ACQUIRE);
+		const uint32_t prod_tail = r->prod.tail;
+		/* The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * cons_head > prod_tail). So 'entries' is always between 0
+		 * and size(ring)-1.
+		 */
+		*entries = (prod_tail - *old_head);
+
+		/* Set the actual entries for dequeue */
+		if (n > *entries)
+			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
+
+		if (unlikely(n == 0))
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sc)
+			r->cons.head = *new_head, success = 1;
+		else
+			success = __atomic_compare_exchange_n(&r->cons.head,
+							old_head, *new_head,
+							0, __ATOMIC_ACQUIRE,
+							__ATOMIC_RELAXED);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+#endif /* _RTE_RING_C11_MEM_H_ */
-- 
2.7.4

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Thomas Monjalon]

17/01/2018 05:03, Jia He:
> To support C11 memory model barrier, 2 options are suggested by Jerin:
> 1. use rte_smp_rmb
> 2. use load_acquire/store_release(refer to [1]).
> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
> on any architectures so far.

In previous patches, it was enabled for ARM.
You decided to not enable it at all?

>  config/common_linuxapp             |   2 +

It should be defined in common_base, not common_linuxapp.

> --- /dev/null
> +++ b/lib/librte_ring/rte_ring_c11_mem.h
> @@ -0,0 +1,193 @@
> +/*-
> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   SPDX-License-Identifier: BSD-3-Clause
> + */

It is not complying with the template.
Please check the license/ directory.

Why is it Intel Copyright?
"All rights reserved" is probably not needed.

> +/*
> + * Derived from FreeBSD's bufring.h
> + *
> + **************************************************************************
> + *
> + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright notice,
> + *    this list of conditions and the following disclaimer.
> + *
> + * 2. The name of Kip Macy nor the names of other
> + *    contributors may be used to endorse or promote products derived from
> + *    this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + *
> + ***************************************************************************/

This double license may be an issue.
Hemant, comment?

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Jia He]

Hi Thomas

On 1/17/2018 4:24 PM, Thomas Monjalon Wrote:
> 17/01/2018 05:03, Jia He:
>> To support C11 memory model barrier, 2 options are suggested by Jerin:
>> 1. use rte_smp_rmb
>> 2. use load_acquire/store_release(refer to [1]).
>> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
>> on any architectures so far.
> In previous patches, it was enabled for ARM.
> You decided to not enable it at all?
Sorry, maybe I misunderstand your previous mail.
 >This config option should be added in the common file (as disabled).
Do you mean  CONFIG_RTE_RING_USE_C11_MEM_MODEL=n in comm_base and
"y" in armv8 config?

Cheers,
Jia
>
>>   config/common_linuxapp             |   2 +
> It should be defined in common_base, not common_linuxapp.
>
>> --- /dev/null
>> +++ b/lib/librte_ring/rte_ring_c11_mem.h
>> @@ -0,0 +1,193 @@
>> +/*-
>> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
>> + *   All rights reserved.
>> + *
>> + *   SPDX-License-Identifier: BSD-3-Clause
>> + */
> It is not complying with the template.
> Please check the license/ directory.
>
> Why is it Intel Copyright?
> "All rights reserved" is probably not needed.
>
>> +/*
>> + * Derived from FreeBSD's bufring.h
>> + *
>> + **************************************************************************
>> + *
>> + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
>> + * All rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright notice,
>> + *    this list of conditions and the following disclaimer.
>> + *
>> + * 2. The name of Kip Macy nor the names of other
>> + *    contributors may be used to endorse or promote products derived from
>> + *    this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
>> + * POSSIBILITY OF SUCH DAMAGE.
>> + *
>> + ***************************************************************************/
> This double license may be an issue.
> Hemant, comment?
>

-- 
Cheers,
Jia

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Thomas Monjalon]

17/01/2018 09:47, Jia He:
> 
> Hi Thomas
> 
> On 1/17/2018 4:24 PM, Thomas Monjalon Wrote:
> > 17/01/2018 05:03, Jia He:
> >> To support C11 memory model barrier, 2 options are suggested by Jerin:
> >> 1. use rte_smp_rmb
> >> 2. use load_acquire/store_release(refer to [1]).
> >> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
> >> on any architectures so far.
> > In previous patches, it was enabled for ARM.
> > You decided to not enable it at all?
> Sorry, maybe I misunderstand your previous mail.
>  >This config option should be added in the common file (as disabled).
> Do you mean  CONFIG_RTE_RING_USE_C11_MEM_MODEL=n in comm_base and
> "y" in armv8 config?

Yes, exactly

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Thomas Monjalon]

17/01/2018 10:09, Thomas Monjalon:
> 17/01/2018 09:47, Jia He:
> > 
> > Hi Thomas
> > 
> > On 1/17/2018 4:24 PM, Thomas Monjalon Wrote:
> > > 17/01/2018 05:03, Jia He:
> > >> To support C11 memory model barrier, 2 options are suggested by Jerin:
> > >> 1. use rte_smp_rmb
> > >> 2. use load_acquire/store_release(refer to [1]).
> > >> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
> > >> on any architectures so far.
> > > In previous patches, it was enabled for ARM.
> > > You decided to not enable it at all?
> > Sorry, maybe I misunderstand your previous mail.
> >  >This config option should be added in the common file (as disabled).
> > Do you mean  CONFIG_RTE_RING_USE_C11_MEM_MODEL=n in comm_base and
> > "y" in armv8 config?
> 
> Yes, exactly

Please, could you send a v9?
Thanks

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Jia He]

On 1/19/2018 7:52 AM, Thomas Monjalon Wrote:
> 17/01/2018 10:09, Thomas Monjalon:
>> 17/01/2018 09:47, Jia He:
>>> Hi Thomas
>>>
>>> On 1/17/2018 4:24 PM, Thomas Monjalon Wrote:
>>>> 17/01/2018 05:03, Jia He:
>>>>> To support C11 memory model barrier, 2 options are suggested by Jerin:
>>>>> 1. use rte_smp_rmb
>>>>> 2. use load_acquire/store_release(refer to [1]).
>>>>> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
>>>>> on any architectures so far.
>>>> In previous patches, it was enabled for ARM.
>>>> You decided to not enable it at all?
>>> Sorry, maybe I misunderstand your previous mail.
>>>   >This config option should be added in the common file (as disabled).
>>> Do you mean  CONFIG_RTE_RING_USE_C11_MEM_MODEL=n in comm_base and
>>> "y" in armv8 config?
>> Yes, exactly
> Please, could you send a v9?
> Thanks

>>This double  license may be an issue.

>>Hemant,  comment?

Ok， should I remove the double license info before Hemant's confirmation?

-- 
Cheers,
Jia

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Hemant Agrawal]

Hi Jia,

On 1/17/2018 9:33 AM, Jia He wrote:
> Move the common part of rte_ring.h into rte_ring_generic.h.
> Move the memory barrier part into update_tail().
>
> No functional changes here.
>
> Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
> new file mode 100644
> index 0000000..01f2cae
> --- /dev/null
> +++ b/lib/librte_ring/rte_ring_generic.h
> @@ -0,0 +1,202 @@
> +/*-
> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   SPDX-License-Identifier: BSD-3-Clause

The SPDX should be first line. See other files for Intel or NXP.

> + */
> +
> +/*
> + * Derived from FreeBSD's bufring.h
> + *
> + **************************************************************************
> + *
> + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright notice,
> + *    this list of conditions and the following disclaimer.
> + *
> + * 2. The name of Kip Macy nor the names of other
> + *    contributors may be used to endorse or promote products derived from
> + *    this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + *
> + ***************************************************************************/
> +

This is BSD-2-freebsd, which is not a approved license for DPDK.
Can you ask Kip Macy, if he/she is ok to re-license it with BSD-3?

Please check with legal, if you can just keep the copyright of Kip Macy 
and re license it with BSD-3.

I see the BSD-3 license to be permissive enough to be re-licensed as 
BSD-3. But I am not a lawyer.

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Hemant Agrawal]

HI Jia,

On 1/17/2018 2:17 PM, Jia He wrote:
>
> Hi Thomas
>
> On 1/17/2018 4:24 PM, Thomas Monjalon Wrote:
>> 17/01/2018 05:03, Jia He:
>>> To support C11 memory model barrier, 2 options are suggested by Jerin:
>>> 1. use rte_smp_rmb
>>> 2. use load_acquire/store_release(refer to [1]).
>>> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
>>> on any architectures so far.
>> In previous patches, it was enabled for ARM.
>> You decided to not enable it at all?
> Sorry, maybe I misunderstand your previous mail.
>>This config option should be added in the common file (as disabled).
> Do you mean  CONFIG_RTE_RING_USE_C11_MEM_MODEL=n in comm_base and
> "y" in armv8 config?
>
> Cheers,
> Jia
>>
>>>   config/common_linuxapp             |   2 +
>> It should be defined in common_base, not common_linuxapp.
>>
>>> --- /dev/null
>>> +++ b/lib/librte_ring/rte_ring_c11_mem.h
>>> @@ -0,0 +1,193 @@
>>> +/*-
>>> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
>>> + *   All rights reserved.
>>> + *
>>> + *   SPDX-License-Identifier: BSD-3-Clause
>>> + */
>> It is not complying with the template.
>> Please check the license/ directory.
>>
>> Why is it Intel Copyright?
>> "All rights reserved" is probably not needed.

SPDX shall be first line.
Agree, why are you copyrighting it to Intel?

>>
>>> +/*
>>> + * Derived from FreeBSD's bufring.h
>>> + *
>>> +
>>> **************************************************************************
>>>
>>> + *
>>> + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
>>> + * All rights reserved.
>>> + *
>>> + * Redistribution and use in source and binary forms, with or without
>>> + * modification, are permitted provided that the following
>>> conditions are met:
>>> + *
>>> + * 1. Redistributions of source code must retain the above copyright
>>> notice,
>>> + *    this list of conditions and the following disclaimer.
>>> + *
>>> + * 2. The name of Kip Macy nor the names of other
>>> + *    contributors may be used to endorse or promote products
>>> derived from
>>> + *    this software without specific prior written permission.
>>> + *
>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>>> CONTRIBUTORS "AS IS"
>>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
>>> TO, THE
>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
>>> PARTICULAR PURPOSE
>>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
>>> CONTRIBUTORS BE
>>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
>>> BUSINESS
>>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
>>> WHETHER IN
>>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
>>> OTHERWISE)
>>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
>>> ADVISED OF THE
>>> + * POSSIBILITY OF SUCH DAMAGE.
>>> + *
>>> +
>>> ***************************************************************************/
>>>
>> This double license may be an issue.
>> Hemant, comment?
>>
>

Check my response in your other patch. This is not a DPDK acceptable 
license. It need approval from GB.

I could not locate a BSD-3 version of bufring.h
What can be accepted in this case, without GB approval:
BSD-3 only license
  + Keep copyright of Kip Macy
  + Put statement that Kip Macy allowed it for BSD-3 uses. Or, Your 
company (the copyright owner) is re-licensing the original 
"BSD-2-freebsd" as "BSD-3".
+ keep the Derived from statement.

Re: [dpdk-dev] [PATCH v8 3/3] ring: introduce new header file to support C11 memory model

[Hemant Agrawal]

My thunderbird mails are getting delayed.

I have provided a response there that this patch is not ok in it's current state.

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Olivier Matz]

Hi Hemant,

On Fri, Jan 19, 2018 at 07:45:30PM +0530, Hemant Agrawal wrote:
> Hi Jia,
> 
> On 1/17/2018 9:33 AM, Jia He wrote:
> > Move the common part of rte_ring.h into rte_ring_generic.h.
> > Move the memory barrier part into update_tail().
> > 
> > No functional changes here.
> > 
> > Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> > Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
> > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> > diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
> > new file mode 100644
> > index 0000000..01f2cae
> > --- /dev/null
> > +++ b/lib/librte_ring/rte_ring_generic.h
> > @@ -0,0 +1,202 @@
> > +/*-
> > + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   SPDX-License-Identifier: BSD-3-Clause
> 
> The SPDX should be first line. See other files for Intel or NXP.
> 
> > + */
> > +
> > +/*
> > + * Derived from FreeBSD's bufring.h
> > + *
> > + **************************************************************************
> > + *
> > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are met:
> > + *
> > + * 1. Redistributions of source code must retain the above copyright notice,
> > + *    this list of conditions and the following disclaimer.
> > + *
> > + * 2. The name of Kip Macy nor the names of other
> > + *    contributors may be used to endorse or promote products derived from
> > + *    this software without specific prior written permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + ***************************************************************************/
> > +
> 
> This is BSD-2-freebsd, which is not a approved license for DPDK.
> Can you ask Kip Macy, if he/she is ok to re-license it with BSD-3?
> 
> Please check with legal, if you can just keep the copyright of Kip Macy and
> re license it with BSD-3.
> 
> I see the BSD-3 license to be permissive enough to be re-licensed as BSD-3.
> But I am not a lawyer.
> 

I agree this is something we should do, as a maintainer of librte_ring,
I can do it.

But here, Jia is just moving code in a new file. I don't think this should
block his patchset from beeing included.

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Hemant Agrawal]

Hi Olivier,

> On Fri, Jan 19, 2018 at 07:45:30PM +0530, Hemant Agrawal wrote:
> > Hi Jia,
> >
> > On 1/17/2018 9:33 AM, Jia He wrote:
> > > Move the common part of rte_ring.h into rte_ring_generic.h.
> > > Move the memory barrier part into update_tail().
> > >
> > > No functional changes here.
> > >
> > > Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> > > Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
> > > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > > diff --git a/lib/librte_ring/rte_ring_generic.h
> > > b/lib/librte_ring/rte_ring_generic.h
> > > new file mode 100644
> > > index 0000000..01f2cae
> > > --- /dev/null
> > > +++ b/lib/librte_ring/rte_ring_generic.h
> > > @@ -0,0 +1,202 @@
> > > +/*-
> > > + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> > > + *   All rights reserved.
> > > + *
> > > + *   SPDX-License-Identifier: BSD-3-Clause
> >
> > The SPDX should be first line. See other files for Intel or NXP.

[Hemant] Don't add SPDX to this file. 
 This file is not BSD-3 licensed.  Please keep the full text as in the original file.


>
> > > + */
> > > +
> > > +/*
> > > + * Derived from FreeBSD's bufring.h
> > > + *
> > > +
> > >
> +*********************************************************
> **********
> > > +*******
> > > + *
> > > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
> > > + * All rights reserved.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or
> > > +without
> > > + * modification, are permitted provided that the following conditions
> are met:
> > > + *
> > > + * 1. Redistributions of source code must retain the above copyright
> notice,
> > > + *    this list of conditions and the following disclaimer.
> > > + *
> > > + * 2. The name of Kip Macy nor the names of other
> > > + *    contributors may be used to endorse or promote products derived
> from
> > > + *    this software without specific prior written permission.
> > > + *
> > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > > +LIMITED TO, THE
> > > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> > > +PARTICULAR PURPOSE
> > > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> > > +CONTRIBUTORS BE
> > > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> EXEMPLARY,
> > > +OR
> > > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> > > +PROCUREMENT OF
> > > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
> OR
> > > +BUSINESS
> > > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> LIABILITY,
> > > +WHETHER IN
> > > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> > > +OTHERWISE)
> > > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> > > +ADVISED OF THE
> > > + * POSSIBILITY OF SUCH DAMAGE.
> > > + *
> > > +
> > >
> +*********************************************************
> **********
> > > +********/
> > > +
> >
> > This is BSD-2-freebsd, which is not a approved license for DPDK.
> > Can you ask Kip Macy, if he/she is ok to re-license it with BSD-3?
> >
> > Please check with legal, if you can just keep the copyright of Kip
> > Macy and re license it with BSD-3.
> >
> > I see the BSD-3 license to be permissive enough to be re-licensed as BSD-3.
> > But I am not a lawyer.
> >
> 
> I agree this is something we should do, as a maintainer of librte_ring, I can
> do it.
> 
> But here, Jia is just moving code in a new file. I don't think this should block
> his patchset from beeing included.

 [Hemant]   I thought of blocking this kind of moves, so that we get the license complaint of DPDK faster 😊

Jia, shall keep the original copyrights and headers in this file (i.e. No SPDX).   You need to fix it along with rte_ring.h in near future.

Regards,
Hemant

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Jia He]

Hi Hermant


On 1/20/2018 12:47 AM, Hemant Agrawal Wrote:
> Hi Olivier,
>
>> On Fri, Jan 19, 2018 at 07:45:30PM +0530, Hemant Agrawal wrote:
>>> Hi Jia,
>>>
>>> On 1/17/2018 9:33 AM, Jia He wrote:
>>>> Move the common part of rte_ring.h into rte_ring_generic.h.
>>>> Move the memory barrier part into update_tail().
>>>>
>>>> No functional changes here.
>>>>
>>>> Signed-off-by: Jia He <jia.he@hxt-semitech.com>
>>>> Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>>> Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
>>>> Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>>>> Acked-by: Olivier Matz <olivier.matz@6wind.com>
>>>> ---
>>>> diff --git a/lib/librte_ring/rte_ring_generic.h
>>>> b/lib/librte_ring/rte_ring_generic.h
>>>> new file mode 100644
>>>> index 0000000..01f2cae
>>>> --- /dev/null
>>>> +++ b/lib/librte_ring/rte_ring_generic.h
>>>> @@ -0,0 +1,202 @@
>>>> +/*-
>>>> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
>>>> + *   All rights reserved.
>>>> + *
>>>> + *   SPDX-License-Identifier: BSD-3-Clause
>>> The SPDX should be first line. See other files for Intel or NXP.
> [Hemant] Don't add SPDX to this file.
>   This file is not BSD-3 licensed.  Please keep the full text as in the original file.
>
>
>>>> + */
>>>> +
>>>> +/*
>>>> + * Derived from FreeBSD's bufring.h
>>>> + *
>>>> +
>>>>
>> +*********************************************************
>> **********
>>>> +*******
>>>> + *
>>>> + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
>>>> + * All rights reserved.
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or
>>>> +without
>>>> + * modification, are permitted provided that the following conditions
>> are met:
>>>> + *
>>>> + * 1. Redistributions of source code must retain the above copyright
>> notice,
>>>> + *    this list of conditions and the following disclaimer.
>>>> + *
>>>> + * 2. The name of Kip Macy nor the names of other
>>>> + *    contributors may be used to endorse or promote products derived
>> from
>>>> + *    this software without specific prior written permission.
>>>> + *
>>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>> CONTRIBUTORS "AS IS"
>>>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>>>> +LIMITED TO, THE
>>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
>>>> +PARTICULAR PURPOSE
>>>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
>>>> +CONTRIBUTORS BE
>>>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>> EXEMPLARY,
>>>> +OR
>>>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>>>> +PROCUREMENT OF
>>>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
>> OR
>>>> +BUSINESS
>>>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
>> LIABILITY,
>>>> +WHETHER IN
>>>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
>>>> +OTHERWISE)
>>>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
>>>> +ADVISED OF THE
>>>> + * POSSIBILITY OF SUCH DAMAGE.
>>>> + *
>>>> +
>>>>
>> +*********************************************************
>> **********
>>>> +********/
>>>> +
>>> This is BSD-2-freebsd, which is not a approved license for DPDK.
>>> Can you ask Kip Macy, if he/she is ok to re-license it with BSD-3?
>>>
>>> Please check with legal, if you can just keep the copyright of Kip
>>> Macy and re license it with BSD-3.
>>>
>>> I see the BSD-3 license to be permissive enough to be re-licensed as BSD-3.
>>> But I am not a lawyer.
>>>
>> I agree this is something we should do, as a maintainer of librte_ring, I can
>> do it.
>>
>> But here, Jia is just moving code in a new file. I don't think this should block
>> his patchset from beeing included.
>   [Hemant]   I thought of blocking this kind of moves, so that we get the license complaint of DPDK faster 😊
>
> Jia, shall keep the original copyrights and headers in this file (i.e. No SPDX).   You need to fix it along with rte_ring.h in near future.
>
> Regards,
> Hemant
>
Ok, I will
Besides ,I got the allowance from Kip Macy just now. He/She allowed dpdk 
to license librte_ring.h as BSD-3.

My question:
 >Would you mind allowing dpdk librte_ring.h to be licensed as BSD 3 
instead of BSD 2?
His/her reply:

"I think that's fine. If you're using it be careful I think there's a
fix to memory barrier usage needed more relaxed memory models such as
ARM. I'll check reviews to see if it made it in or not."

-- 
Cheers,
Jia

[dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Jia He]

There are 2 model barrier options in librte_ring suggested by Jerin:
1. use rte_smp_rmb
2. use load_acquire/store_release

CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided for supporting C11 memory
model barrier in librte_ring. By default it is "y" on arm64, "n" on any
other architectures so far.

Already fuctionally tested on the machines as follows:
- on X86
- on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
- on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=n

---
Changelog:
V9: remove the SPDX tag and refine commit logs
V8: Change the license to SPDX tag. Change USE_C11_MEM_MODEL to "n" on
    any architectures by default
V7: fix check-git-log warnings which is suggested by Jerin
V6: minor change in subject and log
V5: split it into 2 patchset due to the milestone concerns, this is the 2st
    one. Also fix checkpatch.pl warnings
V4: split into small patches
V3: arch specific implementation for enqueue/dequeue barrier
V2: let users choose whether using load_acquire/store_release
V1: rte_smp_rmb() between 2 loads

Jia He (3):
  eal/arm64: remove the braces {} for dmb() and dsb()
  ring: introduce new header file to include common functions
  ring: introduce new header file to support C11 memory model

 config/common_armv8a_linuxapp                      |   2 +
 config/common_base                                 |   5 +
 .../common/include/arch/arm/rte_atomic_64.h        |   4 +-
 lib/librte_eventdev/rte_event_ring.h               |   6 +-
 lib/librte_ring/Makefile                           |   4 +-
 lib/librte_ring/rte_ring.h                         | 173 ++--------------
 lib/librte_ring/rte_ring_c11_mem.h                 | 218 ++++++++++++++++++++
 lib/librte_ring/rte_ring_generic.h                 | 228 +++++++++++++++++++++
 8 files changed, 476 insertions(+), 164 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_c11_mem.h
 create mode 100644 lib/librte_ring/rte_ring_generic.h

-- 
2.7.4

[dpdk-dev] [PATCH v9 1/3] eal/arm64: remove the braces {} for dmb() and dsb()

[Jia He]

for the code as follows:
if (condition)
	rte_smp_rmb();
else
	rte_smp_wmb();
Without this patch, compiler will report this error:
error: 'else' without a previous 'if'

Fixes: 84733fd0d75e ("eal/arm64: fix memory barrier definition")
Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
index b6bbd0b..10ccf14 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
@@ -15,8 +15,8 @@ extern "C" {
 
 #include "generic/rte_atomic.h"
 
-#define dsb(opt)  { asm volatile("dsb " #opt : : : "memory"); }
-#define dmb(opt)  { asm volatile("dmb " #opt : : : "memory"); }
+#define dsb(opt) asm volatile("dsb " #opt : : : "memory")
+#define dmb(opt) asm volatile("dmb " #opt : : : "memory")
 
 #define rte_mb() dsb(sy)
 
-- 
2.7.4

[dpdk-dev] [PATCH v9 2/3] ring: introduce new header file to include common functions

[Jia He]

Move the common part of rte_ring.h into rte_ring_generic.h.
Move the memory barrier part into update_tail().

No functional changes here.

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_eventdev/rte_event_ring.h |   6 +-
 lib/librte_ring/Makefile             |   3 +-
 lib/librte_ring/rte_ring.h           | 161 +------------------------
 lib/librte_ring/rte_ring_generic.h   | 228 +++++++++++++++++++++++++++++++++++
 4 files changed, 236 insertions(+), 162 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_generic.h

diff --git a/lib/librte_eventdev/rte_event_ring.h b/lib/librte_eventdev/rte_event_ring.h
index 480d1d0..29d4228 100644
--- a/lib/librte_eventdev/rte_event_ring.h
+++ b/lib/librte_eventdev/rte_event_ring.h
@@ -98,9 +98,8 @@ rte_event_ring_enqueue_burst(struct rte_event_ring *r,
 		goto end;
 
 	ENQUEUE_PTRS(&r->r, &r[1], prod_head, events, n, struct rte_event);
-	rte_smp_wmb();
 
-	update_tail(&r->r.prod, prod_head, prod_next, 1);
+	update_tail(&r->r.prod, prod_head, prod_next, 1, 1);
 end:
 	if (free_space != NULL)
 		*free_space = free_entries - n;
@@ -140,9 +139,8 @@ rte_event_ring_dequeue_burst(struct rte_event_ring *r,
 		goto end;
 
 	DEQUEUE_PTRS(&r->r, &r[1], cons_head, events, n, struct rte_event);
-	rte_smp_rmb();
 
-	update_tail(&r->r.cons, cons_head, cons_next, 1);
+	update_tail(&r->r.cons, cons_head, cons_next, 1, 0);
 
 end:
 	if (available != NULL)
diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
index f5f0d35..281ffb7 100644
--- a/lib/librte_ring/Makefile
+++ b/lib/librte_ring/Makefile
@@ -17,6 +17,7 @@ LIBABIVER := 1
 SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
 
 # install includes
-SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h \
+					rte_ring_generic.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index 7069d52..bc3756a 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -357,91 +357,8 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r);
 	} \
 } while (0)
 
-static __rte_always_inline void
-update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
-		uint32_t single)
-{
-	/*
-	 * If there are other enqueues/dequeues in progress that preceded us,
-	 * we need to wait for them to complete
-	 */
-	if (!single)
-		while (unlikely(ht->tail != old_val))
-			rte_pause();
-
-	ht->tail = new_val;
-}
-
-/**
- * @internal This function updates the producer head for enqueue
- *
- * @param r
- *   A pointer to the ring structure
- * @param is_sp
- *   Indicates whether multi-producer path is needed or not
- * @param n
- *   The number of elements we will want to enqueue, i.e. how far should the
- *   head be moved
- * @param behavior
- *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
- *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
- * @param old_head
- *   Returns head value as it was before the move, i.e. where enqueue starts
- * @param new_head
- *   Returns the current/new head value i.e. where enqueue finishes
- * @param free_entries
- *   Returns the amount of free space in the ring BEFORE head was moved
- * @return
- *   Actual number of objects enqueued.
- *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
- */
-static __rte_always_inline unsigned int
-__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
-		unsigned int n, enum rte_ring_queue_behavior behavior,
-		uint32_t *old_head, uint32_t *new_head,
-		uint32_t *free_entries)
-{
-	const uint32_t capacity = r->capacity;
-	unsigned int max = n;
-	int success;
-
-	do {
-		/* Reset n to the initial burst count */
-		n = max;
-
-		*old_head = r->prod.head;
-
-		/* add rmb barrier to avoid load/load reorder in weak
-		 * memory model. It is noop on x86
-		 */
-		rte_smp_rmb();
-
-		const uint32_t cons_tail = r->cons.tail;
-		/*
-		 *  The subtraction is done between two unsigned 32bits value
-		 * (the result is always modulo 32 bits even if we have
-		 * *old_head > cons_tail). So 'free_entries' is always between 0
-		 * and capacity (which is < size).
-		 */
-		*free_entries = (capacity + cons_tail - *old_head);
-
-		/* check that we have enough room in ring */
-		if (unlikely(n > *free_entries))
-			n = (behavior == RTE_RING_QUEUE_FIXED) ?
-					0 : *free_entries;
-
-		if (n == 0)
-			return 0;
-
-		*new_head = *old_head + n;
-		if (is_sp)
-			r->prod.head = *new_head, success = 1;
-		else
-			success = rte_atomic32_cmpset(&r->prod.head,
-					*old_head, *new_head);
-	} while (unlikely(success == 0));
-	return n;
-}
+/* Move common functions to generic file */
+#include "rte_ring_generic.h"
 
 /**
  * @internal Enqueue several objects on the ring
@@ -477,9 +394,8 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table,
 		goto end;
 
 	ENQUEUE_PTRS(r, &r[1], prod_head, obj_table, n, void *);
-	rte_smp_wmb();
 
-	update_tail(&r->prod, prod_head, prod_next, is_sp);
+	update_tail(&r->prod, prod_head, prod_next, is_sp, 1);
 end:
 	if (free_space != NULL)
 		*free_space = free_entries - n;
@@ -487,74 +403,6 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table,
 }
 
 /**
- * @internal This function updates the consumer head for dequeue
- *
- * @param r
- *   A pointer to the ring structure
- * @param is_sc
- *   Indicates whether multi-consumer path is needed or not
- * @param n
- *   The number of elements we will want to enqueue, i.e. how far should the
- *   head be moved
- * @param behavior
- *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
- *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
- * @param old_head
- *   Returns head value as it was before the move, i.e. where dequeue starts
- * @param new_head
- *   Returns the current/new head value i.e. where dequeue finishes
- * @param entries
- *   Returns the number of entries in the ring BEFORE head was moved
- * @return
- *   - Actual number of objects dequeued.
- *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
- */
-static __rte_always_inline unsigned int
-__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
-		unsigned int n, enum rte_ring_queue_behavior behavior,
-		uint32_t *old_head, uint32_t *new_head,
-		uint32_t *entries)
-{
-	unsigned int max = n;
-	int success;
-
-	/* move cons.head atomically */
-	do {
-		/* Restore n as it may change every loop */
-		n = max;
-
-		*old_head = r->cons.head;
-
-		/* add rmb barrier to avoid load/load reorder in weak
-		 * memory model. It is noop on x86
-		 */
-		rte_smp_rmb();
-
-		const uint32_t prod_tail = r->prod.tail;
-		/* The subtraction is done between two unsigned 32bits value
-		 * (the result is always modulo 32 bits even if we have
-		 * cons_head > prod_tail). So 'entries' is always between 0
-		 * and size(ring)-1. */
-		*entries = (prod_tail - *old_head);
-
-		/* Set the actual entries for dequeue */
-		if (n > *entries)
-			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
-
-		if (unlikely(n == 0))
-			return 0;
-
-		*new_head = *old_head + n;
-		if (is_sc)
-			r->cons.head = *new_head, success = 1;
-		else
-			success = rte_atomic32_cmpset(&r->cons.head, *old_head,
-					*new_head);
-	} while (unlikely(success == 0));
-	return n;
-}
-
-/**
  * @internal Dequeue several objects from the ring
  *
  * @param r
@@ -588,9 +436,8 @@ __rte_ring_do_dequeue(struct rte_ring *r, void **obj_table,
 		goto end;
 
 	DEQUEUE_PTRS(r, &r[1], cons_head, obj_table, n, void *);
-	rte_smp_rmb();
 
-	update_tail(&r->cons, cons_head, cons_next, is_sc);
+	update_tail(&r->cons, cons_head, cons_next, is_sc, 0);
 
 end:
 	if (available != NULL)
diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h
new file mode 100644
index 0000000..8c3e65b
--- /dev/null
+++ b/lib/librte_ring/rte_ring_generic.h
@@ -0,0 +1,228 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * Derived from FreeBSD's bufring.h
+ *
+ **************************************************************************
+ *
+ * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *
+ * 2. The name of Kip Macy nor the names of other
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ***************************************************************************/
+
+#ifndef _RTE_RING_GENERIC_H_
+#define _RTE_RING_GENERIC_H_
+
+static __rte_always_inline void
+update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
+		uint32_t single, uint32_t enqueue)
+{
+	if (enqueue)
+		rte_smp_wmb();
+	else
+		rte_smp_rmb();
+	/*
+	 * If there are other enqueues/dequeues in progress that preceded us,
+	 * we need to wait for them to complete
+	 */
+	if (!single)
+		while (unlikely(ht->tail != old_val))
+			rte_pause();
+
+	ht->tail = new_val;
+}
+
+/**
+ * @internal This function updates the producer head for enqueue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sp
+ *   Indicates whether multi-producer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where enqueue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where enqueue finishes
+ * @param free_entries
+ *   Returns the amount of free space in the ring BEFORE head was moved
+ * @return
+ *   Actual number of objects enqueued.
+ *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *free_entries)
+{
+	const uint32_t capacity = r->capacity;
+	unsigned int max = n;
+	int success;
+
+	do {
+		/* Reset n to the initial burst count */
+		n = max;
+
+		*old_head = r->prod.head;
+
+		/* add rmb barrier to avoid load/load reorder in weak
+		 * memory model. It is noop on x86
+		 */
+		rte_smp_rmb();
+
+		const uint32_t cons_tail = r->cons.tail;
+		/*
+		 *  The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * *old_head > cons_tail). So 'free_entries' is always between 0
+		 * and capacity (which is < size).
+		 */
+		*free_entries = (capacity + cons_tail - *old_head);
+
+		/* check that we have enough room in ring */
+		if (unlikely(n > *free_entries))
+			n = (behavior == RTE_RING_QUEUE_FIXED) ?
+					0 : *free_entries;
+
+		if (n == 0)
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sp)
+			r->prod.head = *new_head, success = 1;
+		else
+			success = rte_atomic32_cmpset(&r->prod.head,
+					*old_head, *new_head);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+/**
+ * @internal This function updates the consumer head for dequeue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sc
+ *   Indicates whether multi-consumer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where dequeue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where dequeue finishes
+ * @param entries
+ *   Returns the number of entries in the ring BEFORE head was moved
+ * @return
+ *   - Actual number of objects dequeued.
+ *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *entries)
+{
+	unsigned int max = n;
+	int success;
+
+	/* move cons.head atomically */
+	do {
+		/* Restore n as it may change every loop */
+		n = max;
+
+		*old_head = r->cons.head;
+
+		/* add rmb barrier to avoid load/load reorder in weak
+		 * memory model. It is noop on x86
+		 */
+		rte_smp_rmb();
+
+		const uint32_t prod_tail = r->prod.tail;
+		/* The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * cons_head > prod_tail). So 'entries' is always between 0
+		 * and size(ring)-1.
+		 */
+		*entries = (prod_tail - *old_head);
+
+		/* Set the actual entries for dequeue */
+		if (n > *entries)
+			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
+
+		if (unlikely(n == 0))
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sc)
+			r->cons.head = *new_head, success = 1;
+		else
+			success = rte_atomic32_cmpset(&r->cons.head, *old_head,
+					*new_head);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+#endif /* _RTE_RING_GENERIC_H_ */
-- 
2.7.4

[dpdk-dev] [PATCH v9 3/3] ring: introduce new header file to support C11 memory model

[Jia He]

This patch is to support C11 memory model barrier in librte_ring.

There are 2 barrier implementation options in librte_ring (suggested
by Jerin).
1. use rte_smp_rmb
2. use load_acquire/store_release(refer to [1]).
The reason why providing 2 options is the performance benchmark
difference in different arm machines, refer to [2].

CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided, and by default it is "n"
on any architectures and only "y" on arm64 so far.

[1] https://github.com/freebsd/freebsd/blob/master/sys/sys/buf_ring.h#L170
[2] http://dpdk.org/ml/archives/dev/2017-October/080861.html

Signed-off-by: Jia He <jia.he@hxt-semitech.com>
Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: Jianbo Liu <jianbo.liu@arm.com>
---
 config/common_armv8a_linuxapp      |   2 +
 config/common_base                 |   5 +
 lib/librte_ring/Makefile           |   3 +-
 lib/librte_ring/rte_ring.h         |  14 ++-
 lib/librte_ring/rte_ring_c11_mem.h | 218 +++++++++++++++++++++++++++++++++++++
 5 files changed, 240 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_ring/rte_ring_c11_mem.h

diff --git a/config/common_armv8a_linuxapp b/config/common_armv8a_linuxapp
index 790e716..0a1f370 100644
--- a/config/common_armv8a_linuxapp
+++ b/config/common_armv8a_linuxapp
@@ -34,3 +34,5 @@ CONFIG_RTE_LIBRTE_SFC_EFX_PMD=n
 CONFIG_RTE_LIBRTE_AVP_PMD=n
 
 CONFIG_RTE_SCHED_VECTOR=n
+
+CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
diff --git a/config/common_base b/config/common_base
index 170a389..0aadda8 100644
--- a/config/common_base
+++ b/config/common_base
@@ -843,3 +843,8 @@ CONFIG_RTE_APP_CRYPTO_PERF=y
 # Compile the eventdev application
 #
 CONFIG_RTE_APP_EVENTDEV=y
+
+#
+# Use C11 memory model barrier in Librte_ring
+#
+CONFIG_RTE_RING_USE_C11_MEM_MODEL=n
diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile
index 281ffb7..bde8907 100644
--- a/lib/librte_ring/Makefile
+++ b/lib/librte_ring/Makefile
@@ -18,6 +18,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
 
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h \
-					rte_ring_generic.h
+					rte_ring_generic.h \
+					rte_ring_c11_mem.h
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
index bc3756a..cbf2f19 100644
--- a/lib/librte_ring/rte_ring.h
+++ b/lib/librte_ring/rte_ring.h
@@ -357,8 +357,20 @@ void rte_ring_dump(FILE *f, const struct rte_ring *r);
 	} \
 } while (0)
 
-/* Move common functions to generic file */
+/* Between load and load. there might be cpu reorder in weak model
+ * (powerpc/arm).
+ * There are 2 choices for the users
+ * 1.use rmb() memory barrier
+ * 2.use one-direcion load_acquire/store_release barrier,defined by
+ * CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
+ * It depends on performance test results.
+ * By default, move common functions to rte_ring_generic.h
+ */
+#ifdef RTE_RING_USE_C11_MEM_MODEL
+#include "rte_ring_c11_mem.h"
+#else
 #include "rte_ring_generic.h"
+#endif
 
 /**
  * @internal Enqueue several objects on the ring
diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
new file mode 100644
index 0000000..8ccb937
--- /dev/null
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -0,0 +1,218 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2017 HXT-semitech Corporation.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of HXT-semitech Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * Derived from FreeBSD's bufring.h
+ *
+ **************************************************************************
+ *
+ * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ *
+ * 2. The name of Kip Macy nor the names of other
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ***************************************************************************/
+
+#ifndef _RTE_RING_C11_MEM_H_
+#define _RTE_RING_C11_MEM_H_
+
+static __rte_always_inline void
+update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
+		uint32_t single, uint32_t enqueue)
+{
+	RTE_SET_USED(enqueue);
+
+	/*
+	 * If there are other enqueues/dequeues in progress that preceded us,
+	 * we need to wait for them to complete
+	 */
+	if (!single)
+		while (unlikely(ht->tail != old_val))
+			rte_pause();
+
+	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
+}
+
+/**
+ * @internal This function updates the producer head for enqueue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sp
+ *   Indicates whether multi-producer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Enqueue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where enqueue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where enqueue finishes
+ * @param free_entries
+ *   Returns the amount of free space in the ring BEFORE head was moved
+ * @return
+ *   Actual number of objects enqueued.
+ *   If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_prod_head(struct rte_ring *r, int is_sp,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *free_entries)
+{
+	const uint32_t capacity = r->capacity;
+	unsigned int max = n;
+	int success;
+
+	do {
+		/* Reset n to the initial burst count */
+		n = max;
+
+		*old_head = __atomic_load_n(&r->prod.head,
+					__ATOMIC_ACQUIRE);
+		const uint32_t cons_tail = r->cons.tail;
+		/*
+		 *  The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * *old_head > cons_tail). So 'free_entries' is always between 0
+		 * and capacity (which is < size).
+		 */
+		*free_entries = (capacity + cons_tail - *old_head);
+
+		/* check that we have enough room in ring */
+		if (unlikely(n > *free_entries))
+			n = (behavior == RTE_RING_QUEUE_FIXED) ?
+					0 : *free_entries;
+
+		if (n == 0)
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sp)
+			r->prod.head = *new_head, success = 1;
+		else
+			success = __atomic_compare_exchange_n(&r->prod.head,
+					old_head, *new_head,
+					0, __ATOMIC_ACQUIRE,
+					__ATOMIC_RELAXED);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+/**
+ * @internal This function updates the consumer head for dequeue
+ *
+ * @param r
+ *   A pointer to the ring structure
+ * @param is_sc
+ *   Indicates whether multi-consumer path is needed or not
+ * @param n
+ *   The number of elements we will want to enqueue, i.e. how far should the
+ *   head be moved
+ * @param behavior
+ *   RTE_RING_QUEUE_FIXED:    Dequeue a fixed number of items from a ring
+ *   RTE_RING_QUEUE_VARIABLE: Dequeue as many items as possible from ring
+ * @param old_head
+ *   Returns head value as it was before the move, i.e. where dequeue starts
+ * @param new_head
+ *   Returns the current/new head value i.e. where dequeue finishes
+ * @param entries
+ *   Returns the number of entries in the ring BEFORE head was moved
+ * @return
+ *   - Actual number of objects dequeued.
+ *     If behavior == RTE_RING_QUEUE_FIXED, this will be 0 or n only.
+ */
+static __rte_always_inline unsigned int
+__rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
+		unsigned int n, enum rte_ring_queue_behavior behavior,
+		uint32_t *old_head, uint32_t *new_head,
+		uint32_t *entries)
+{
+	unsigned int max = n;
+	int success;
+
+	/* move cons.head atomically */
+	do {
+		/* Restore n as it may change every loop */
+		n = max;
+		*old_head = __atomic_load_n(&r->cons.head,
+					__ATOMIC_ACQUIRE);
+		const uint32_t prod_tail = r->prod.tail;
+		/* The subtraction is done between two unsigned 32bits value
+		 * (the result is always modulo 32 bits even if we have
+		 * cons_head > prod_tail). So 'entries' is always between 0
+		 * and size(ring)-1.
+		 */
+		*entries = (prod_tail - *old_head);
+
+		/* Set the actual entries for dequeue */
+		if (n > *entries)
+			n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : *entries;
+
+		if (unlikely(n == 0))
+			return 0;
+
+		*new_head = *old_head + n;
+		if (is_sc)
+			r->cons.head = *new_head, success = 1;
+		else
+			success = __atomic_compare_exchange_n(&r->cons.head,
+							old_head, *new_head,
+							0, __ATOMIC_ACQUIRE,
+							__ATOMIC_RELAXED);
+	} while (unlikely(success == 0));
+	return n;
+}
+
+#endif /* _RTE_RING_C11_MEM_H_ */
-- 
2.7.4

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Jia He]

Hi hermant


On 1/22/2018 1:15 PM, Hemant Agrawal Wrote:
> Hi Jia,
>
> On 1/22/2018 7:23 AM, Jia He wrote:
>>>>> This is BSD-2-freebsd, which is not a approved license for DPDK.
>>>>> Can you ask Kip Macy, if he/she is ok to re-license it with BSD-3?
>>>>>
>>>>> Please check with legal, if you can just keep the copyright of Kip
>>>>> Macy and re license it with BSD-3.
>>>>>
>>>>> I see the BSD-3 license to be permissive enough to be re-licensed as
>>>>> BSD-3.
>>>>> But I am not a lawyer.
>>>>>
>>>> I agree this is something we should do, as a maintainer of
>>>> librte_ring, I can
>>>> do it.
>>>>
>>>> But here, Jia is just moving code in a new file. I don't think this
>>>> should block
>>>> his patchset from beeing included.
>>>   [Hemant]   I thought of blocking this kind of moves, so that we get
>>> the license complaint of DPDK faster 😊
>>>
>>> Jia, shall keep the original copyrights and headers in this file (i.e.
>>> No SPDX).   You need to fix it along with rte_ring.h in near future.
>>>
>>> Regards,
>>> Hemant
>>>
>> Ok, I will
>> Besides ,I got the allowance from Kip Macy just now. He/She allowed dpdk
>> to license librte_ring.h as BSD-3.
>>
>> My question:
>>> Would you mind allowing dpdk librte_ring.h to be licensed as BSD 3
>> instead of BSD 2?
>> His/her reply:
>>
>> "I think that's fine. If you're using it be careful I think there's a
>> fix to memory barrier usage needed more relaxed memory models such as
>> ARM. I'll check reviews to see if it made it in or not."
>>
>
> That is good. Your Patch v9 looks ok.
>
> Will you please also add another patch for following:
> (all files in librte_ring - having BSD-2 license)
>
> /* SPDX-License-Identifier: BSD-3-Clause
>  *
>  * Copyright ..... (Intel or your company)
>  * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org
>  * All rights reserved.
>  * Derived from FreeBSD's bufring.h
>  * Used as BSD-3 Licensed with permission from Kip Macy.
>  */
>
>
With pleasure, thanks

-- 
Cheers,
Jia

Re: [dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Thomas Monjalon]

22/01/2018 05:41, Jia He:
> Changelog:
> V9: remove the SPDX tag and refine commit logs

Why did you remove the SPDX tag?

You need to fix the licensing issue.
BSD-2-Clause is uncommon in DPDK.

Re: [dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Olivier Matz]

On Mon, Jan 22, 2018 at 08:53:01AM +0100, Thomas Monjalon wrote:
> 22/01/2018 05:41, Jia He:
> > Changelog:
> > V9: remove the SPDX tag and refine commit logs
> 
> Why did you remove the SPDX tag?
> 
> You need to fix the licensing issue.
> BSD-2-Clause is uncommon in DPDK.
> 

Thomas,

The licensing issue existed before Jia's patch. I think we can
solve it separately.

Olivier

Re: [dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Olivier Matz]

On Mon, Jan 22, 2018 at 09:26:49AM +0100, Olivier Matz wrote:
> On Mon, Jan 22, 2018 at 08:53:01AM +0100, Thomas Monjalon wrote:
> > 22/01/2018 05:41, Jia He:
> > > Changelog:
> > > V9: remove the SPDX tag and refine commit logs
> > 
> > Why did you remove the SPDX tag?
> > 
> > You need to fix the licensing issue.
> > BSD-2-Clause is uncommon in DPDK.
> > 
> 
> Thomas,
> 
> The licensing issue existed before Jia's patch. I think we can
> solve it separately.
> 
> Olivier

Sorry, I missed a mail, please ignore this one.

Re: [dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Hemant Agrawal]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, January 22, 2018 1:23 PM
> To: Jia He <hejianet@gmail.com>
> Cc: dev@dpdk.org; Hemant Agrawal <hemant.agrawal@nxp.com>; Jerin
> Jacob <jerin.jacob@caviumnetworks.com>; Jianbo Liu
> <jianbo.liu@arm.com>; Jan Viktorin <viktorin@rehivetech.com>; Olivier
> Matz <olivier.matz@6wind.com>; konstantin.ananyev@intel.com;
> bruce.richardson@intel.com
> Subject: Re: [PATCH v9 0/3] support c11 memory model barrier in
> librte_ring
> Importance: High
> 
> 22/01/2018 05:41, Jia He:
> > Changelog:
> > V9: remove the SPDX tag and refine commit logs
> 
> Why did you remove the SPDX tag?
> 
> You need to fix the licensing issue.
> BSD-2-Clause is uncommon in DPDK.

[Hemant] the license issue existed before this patch. So I have asked Jia to send another patch over this series to fix it in these as well as other files of librte_ring.

Series-Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Re: [dpdk-dev] [PATCH v8 2/3] ring: introduce new header file to include common functions

[Stephen Hemminger]

On Fri, 19 Jan 2018 16:47:36 +0000
Hemant Agrawal <hemant.agrawal@nxp.com> wrote:

> Hi Olivier,
> 
> > On Fri, Jan 19, 2018 at 07:45:30PM +0530, Hemant Agrawal wrote:  
> > > Hi Jia,
> > >
> > > On 1/17/2018 9:33 AM, Jia He wrote:  
> > > > Move the common part of rte_ring.h into rte_ring_generic.h.
> > > > Move the memory barrier part into update_tail().
> > > >
> > > > No functional changes here.
> > > >
> > > > Signed-off-by: Jia He <jia.he@hxt-semitech.com>
> > > > Suggested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > Suggested-by: Ananyev Konstantin <konstantin.ananyev@intel.com>
> > > > Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > > diff --git a/lib/librte_ring/rte_ring_generic.h
> > > > b/lib/librte_ring/rte_ring_generic.h
> > > > new file mode 100644
> > > > index 0000000..01f2cae
> > > > --- /dev/null
> > > > +++ b/lib/librte_ring/rte_ring_generic.h
> > > > @@ -0,0 +1,202 @@
> > > > +/*-
> > > > + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> > > > + *   All rights reserved.
> > > > + *
> > > > + *   SPDX-License-Identifier: BSD-3-Clause  
> > >
> > > The SPDX should be first line. See other files for Intel or NXP.  
> 
> [Hemant] Don't add SPDX to this file. 
>  This file is not BSD-3 licensed.  Please keep the full text as in the original file.


If it can't be BSD-3 licensed, then it needs to be removed. Mixing licenses is a real problem

Re: [dpdk-dev] [PATCH v9 0/3] support c11 memory model barrier in librte_ring

[Thomas Monjalon]

22/01/2018 05:41, Jia He:
> There are 2 model barrier options in librte_ring suggested by Jerin:
> 1. use rte_smp_rmb
> 2. use load_acquire/store_release
> 
> CONFIG_RTE_RING_USE_C11_MEM_MODEL is provided for supporting C11 memory
> model barrier in librte_ring. By default it is "y" on arm64, "n" on any
> other architectures so far.
> 
> Already fuctionally tested on the machines as follows:
> - on X86
> - on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=y
> - on arm64 with CONFIG_RTE_RING_USE_C11_MEM_MODEL=n

Applied, thanks