[v2,2/2] l3fwd-power: implement proper shutdown handling

Message ID f2aa59260da30f5bc1e9fb7b22cbb70eb39f02db.1587405227.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] l3fwd-power: exit if initializing power library failed |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Burakov, Anatoly April 20, 2020, 5:56 p.m. UTC
  Currently, shutdown for l3fwd-power application is all over the place
and may or may not happen either in the signal handler or in the main()
function. Fix this so that the signal handler will only set the exit
variable, thereby allowing all of the loops to end properly and proceed
to deinitialize everything.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
---

Notes:
    v2:
    - rebase on top of latest master

 examples/l3fwd-power/main.c | 74 ++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 34 deletions(-)
  

Comments

Thomas Monjalon April 26, 2020, 9:52 p.m. UTC | #1
20/04/2020 19:56, Anatoly Burakov:
> Currently, shutdown for l3fwd-power application is all over the place
> and may or may not happen either in the signal handler or in the main()
> function. Fix this so that the signal handler will only set the exit
> variable, thereby allowing all of the loops to end properly and proceed
> to deinitialize everything.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: David Hunt <david.hunt@intel.com>
> Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>

Applied, thanks
  
Xie, WeiX April 28, 2020, 11:19 a.m. UTC | #2
Tested-by:  Xie,WeiX < weix.xie@intel.com>

Regards,
Xie Wei


-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Anatoly Burakov
Sent: Tuesday, April 21, 2020 1:57 AM
To: dev@dpdk.org
Cc: Hunt, David <david.hunt@intel.com>; Pattan, Reshma <reshma.pattan@intel.com>
Subject: [dpdk-dev] [PATCH v2 2/2] l3fwd-power: implement proper shutdown handling

Currently, shutdown for l3fwd-power application is all over the place and may or may not happen either in the signal handler or in the main() function. Fix this so that the signal handler will only set the exit variable, thereby allowing all of the loops to end properly and proceed to deinitialize everything.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: David Hunt <david.hunt@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
---

Notes:
    v2:
    - rebase on top of latest master

 examples/l3fwd-power/main.c | 74 ++++++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c index be50ec049..293b3da4a 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -422,41 +422,10 @@ static int is_done(void)  static void  signal_exit_now(int sigtype)  {
-	unsigned lcore_id;
-	unsigned int portid;
-	int ret;
-
-	if (sigtype == SIGINT) {
-		if (app_mode == APP_MODE_EMPTY_POLL ||
-				app_mode == APP_MODE_TELEMETRY)
-			quit_signal = true;
 
+	if (sigtype == SIGINT)
+		quit_signal = true;
 
-		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-			if (rte_lcore_is_enabled(lcore_id) == 0)
-				continue;
-
-			/* init power management library */
-			ret = rte_power_exit(lcore_id);
-			if (ret)
-				rte_exit(EXIT_FAILURE, "Power management "
-					"library de-initialization failed on "
-							"core%u\n", lcore_id);
-		}
-
-		if (app_mode != APP_MODE_EMPTY_POLL) {
-			RTE_ETH_FOREACH_DEV(portid) {
-				if ((enabled_port_mask & (1 << portid)) == 0)
-					continue;
-
-				rte_eth_dev_stop(portid);
-				rte_eth_dev_close(portid);
-			}
-		}
-	}
-
-	if (app_mode != APP_MODE_EMPTY_POLL)
-		rte_exit(EXIT_SUCCESS, "User forced exit\n");
 }
 
 /*  Freqency scale down timer callback */ @@ -1196,7 +1165,7 @@ main_loop(__rte_unused void *dummy)
 	else
 		RTE_LOG(INFO, L3FWD_POWER, "RX interrupt won't enable.\n");
 
-	while (1) {
+	while (!is_done()) {
 		stats[lcore_id].nb_iteration_looped++;
 
 		cur_tsc = rte_rdtsc();
@@ -1343,6 +1312,8 @@ main_loop(__rte_unused void *dummy)
 			stats[lcore_id].sleep_time += lcore_idle_hint;
 		}
 	}
+
+	return 0;
 }
 
 static int
@@ -2080,6 +2051,26 @@ init_power_library(void)
 	}
 	return ret;
 }
