[v3,2/4] eal: lcore state FINISHED is not required

Message ID 20211025045237.19878-3-honnappa.nagarahalli@arm.com (mailing list archive)
State Accepted, 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 Oct. 25, 2021, 4:52 a.m. UTC
  FINISHED state seems to be used to indicate that the worker's update
of the 'state' is not visible to other threads. There seems to be no
requirement to have such a state.

Since the FINISHED state is removed, the API rte_eal_wait_lcore
is updated to always return the status of the last function that
ran in the worker core.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
Reviewed-by: Feifei Wang <feifei.wang@arm.com>
---
 drivers/event/dpaa2/dpaa2_eventdev_selftest.c |  2 +-
 drivers/event/octeontx/ssovf_evdev_selftest.c |  2 +-
 drivers/event/sw/sw_evdev_selftest.c          |  4 ++--
 examples/l2fwd-keepalive/main.c               |  3 +--
 lib/eal/common/eal_common_launch.c            |  7 ++-----
 lib/eal/freebsd/eal_thread.c                  |  4 ++--
 lib/eal/include/rte_launch.h                  | 21 +++++++++----------
 lib/eal/include/rte_service.h                 |  4 +---
 lib/eal/linux/eal_thread.c                    | 10 ++-------
 lib/eal/windows/eal_thread.c                  | 10 ++-------
 10 files changed, 24 insertions(+), 43 deletions(-)
  

Patch

diff --git a/drivers/event/dpaa2/dpaa2_eventdev_selftest.c b/drivers/event/dpaa2/dpaa2_eventdev_selftest.c
index cd7311a94d..bbbd20951f 100644
--- a/drivers/event/dpaa2/dpaa2_eventdev_selftest.c
+++ b/drivers/event/dpaa2/dpaa2_eventdev_selftest.c
@@ -468,7 +468,7 @@  wait_workers_to_join(int lcore, const rte_atomic32_t *count)
 	RTE_SET_USED(count);
 
 	print_cycles = cycles = rte_get_timer_cycles();
