diff mbox series

[v2,5/6] eal: ensure memory operations are visible to main

Message ID 20210909231312.2572006-6-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 Sept. 9, 2021, 11:13 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>
Reviewed-by: Feifei Wang <feifei.wang@arm.com>
---
 lib/eal/common/eal_common_launch.c |  2 --
 lib/eal/freebsd/eal_thread.c       | 22 ++++++++++++++++++----
 lib/eal/linux/eal_thread.c         | 23 ++++++++++++++++++-----
 lib/eal/windows/eal_thread.c       | 21 +++++++++++++++++----
 4 files changed, 53 insertions(+), 15 deletions(-)
diff mbox series

Patch

diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c
index 4bc842417a..e95dadffb3 100644
--- a/lib/eal/common/eal_common_launch.c
+++ b/lib/eal/common/eal_common_launch.c
@@ -27,8 +27,6 @@  rte_eal_wait_lcore(unsigned worker_id)
 					__ATOMIC_ACQUIRE) != WAIT)
 		rte_pause();
 
-	rte_rmb();
-
 	return lcore_config[worker_id].ret;
 }
 
diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c
index e5e3edf79b..3b18030d73 100644
--- a/lib/eal/freebsd/eal_thread.c
+++ b/lib/eal/freebsd/eal_thread.c
@@ -39,7 +39,11 @@  rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id)
 	int w2m = lcore_config[worker_id].pipe_worker2main[0];
 	int rc = -EBUSY;
 
-	if (lcore_config[worker_id].state != WAIT)
+	/* Check if the worker is in 'WAIT' state. Use acquire order
+	 * since 'state' variable is used as the guard variable.
+	 */
+	if (__atomic_load_n(&lcore_config[worker_id].state,
+					__ATOMIC_ACQUIRE) != WAIT)
 		goto finish;
 
 	lcore_config[worker_id].arg = arg;
@@ -115,7 +119,11 @@  eal_thread_loop(__rte_unused void *arg)
 		if (n <= 0)
 			rte_panic("cannot read on configuration pipe\n");
 
-		lcore_config[lcore_id].state = RUNNING;
+		/* Set the state to 'RUNNING'. Use release order
+		 * since 'state' variable is used as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, RUNNING,
+					__ATOMIC_RELEASE);
 
 		/* send ack */
 		n = 0;
@@ -139,8 +147,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/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c
index c79e09c9ad..c7f0f9b2f7 100644
--- a/lib/eal/linux/eal_thread.c
+++ b/lib/eal/linux/eal_thread.c
@@ -39,13 +39,17 @@  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)
+	/* Check if the worker is in 'WAIT' state. Use acquire order
+	 * since 'state' variable is used as the guard variable.
+	 */
+	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 +119,11 @@  eal_thread_loop(__rte_unused void *arg)
 		if (n <= 0)
 			rte_panic("cannot read on configuration pipe\n");
 
-		lcore_config[lcore_id].state = RUNNING;
+		/* Set the state to 'RUNNING'. Use release order
+		 * since 'state' variable is used as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, RUNNING,
+					__ATOMIC_RELEASE);
 
 		/* send ack */
 		n = 0;
@@ -139,9 +147,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/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c
index 977911905e..54fa93fa62 100644
--- a/lib/eal/windows/eal_thread.c
+++ b/lib/eal/windows/eal_thread.c
@@ -29,7 +29,11 @@  rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id)
 	int m2w = lcore_config[worker_id].pipe_main2worker[1];
 	int w2m = lcore_config[worker_id].pipe_worker2main[0];
 
-	if (lcore_config[worker_id].state != WAIT)
+	/* Check if the worker is in 'WAIT' state. Use acquire order
+	 * since 'state' variable is used as the guard variable.
+	 */
+	if (__atomic_load_n(&lcore_config[worker_id].state,
+					__ATOMIC_ACQUIRE) != WAIT)
 		return -EBUSY;
 
 	lcore_config[worker_id].arg = arg;
@@ -99,7 +103,11 @@  eal_thread_loop(void *arg __rte_unused)
 		if (n <= 0)
 			rte_panic("cannot read on configuration pipe\n");
 
-		lcore_config[lcore_id].state = RUNNING;
+		/* Set the state to 'RUNNING'. Use release order
+		 * since 'state' variable is used as the guard variable.
+		 */
+		__atomic_store_n(&lcore_config[lcore_id].state, RUNNING,
+					__ATOMIC_RELEASE);
 
 		/* send ack */
 		n = 0;
@@ -123,9 +131,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);
 	}
 }