+
+static int
+deinit_power_library(void)
+{
+	unsigned int lcore_id;
+	int ret = 0;
+
+	RTE_LCORE_FOREACH(lcore_id) {
+		/* deinit power management library */
+		ret = rte_power_exit(lcore_id);
+		if (ret) {
+			RTE_LOG(ERR, POWER,
+				"Library deinitialization failed on core %u\n",
+				lcore_id);
+			return ret;
+		}
+	}
+	return ret;
+}
+
 static void
 update_telemetry(__rte_unused struct rte_timer *tim,
 		__rte_unused void *arg)
@@ -2530,8 +2521,23 @@ main(int argc, char **argv)
 			return -1;
 	}
 
+	RTE_ETH_FOREACH_DEV(portid)
+	{
+		if ((enabled_port_mask & (1 << portid)) == 0)
+			continue;
+
+		rte_eth_dev_stop(portid);
+		rte_eth_dev_close(portid);
+	}
+
 	if (app_mode == APP_MODE_EMPTY_POLL)
 		rte_power_empty_poll_stat_free();
 
+	if (app_mode != APP_MODE_TELEMETRY && deinit_power_library())
+		rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
+
+	if (rte_eal_cleanup() < 0)
+		RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n");
+
 	return 0;
 }
--
2.17.1
  

Patch

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index be50ec049..293b3da4a 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -422,41 +422,10 @@  static int is_done(void)
 static void
 signal_exit_now(int sigtype)
 {
-	unsigned lcore_id;
-	unsigned int portid;
-	int ret;
-
-	if (sigtype == SIGINT) {
-		if (app_mode == APP_MODE_EMPTY_POLL ||
-				app_mode == APP_MODE_TELEMETRY)
-			quit_signal = true;
 
+	if (sigtype == SIGINT)
+		quit_signal = true;
 
-		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-			if (rte_lcore_is_enabled(lcore_id) == 0)
-				continue;
-
-			/* init power management library */
-			ret = rte_power_exit(lcore_id);
-			if (ret)
-				rte_exit(EXIT_FAILURE, "Power management "
-					"library de-initialization failed on "
-							"core%u\n", lcore_id);
-		}
-
-		if (app_mode != APP_MODE_EMPTY_POLL) {
-			RTE_ETH_FOREACH_DEV(portid) {
-				if ((enabled_port_mask & (1 << portid)) == 0)
-					continue;
-
-				rte_eth_dev_stop(portid);
-				rte_eth_dev_close(portid);
-			}
-		}
-	}
-
-	if (app_mode != APP_MODE_EMPTY_POLL)
-		rte_exit(EXIT_SUCCESS, "User forced exit\n");
 }
 
 /*  Freqency scale down timer callback */
@@ -1196,7 +1165,7 @@  main_loop(__rte_unused void *dummy)
 	else
 		RTE_LOG(INFO, L3FWD_POWER, "RX interrupt won't enable.\n");
 
-	while (1) {
+	while (!is_done()) {
 		stats[lcore_id].nb_iteration_looped++;
 
 		cur_tsc = rte_rdtsc();
@@ -1343,6 +1312,8 @@  main_loop(__rte_unused void *dummy)
 			stats[lcore_id].sleep_time += lcore_idle_hint;
 		}
 	}
+
+	return 0;
 }
 
 static int
@@ -2080,6 +2051,26 @@  init_power_library(void)
 	}
 	return ret;
 }
+
+static int
+deinit_power_library(void)
+{
+	unsigned int lcore_id;
+	int ret = 0;
+
+	RTE_LCORE_FOREACH(lcore_id) {
+		/* deinit power management library */
+		ret = rte_power_exit(lcore_id);
+		if (ret) {
+			RTE_LOG(ERR, POWER,
+				"Library deinitialization failed on core %u\n",
+				lcore_id);
+			return ret;
+		}
+	}
+	return ret;
+}
+
 static void
 update_telemetry(__rte_unused struct rte_timer *tim,
 		__rte_unused void *arg)
@@ -2530,8 +2521,23 @@  main(int argc, char **argv)
 			return -1;
 	}
 
+	RTE_ETH_FOREACH_DEV(portid)
+	{
+		if ((enabled_port_mask & (1 << portid)) == 0)
+			continue;
+
+		rte_eth_dev_stop(portid);
+		rte_eth_dev_close(portid);
+	}
+
 	if (app_mode == APP_MODE_EMPTY_POLL)
 		rte_power_empty_poll_stat_free();
 
+	if (app_mode != APP_MODE_TELEMETRY && deinit_power_library())
+		rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
+
+	if (rte_eal_cleanup() < 0)
+		RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n");
+
 	return 0;
 }