DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ring: fix c11 memory ordering issue
@ 2018-08-06  1:18 Gavin Hu
  2018-08-06  9:19 ` Thomas Monjalon
  2018-08-07  3:19 ` [dpdk-dev] [PATCH v2] " Gavin Hu
  0 siblings, 2 replies; 131+ messages in thread
From: Gavin Hu @ 2018-08-06  1:18 UTC (permalink / raw)
  To: dev
  Cc: gavin.hu, Honnappa.Nagarahalli, steve.capper, Ola.Liljedahl,
	jerin.jacob, hemant.agrawal, jia.he, stable

1) In update_tail, read ht->tail using __atomic_load.
2) In __rte_ring_move_prod_head, move the __atomic_load_n up and out of
   the do {} while loop as upon failure the old_head will be updated,
   another load is not necessary.
3) Synchronize the load-acquires of prod.tail and cons.tail with store-
   releases of update_tail which releases all ring updates up to the
   value of ht->tail.
4) When calling __atomic_compare_exchange_n, relaxed ordering for both
   success and failure, as multiple threads can work independently on
   the same end of the ring (either enqueue or dequeue) without
   synchronization, not as operating on tail, which has to be finished
   in sequence.

Fixes: 39368ebfc6 ("ring: introduce C11 memory model barrier option")
Cc: stable@dpdk.org

Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ola Liljedahl <Ola.Liljedahl@arm.com>
Signed-off-by: Gavin Hu <gavin.hu@arm.com>
---
 lib/librte_ring/rte_ring_c11_mem.h | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/lib/librte_ring/rte_ring_c11_mem.h b/lib/librte_ring/rte_ring_c11_mem.h
index 94df3c4a6..cfa3be4a7 100644
--- a/lib/librte_ring/rte_ring_c11_mem.h
+++ b/lib/librte_ring/rte_ring_c11_mem.h
@@ -21,7 +21,8 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val,
 	 * we need to wait for them to complete
 	 */
 	if (!single)
-		while (unlikely(ht->tail != old_val))
+		while (unlikely(old_val != __atomic_load_n(&ht->tail,
+						__ATOMIC_RELAXED)))
 			rte_pause();
 
 	__atomic_store_n(&ht->tail, new_val, __ATOMIC_RELEASE);
@@ -60,20 +61,24 @@ __rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp,
 	unsigned int max = n;
 	int success;
 
+	*old_head = __atomic_load_n(&r->prod.head, __ATOMIC_RELAXED);
 	do {
 		/* Reset n to the initial burst count */
 		n = max;
 
-		*old_head = __atomic_load_n(&r->prod.head,
-					__ATOMIC_ACQUIRE);
 
-		/*
-		 *  The subtraction is done between two unsigned 32bits value
+		/* load-acquire synchronize with store-release of ht->tail
+		 * in update_tail.
+		 */
+		const uint32_t cons_tail = __atomic_load_n(&r->cons.tail,
+							__ATOMIC_ACQUIRE);
+
+		/* 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 + r->cons.tail - *old_head);
+		*free_entries = (capacity + cons_tail - *old_head);
 
 		/* check that we have enough room in ring */
 		if (unlikely(n > *free_entries))
@@ -87,9 +92,10 @@ __rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp,
 		if (is_sp)
 			r->prod.head = *new_head, success = 1;
 		else
+			/* on failure, *old_head is updated */
 			success = __atomic_compare_exchange_n(&r->prod.head,
 					old_head, *new_head,
-					0, __ATOMIC_ACQUIRE,
+					/*weak=*/0, __ATOMIC_RELAXED,
 					__ATOMIC_RELAXED);
 	} while (unlikely(success == 0));
 	return n;
@@ -128,18 +134,23 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
 	int success;
 
 	/* move cons.head atomically */
+	*old_head = __atomic_load_n(&r->cons.head, __ATOMIC_RELAXED);
 	do {
 		/* Restore n as it may change every loop */
 		n = max;
-		*old_head = __atomic_load_n(&r->cons.head,
-					__ATOMIC_ACQUIRE);
+
+		/* this load-acquire synchronize with store-release of ht->tail
+		 * in update_tail.
+		 */
+		const uint32_t prod_tail = __atomic_load_n(&r->prod.tail,
+							__ATOMIC_ACQUIRE);
 
 		/* 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 = (r->prod.tail - *old_head);
+		*entries = (prod_tail - *old_head);
 
 		/* Set the actual entries for dequeue */
 		if (n > *entries)
@@ -152,10 +163,11 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc,
 		if (is_sc)
 			r->cons.head = *new_head, success = 1;
 		else
+			/* on failure, *old_head will be updated */
 			success = __atomic_compare_exchange_n(&r->cons.head,
-							old_head, *new_head,
-							0, __ATOMIC_ACQUIRE,
-							__ATOMIC_RELAXED);
+						old_head, *new_head,
+						/*weak=*/0, __ATOMIC_RELAXED,
+						__ATOMIC_RELAXED);
 	} while (unlikely(success == 0));
 	return n;
 }