-	while (rte_eal_get_lcore_state(lcore) != FINISHED) {
+	while (rte_eal_get_lcore_state(lcore) != WAIT) {
 		uint64_t new_cycles = rte_get_timer_cycles();
 
 		if (new_cycles - print_cycles > rte_get_timer_hz()) {
diff --git a/drivers/event/octeontx/ssovf_evdev_selftest.c b/drivers/event/octeontx/ssovf_evdev_selftest.c
index 528f99dd84..d7b0d22111 100644
--- a/drivers/event/octeontx/ssovf_evdev_selftest.c
+++ b/drivers/event/octeontx/ssovf_evdev_selftest.c
@@ -579,7 +579,7 @@  wait_workers_to_join(int lcore, const rte_atomic32_t *count)
 	RTE_SET_USED(count);
 
 	print_cycles = cycles = rte_get_timer_cycles();
-	while (rte_eal_get_lcore_state(lcore) != FINISHED) {
+	while (rte_eal_get_lcore_state(lcore) != WAIT) {
 		uint64_t new_cycles = rte_get_timer_cycles();
 
 		if (new_cycles - print_cycles > rte_get_timer_hz()) {
diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c
index d53e903129..9768d3a0c7 100644
--- a/drivers/event/sw/sw_evdev_selftest.c
+++ b/drivers/event/sw/sw_evdev_selftest.c
@@ -3140,8 +3140,8 @@  worker_loopback(struct test *t, uint8_t disable_implicit_release)
 	rte_eal_remote_launch(worker_loopback_worker_fn, t, w_lcore);
 
 	print_cycles = cycles = rte_get_timer_cycles();
-	while (rte_eal_get_lcore_state(p_lcore) != FINISHED ||
-			rte_eal_get_lcore_state(w_lcore) != FINISHED) {
+	while (rte_eal_get_lcore_state(p_lcore) != WAIT ||
+			rte_eal_get_lcore_state(w_lcore) != WAIT) {
 
 		rte_service_run_iter_on_app_lcore(t->service_id, 1);
 
diff --git a/examples/l2fwd-keepalive/main.c b/examples/l2fwd-keepalive/main.c
index 4e1a17cfe4..b4f9d7334b 100644
--- a/examples/l2fwd-keepalive/main.c
+++ b/examples/l2fwd-keepalive/main.c
@@ -507,8 +507,7 @@  dead_core(__rte_unused void *ptr_data, const int id_core)
 	if (terminate_signal_received)
 		return;
 	printf("Dead core %i - restarting..\n", id_core);
-	if (rte_eal_get_lcore_state(id_core) == FINISHED) {
-		rte_eal_wait_lcore(id_core);
+	if (rte_eal_get_lcore_state(id_core) == WAIT) {
 		rte_eal_remote_launch(l2fwd_launch_one_lcore, NULL, id_core);
 	} else {
 		printf("..false positive!\n");
diff --git a/lib/eal/common/eal_common_launch.c b/lib/eal/common/eal_common_launch.c
index 34f854ad80..78fd940267 100644
--- a/lib/eal/common/eal_common_launch.c
+++ b/lib/eal/common/eal_common_launch.c
@@ -26,14 +26,11 @@  rte_eal_wait_lcore(unsigned worker_id)
 	if (lcore_config[worker_id].state == WAIT)
 		return 0;
 
-	while (lcore_config[worker_id].state != WAIT &&
-	       lcore_config[worker_id].state != FINISHED)
+	while (lcore_config[worker_id].state != WAIT)
 		rte_pause();
 
 	rte_rmb();
 
-	/* we are in finished state, go to wait state */
-	lcore_config[worker_id].state = WAIT;
 	return lcore_config[worker_id].ret;
 }
 
@@ -62,7 +59,7 @@  rte_eal_mp_remote_launch(int (*f)(void *), void *arg,
 
 	if (call_main == CALL_MAIN) {
 		lcore_config[main_lcore].ret = f(arg);
-		lcore_config[main_lcore].state = FINISHED;
+		lcore_config[main_lcore].state = WAIT;
 	}
 
 	return 0;
diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c
index bbc3a8e985..ce314c1dd4 100644
--- a/lib/eal/freebsd/eal_thread.c
+++ b/lib/eal/freebsd/eal_thread.c
@@ -28,7 +28,7 @@ 
 /*
  * Send a message to a worker lcore identified by worker_id to call a
  * function f with argument arg. Once the execution is done, the
- * remote lcore switch in FINISHED state.
+ * remote lcore switches to WAIT state.
  */
 int
 rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id)
@@ -129,7 +129,7 @@  eal_thread_loop(__rte_unused void *arg)
 		lcore_config[lcore_id].f = NULL;
 		lcore_config[lcore_id].arg = NULL;
 		rte_wmb();
-		lcore_config[lcore_id].state = FINISHED;
+		lcore_config[lcore_id].state = WAIT;
 	}
 
 	/* never reached */
diff --git a/lib/eal/include/rte_launch.h b/lib/eal/include/rte_launch.h
index 22a901ce62..f2d386e6e2 100644
--- a/lib/eal/include/rte_launch.h
+++ b/lib/eal/include/rte_launch.h
@@ -19,9 +19,10 @@  extern "C" {
  * State of an lcore.
  */
 enum rte_lcore_state_t {
-	WAIT,       /**< waiting a new command */
-	RUNNING,    /**< executing command */
-	FINISHED,   /**< command executed */
+	WAIT,
+	/**< waiting for new command */
+	RUNNING,
+	/**< executing command */
 };
 
 /**
@@ -41,7 +42,7 @@  typedef int (lcore_function_t)(void *);
  *
  * When the remote lcore receives the message, it switches to
  * the RUNNING state, then calls the function f with argument arg. Once the
- * execution is done, the remote lcore switches to a FINISHED state and
+ * execution is done, the remote lcore switches to WAIT state and
  * the return value of f is stored in a local variable to be read using
  * rte_eal_wait_lcore().
  *
@@ -118,18 +119,16 @@  enum rte_lcore_state_t rte_eal_get_lcore_state(unsigned int worker_id);
  *
  * To be executed on the MAIN lcore only.
  *
- * If the worker lcore identified by the worker_id is in a FINISHED state,
- * switch to the WAIT state. If the lcore is in RUNNING state, wait until
- * the lcore finishes its job and moves to the FINISHED state.
+ * If the lcore identified by the worker_id is in RUNNING state, wait until
+ * the lcore finishes its job and moves to the WAIT state.
  *
  * @param worker_id
  *   The identifier of the lcore.
  * @return
- *   - 0: If the lcore identified by the worker_id is in a WAIT state.
+ *   - 0: If the remote launch function was never called on the lcore
+ *     identified by the worker_id.
  *   - The value that was returned by the previous remote launch
- *     function call if the lcore identified by the worker_id was in a
- *     FINISHED or RUNNING state. In this case, it changes the state
- *     of the lcore to WAIT.
+ *     function call.
  */
 int rte_eal_wait_lcore(unsigned worker_id);
 
diff --git a/lib/eal/include/rte_service.h b/lib/eal/include/rte_service.h
index c7d037d862..646533e871 100644
--- a/lib/eal/include/rte_service.h
+++ b/lib/eal/include/rte_service.h
@@ -321,9 +321,7 @@  int32_t rte_service_lcore_count(void);
  * from duty, just unmaps all services / cores, and stops() the service cores.
  * The runstate of services is not modified.
  *
- * The cores that are stopped with this call, are in FINISHED state and
- * the application must take care of bringing them back to a launchable state:
- * e.g. call *rte_eal_lcore_wait* on the lcore_id.
+ * The cores that are stopped with this call, are in WAIT state.
  *
  * @retval 0 Success
  */
diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c
index 8f3c0dafd6..e2c9e5a3f7 100644
--- a/lib/eal/linux/eal_thread.c
+++ b/lib/eal/linux/eal_thread.c
@@ -28,7 +28,7 @@ 
 /*
  * Send a message to a worker lcore identified by worker_id to call a
  * function f with argument arg. Once the execution is done, the
- * remote lcore switch in FINISHED state.
+ * remote lcore switches to WAIT state.
  */
 int
 rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id)
@@ -130,13 +130,7 @@  eal_thread_loop(__rte_unused void *arg)
 		lcore_config[lcore_id].arg = NULL;
 		rte_wmb();
 
-		/* when a service core returns, it should go directly to WAIT
-		 * state, because the application will not lcore_wait() for it.
-		 */
-		if (lcore_config[lcore_id].core_role == ROLE_SERVICE)
-			lcore_config[lcore_id].state = WAIT;
-		else
-			lcore_config[lcore_id].state = FINISHED;
+		lcore_config[lcore_id].state = WAIT;
 	}
 
 	/* never reached */
diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c
index df1df5d02c..0028123c95 100644
--- a/lib/eal/windows/eal_thread.c
+++ b/lib/eal/windows/eal_thread.c
@@ -19,7 +19,7 @@ 
 /*
  * Send a message to a worker lcore identified by worker_id to call a
  * function f with argument arg. Once the execution is done, the
- * remote lcore switch in FINISHED state.
+ * remote lcore switches to WAIT state.
  */
 int
 rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id)
@@ -114,13 +114,7 @@  eal_thread_loop(void *arg __rte_unused)
 		lcore_config[lcore_id].arg = NULL;
 		rte_wmb();
 
-		/* when a service core returns, it should go directly to WAIT
-		 * state, because the application will not lcore_wait() for it.
-		 */
-		if (lcore_config[lcore_id].core_role == ROLE_SERVICE)
-			lcore_config[lcore_id].state = WAIT;
-		else
-			lcore_config[lcore_id].state = FINISHED;
+		lcore_config[lcore_id].state = WAIT;
 	}
 }