[RFC,4/5] eal: ensure memory operations are visible to main

Message ID 20210224212018.17576-5-honnappa.nagarahalli@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series Use correct memory ordering in eal functions |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Honnappa Nagarahalli Feb. 24, 2021, 9:20 p.m. UTC
  Ensure that the memory operations in worker thread, that happen
before it returns the status of the assigned function, are
visible to the main thread.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_eal/common/eal_common_launch.c |  8 ++++----
 lib/librte_eal/freebsd/eal_thread.c       | 10 ++++++++--
 lib/librte_eal/linux/eal_thread.c         | 17 ++++++++++++-----
 lib/librte_eal/windows/eal_thread.c       |  9 +++++++--
 4 files changed, 31 insertions(+), 13 deletions(-)
  

Patch

diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
index 78fd94026..9cc71801a 100644
--- a/lib/librte_eal/common/eal_common_launch.c
+++ b/lib/librte_eal/common/eal_common_launch.c
@@ -23,14 +23,14 @@ 
 int
 rte_eal_wait_lcore(unsigned worker_id)
 {
-	if (lcore_config[worker_id].state == WAIT)
+	if (__atomic_load_n(&lcore_config[worker_id].state,
+					__ATOMIC_ACQUIRE) == WAIT)
 		return 0;
 
-	while (lcore_config[worker_id].state != WAIT)
+	while (__atomic_load_n(&lcore_config[worker_id].state,
+					__ATOMIC_ACQUIRE) != WAIT)
 		rte_pause();
 
-	rte_rmb();
-
 	return lcore_config[worker_id].ret;
 }
 
diff --git a/lib/librte_eal/freebsd/eal_thread.c b/lib/librte_eal/freebsd/eal_thread.c
index 6d6f1e2fd..58c8502de 100644
--- a/lib/librte_eal/freebsd/eal_thread.c
+++ b/lib/librte_eal/freebsd/eal_thread.c
@@ -139,8 +139,14 @@  eal_thread_loop(__rte_unused void *arg)
 		lcore_config[lcore_id].ret = ret;
 		lcore_config[lcore_id].f = NULL;
 		lcore_config[lcore_id].arg = NULL;
-		rte_wmb();
-		lcore_config[lcore_id].state = WAIT;
+
+		/* Store the state with release order to ensure that
+		 * the memory operations from the worker thread
+		 * are completed before the state is updated.
+		 * Use 'state' as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
+					__ATOMIC_RELEASE);
 	}
 
 	/* never reached */
diff --git a/lib/librte_eal/linux/eal_thread.c b/lib/librte_eal/linux/eal_thread.c
index 7b9463df3..eab6fa652 100644
--- a/lib/librte_eal/linux/eal_thread.c
+++ b/lib/librte_eal/linux/eal_thread.c
@@ -39,13 +39,14 @@  rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id)
 	int w2m = lcore_config[worker_id].pipe_worker2main[0];
 	int rc = -EBUSY;
 
-	if (lcore_config[worker_id].state != WAIT)
+	if (__atomic_load_n(&lcore_config[worker_id].state,
+					__ATOMIC_ACQUIRE) != WAIT)
 		goto finish;
 
 	lcore_config[worker_id].arg = arg;
 	/* Ensure that all the memory operations are completed
 	 * before the worker thread starts running the function.
-	 * Use worker thread function as the guard variable.
+	 * Use worker thread function pointer as the guard variable.
 	 */
 	__atomic_store_n(&lcore_config[worker_id].f, f, __ATOMIC_RELEASE);
 
@@ -115,7 +116,8 @@  eal_thread_loop(__rte_unused void *arg)
 		if (n <= 0)
 			rte_panic("cannot read on configuration pipe\n");
 
-		lcore_config[lcore_id].state = RUNNING;
+		__atomic_store_n(&lcore_config[lcore_id].state, RUNNING,
+					__ATOMIC_RELEASE);
 
 		/* send ack */
 		n = 0;
@@ -139,9 +141,14 @@  eal_thread_loop(__rte_unused void *arg)
 		lcore_config[lcore_id].ret = ret;
 		lcore_config[lcore_id].f = NULL;
 		lcore_config[lcore_id].arg = NULL;
-		rte_wmb();
 
-		lcore_config[lcore_id].state = WAIT;
+		/* Store the state with release order to ensure that
+		 * the memory operations from the worker thread
+		 * are completed before the state is updated.
+		 * Use 'state' as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
+					__ATOMIC_RELEASE);
 	}
 
 	/* never reached */
diff --git a/lib/librte_eal/windows/eal_thread.c b/lib/librte_eal/windows/eal_thread.c
index 35d059a30..fb1ec4b4f 100644
--- a/lib/librte_eal/windows/eal_thread.c
+++ b/lib/librte_eal/windows/eal_thread.c
@@ -123,9 +123,14 @@  eal_thread_loop(void *arg __rte_unused)
 		lcore_config[lcore_id].ret = ret;
 		lcore_config[lcore_id].f = NULL;
 		lcore_config[lcore_id].arg = NULL;
-		rte_wmb();
 
-		lcore_config[lcore_id].state = WAIT;
+		/* Store the state with release order to ensure that
+		 * the memory operations from the worker thread
+		 * are completed before the state is updated.
+		 * Use 'state' as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, WAIT,
+					__ATOMIC_RELEASE);
 	}
 }