-- 
2.11.0

^ permalink raw reply	[flat|nested] 131+ messages in thread

end of thread, other threads:[~2018-11-06 11:03 UTC | newest]

Thread overview: 131+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-06  1:18 [dpdk-dev] [PATCH] ring: fix c11 memory ordering issue Gavin Hu
2018-08-06  9:19 ` Thomas Monjalon
2018-08-08  1:39   ` Gavin Hu
2018-08-07  3:19 ` [dpdk-dev] [PATCH v2] " Gavin Hu
2018-08-07  5:56   ` He, Jia
2018-08-07  7:56     ` Gavin Hu
2018-08-08  3:07       ` Jerin Jacob
2018-08-08  7:23         ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2018-09-17  7:47   ` [dpdk-dev] [PATCH v3 1/3] app/testpmd: show errno along with flow API errors Gavin Hu
2018-09-17  7:47     ` [dpdk-dev] [PATCH v3 2/3] net/i40e: remove invalid comment Gavin Hu
2018-09-17  8:25       ` Gavin Hu (Arm Technology China)
2018-09-17  7:47     ` [dpdk-dev] [PATCH v3 3/3] doc: add cross compile part for sample applications Gavin Hu
2018-09-17  9:48       ` Jerin Jacob
2018-09-17 10:28         ` Gavin Hu (Arm Technology China)
2018-09-17 10:34           ` Jerin Jacob
2018-09-17 10:55             ` Gavin Hu (Arm Technology China)
2018-09-17 10:49       ` [dpdk-dev] [PATCH v4] " Gavin Hu
2018-09-17 10:53         ` [dpdk-dev] [PATCH v5] " Gavin Hu
2018-09-18 11:00           ` Jerin Jacob
2018-09-19  0:33           ` [dpdk-dev] [PATCH v6] " Gavin Hu
2018-09-17  8:11     ` [dpdk-dev] [PATCH v4 1/4] bus/fslmc: fix undefined reference of memsegs Gavin Hu
2018-09-17  8:11       ` [dpdk-dev] [PATCH v4 2/4] ring: read tail using atomic load Gavin Hu
2018-09-20  6:41         ` Jerin Jacob
2018-09-25  9:26           ` Gavin Hu (Arm Technology China)
2018-09-17  8:11       ` [dpdk-dev] [PATCH v4 3/4] ring: synchronize the load and store of the tail Gavin Hu
2018-09-17  8:11       ` [dpdk-dev] [PATCH v4 4/4] ring: move the atomic load of head above the loop Gavin Hu
2018-10-27 14:21         ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2018-09-17  8:17   ` [dpdk-dev] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu
2018-09-17  8:17     ` [dpdk-dev] [PATCH v3 2/3] ring: synchronize the load and store of the tail Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26  9:59         ` Justin He
2018-09-29 10:57       ` Jerin Jacob
2018-10-17  6:29       ` [dpdk-dev] [PATCH 1/2] " Gavin Hu
2018-10-17  6:29         ` [dpdk-dev] [PATCH 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-17  6:35         ` [dpdk-dev] [PATCH 1/2] ring: synchronize the load and store of the tail Gavin Hu (Arm Technology China)
2018-10-27 14:39           ` Thomas Monjalon
2018-10-27 15:00             ` Jerin Jacob
2018-10-27 15:13               ` Thomas Monjalon
2018-10-27 15:34                 ` Jerin Jacob
2018-10-27 15:48                   ` Thomas Monjalon
2018-10-29  2:51                   ` Gavin Hu (Arm Technology China)
2018-10-29  2:57                   ` Gavin Hu (Arm Technology China)
2018-10-29 10:16                     ` Jerin Jacob
2018-10-29 10:47                       ` Thomas Monjalon
2018-10-29 11:10                         ` Jerin Jacob
2018-11-03 20:12                 ` Mattias Rönnblom
2018-11-05 21:51                   ` Honnappa Nagarahalli
2018-11-06 11:03                     ` Mattias Rönnblom
2018-10-31  3:35         ` [dpdk-dev] [PATCH v2 0/2] rte ring c11 bug fix and optimization Gavin Hu
2018-10-31 10:26           ` [dpdk-dev] [PATCH v3 0/2] ring library with c11 memory model " Gavin Hu
2018-10-31 16:58             ` Thomas Monjalon
2018-11-01  9:53             ` [dpdk-dev] [PATCH v4 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-01  9:53             ` [dpdk-dev] [PATCH v4 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-01 17:26               ` Stephen Hemminger
2018-11-02  0:53                 ` Gavin Hu (Arm Technology China)
2018-11-02  4:30                   ` Honnappa Nagarahalli
2018-11-02  7:15                     ` Gavin Hu (Arm Technology China)
2018-11-02  9:36                       ` Thomas Monjalon
2018-11-02 11:23                         ` Gavin Hu (Arm Technology China)
2018-10-31 10:26           ` [dpdk-dev] [PATCH v3 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31 22:07             ` Stephen Hemminger
2018-11-01  9:56               ` Gavin Hu (Arm Technology China)
2018-10-31 10:26           ` [dpdk-dev] [PATCH v3 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  3:35         ` [dpdk-dev] [PATCH v2 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-10-31  3:35         ` [dpdk-dev] [PATCH v2 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-10-31  9:36           ` Thomas Monjalon
2018-10-31 10:27             ` Gavin Hu (Arm Technology China)
2018-11-01  9:53         ` [dpdk-dev] [PATCH v4 0/2] ring library with c11 memory model bug fix and optimization Gavin Hu
2018-11-02 11:21           ` [dpdk-dev] [PATCH v5 " Gavin Hu
2018-11-02 11:21           ` [dpdk-dev] [PATCH v5 1/2] ring: synchronize the load and store of the tail Gavin Hu
2018-11-05  9:30             ` Olivier Matz
2018-11-02 11:21           ` [dpdk-dev] [PATCH v5 2/2] ring: move the atomic load of head above the loop Gavin Hu
2018-11-02 11:43             ` Bruce Richardson
2018-11-03  1:19               ` Gavin Hu (Arm Technology China)
2018-11-03  9:34                 ` Honnappa Nagarahalli
2018-11-05 13:17                   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2018-11-05 13:41                     ` Jerin Jacob
2018-11-05  9:44                 ` [dpdk-dev] " Olivier Matz
2018-11-05 13:36                   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2018-09-17  8:17     ` [dpdk-dev] [PATCH v3 3/3] " Gavin Hu
2018-09-26  9:29       ` Gavin Hu (Arm Technology China)
2018-09-26 10:06         ` Justin He
2018-09-29  7:19           ` Stephen Hemminger
2018-09-29 10:59       ` Jerin Jacob
2018-09-26  9:29     ` [dpdk-dev] [PATCH v3 1/3] ring: read tail using atomic load Gavin Hu (Arm Technology China)
2018-09-26 10:09       ` Justin He
2018-09-29 10:48     ` Jerin Jacob
2018-10-05  0:47       ` Gavin Hu (Arm Technology China)
2018-10-05  8:21         ` Ananyev, Konstantin
2018-10-05 11:15           ` Ola Liljedahl
2018-10-05 11:36             ` Ola Liljedahl
2018-10-05 13:44               ` Ananyev, Konstantin
2018-10-05 14:21                 ` Ola Liljedahl
2018-10-05 15:11                 ` Honnappa Nagarahalli
2018-10-05 17:07                   ` Jerin Jacob
2018-10-05 18:05                     ` Ola Liljedahl
2018-10-05 20:06                       ` Honnappa Nagarahalli
2018-10-05 20:17                         ` Ola Liljedahl
2018-10-05 20:29                           ` Honnappa Nagarahalli
2018-10-05 20:34                             ` Ola Liljedahl
2018-10-06  7:41                               ` Jerin Jacob
2018-10-06 19:44                                 ` Ola Liljedahl
2018-10-06 19:59                                   ` Ola Liljedahl
2018-10-07  4:02                                   ` Jerin Jacob
2018-10-07 20:11                                     ` Ola Liljedahl
2018-10-07 20:44                                     ` Ola Liljedahl
2018-10-08  6:06                                       ` Jerin Jacob
2018-10-08  9:22                                         ` Ola Liljedahl
2018-10-08 10:00                                           ` Jerin Jacob
2018-10-08 10:25                                             ` Ola Liljedahl
2018-10-08 10:33                                               ` Gavin Hu (Arm Technology China)
2018-10-08 10:39                                                 ` Ola Liljedahl
2018-10-08 10:41                                                   ` Gavin Hu (Arm Technology China)
2018-10-08 10:49                                                 ` Jerin Jacob
2018-10-10  6:28                                                   ` Gavin Hu (Arm Technology China)
2018-10-10 19:26                                                     ` Honnappa Nagarahalli
2018-10-08 10:46                                               ` Jerin Jacob
2018-10-08 11:21                                                 ` Ola Liljedahl
2018-10-08 11:50                                                   ` Jerin Jacob
2018-10-08 11:59                                                     ` Ola Liljedahl
2018-10-08 12:05                                                       ` Jerin Jacob
2018-10-08 12:20                                                         ` Jerin Jacob
2018-10-08 12:30                                                           ` Ola Liljedahl
2018-10-09  8:53                                                             ` Olivier Matz
2018-10-09  3:16                                             ` Honnappa Nagarahalli
2018-10-08 14:43                                           ` Bruce Richardson
2018-10-08 14:46                                             ` Ola Liljedahl
2018-10-08 15:45                                               ` Ola Liljedahl
2018-10-08  5:27                               ` Honnappa Nagarahalli
2018-10-08 10:01                                 ` Ola Liljedahl
2018-10-27 14:17     ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).