DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] eal: fix calling cleanup function twice
@ 2023-06-01 16:08 Bruce Richardson
  2023-06-05 16:12 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Bruce Richardson @ 2023-06-01 16:08 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, stable

If an app calls rte_eal_cleanup() inside it's own code, then cleanup
could be called a second time automatically when the app exits. While
mostly harmless, we can avoid any potential issues by guaranteeing that
cleanup only gets called once, in the same way that eal_init only ever
gets called once.

Note: This patch only touches Linux and FreeBSD. Windows EAL does not
have run-once guard on the init function, so omitting it in the cleanup
function.

Fixes: aec9c13c5257 ("eal: add function to release internal resources")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/common/eal_common_debug.c |  4 +++-
 lib/eal/freebsd/eal.c             | 10 ++++++++++
 lib/eal/linux/eal.c               | 10 ++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_debug.c b/lib/eal/common/eal_common_debug.c
index dcb554af1e..9cac9c6390 100644
--- a/lib/eal/common/eal_common_debug.c
+++ b/lib/eal/common/eal_common_debug.c
@@ -4,10 +4,12 @@
 
 #include <stdarg.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_debug.h>
+#include <rte_errno.h>
 
 void
 __rte_panic(const char *funcname, const char *format, ...)
@@ -39,7 +41,7 @@ rte_exit(int exit_code, const char *format, ...)
 	rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
 	va_end(ap);
 
-	if (rte_eal_cleanup() != 0)
+	if (rte_eal_cleanup() != 0 && rte_errno != EALREADY)
 		RTE_LOG(CRIT, EAL,
 			"EAL could not release all resources\n");
 	exit(exit_code);
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index f125a0d3de..a2ce188c83 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -898,6 +898,16 @@ rte_eal_init(int argc, char **argv)
 int
 rte_eal_cleanup(void)
 {
+	static uint32_t run_once;
+	uint32_t has_run = 0;
+
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+			__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
+		RTE_LOG(WARNING, EAL, "Already called cleanup\n");
+		rte_errno = EALREADY;
+		return -1;
+	}
+
 	struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 	rte_service_finalize();
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index c80647d6b2..f8d66b9b6d 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1361,6 +1361,16 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	static uint32_t run_once;
+	uint32_t has_run = 0;
+
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
+		RTE_LOG(WARNING, EAL, "Already called cleanup\n");
+		rte_errno = EALREADY;
+		return -1;
+	}
+
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */
-- 
2.39.2


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

* Re: [PATCH] eal: fix calling cleanup function twice
  2023-06-01 16:08 [PATCH] eal: fix calling cleanup function twice Bruce Richardson
@ 2023-06-05 16:12 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2023-06-05 16:12 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dev, stable, david.marchand, Tyler Retzlaff, dmitry.kozliuk,
	Tal Shnaiderman

01/06/2023 18:08, Bruce Richardson:
> If an app calls rte_eal_cleanup() inside it's own code, then cleanup
> could be called a second time automatically when the app exits. While
> mostly harmless, we can avoid any potential issues by guaranteeing that
> cleanup only gets called once, in the same way that eal_init only ever
> gets called once.
> 
> Note: This patch only touches Linux and FreeBSD. Windows EAL does not
> have run-once guard on the init function, so omitting it in the cleanup
> function.
> 
> Fixes: aec9c13c5257 ("eal: add function to release internal resources")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.

Please we should keep the same behavior in Linux, FreeBSD and Windows.
Is there any volunteer to add run-once guards in init and cleanup
for Windows EAL?




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

end of thread, other threads:[~2023-06-05 16:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01 16:08 [PATCH] eal: fix calling cleanup function twice Bruce Richardson
2023-06-05 16:12 ` 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).