DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v4 0/8] support reinit flow
@ 2023-08-15 14:50 okaya
  2023-08-15 14:50 ` [PATCH v4 1/8] eal: cleanup plugins data okaya
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

We want to be able to call rte_eal_init() and rte_eal_cleanup()
APIs back to back for maintanance reasons.

Here is a summary of the code we have seen so far:

1. some code support getting called multiple times by keeping
a static variable.
2. some code initializes once but never clean up after them and
don't have a cleanup API.
3. some code assumes that they only get called once during the
lifecycle of the process.

Most changes in this patch center around following the #1 design
principle.

Why?

It is not always ideal to reinitialize a DPDK process. Memory needs
to be reinitialized, hugetables need to warm up etc.

Changed from

v1:
Fix checkpatch warnings

v2:
rebase to most recent DPDK.

v3:
pick up Stephen's "eal: cleanup plugins data" as a pre-requisite
patch.

Graham Whyte (1):
  eal: fixes for re-initialization issues

Sinan Kaya (6):
  tailq: skip init if already initialized
  eal_memzone: bail out on initialized
  memseg: init once
  eal_memory: skip initialization
  eal_interrupts: don't reinitialize threads
  eal: initialize worker threads once

Stephen Hemminger (1):
  eal: cleanup plugins data

 lib/eal/common/eal_common_memory.c  |  5 +++
 lib/eal/common/eal_common_memzone.c |  7 +++
 lib/eal/common/eal_common_options.c | 20 +++++++++
 lib/eal/common/eal_common_tailqs.c  | 20 ++++++---
 lib/eal/common/eal_options.h        |  1 +
 lib/eal/common/malloc_heap.c        |  7 +++
 lib/eal/linux/eal.c                 | 66 ++++++++++++++++-------------
 lib/eal/linux/eal_interrupts.c      |  7 +++
 lib/eal/linux/eal_memory.c          | 12 +++++-
 9 files changed, 108 insertions(+), 37 deletions(-)

-- 
2.25.1


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

* [PATCH v4 1/8] eal: cleanup plugins data
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 14:50 ` [PATCH v4 2/8] eal: fixes for re-initialization issues okaya
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  Cc: dev, Stephen Hemminger, Sinan Kaya

From: Stephen Hemminger <stephen@networkplumber.org>

When rte_eal_cleanup is called walk through the list of shared
objects loaded, and close them and free the data structure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_options.c | 12 ++++++++++++
 lib/eal/common/eal_options.h        |  1 +
 lib/eal/linux/eal.c                 |  2 ++
 3 files changed, 15 insertions(+)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 062f1d8d9c..209b6edd76 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -244,6 +244,18 @@ eal_save_args(int argc, char **argv)
 }
 #endif
 
+void
+eal_plugins_cleanup(void)
+{
+	struct shared_driver *solib, *tmp;
+
+	RTE_TAILQ_FOREACH_SAFE(solib, &solib_list, next, tmp) {
+		if (solib->lib_handle)
+			dlclose(solib->lib_handle);
+		free(solib);
+	}
+}
+
 static int
 eal_option_device_add(enum rte_devtype type, const char *optarg)
 {
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..ddbaafc4f1 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -105,6 +105,7 @@ int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
 enum rte_proc_type_t eal_proc_type_detect(void);
 int eal_plugins_init(void);
+void eal_plugins_cleanup(void);
 int eal_save_args(int argc, char **argv);
 int handle_eal_info_request(const char *cmd, const char *params __rte_unused,
 		struct rte_tel_data *d);
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index c6efd92014..dee649bab3 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1398,6 +1398,8 @@ rte_eal_cleanup(void)
 	eal_trace_fini();
 	eal_mp_dev_hotplug_cleanup();
 	rte_eal_alarm_cleanup();
+	eal_plugins_cleanup();
+
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	rte_eal_malloc_heap_cleanup();
-- 
2.25.1


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

* [PATCH v4 2/8] eal: fixes for re-initialization issues
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
  2023-08-15 14:50 ` [PATCH v4 1/8] eal: cleanup plugins data okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 17:49   ` Stephen Hemminger
  2023-08-15 14:50 ` [PATCH v4 3/8] tailq: skip init if already initialized okaya
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  Cc: dev, Graham Whyte, Sinan Kaya

From: Graham Whyte <grwhyte@microsoft.com>

reinitialize the solib link list and clean the globals holding
state for parsing.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
Signed-off-by: Graham Whyte <grwhyte@microsoft.com>
---
 lib/eal/common/eal_common_options.c | 8 ++++++++
 lib/eal/linux/eal.c                 | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 209b6edd76..6042de009d 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -127,6 +127,7 @@ TAILQ_HEAD_INITIALIZER(solib_list);
 static const char *default_solib_dir = RTE_EAL_PMD_PATH;
 #endif
 
+
 /*
  * Stringified version of solib path used by dpdk-pmdinfo.py
  * Note: PLEASE DO NOT ALTER THIS without making a corresponding
@@ -254,6 +255,13 @@ eal_plugins_cleanup(void)
 			dlclose(solib->lib_handle);
 		free(solib);
 	}
+
+	/* Reinitialize solib_list */
+	TAILQ_INIT(&solib_list);
+
+	main_lcore_parsed = 0;
+	mem_parsed = 0;
+	core_parsed = 0;
 }
 
 static int
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index dee649bab3..584c78e640 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -79,6 +79,8 @@ struct lcore_config lcore_config[RTE_MAX_LCORE];
 int rte_cycles_vmware_tsc_map;
 
 
+static uint32_t run_once;
+
 int
 eal_clean_runtime_dir(void)
 {
@@ -505,6 +507,7 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
 		socket_arg[i] = val;
 	}
 
+	__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 	return 0;
 }
 
@@ -967,7 +970,6 @@ int
 rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
-	static uint32_t run_once;
 	uint32_t has_run = 0;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
-- 
2.25.1


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

* [PATCH v4 3/8] tailq: skip init if already initialized
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
  2023-08-15 14:50 ` [PATCH v4 1/8] eal: cleanup plugins data okaya
  2023-08-15 14:50 ` [PATCH v4 2/8] eal: fixes for re-initialization issues okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 14:50 ` [PATCH v4 4/8] eal_memzone: bail out on initialized okaya
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Allows tailq to be reinitialied multiple times
by looking up previously registered tailqs

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_tailqs.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/eal/common/eal_common_tailqs.c b/lib/eal/common/eal_common_tailqs.c
index 580fbf24bc..75c0235438 100644
--- a/lib/eal/common/eal_common_tailqs.c
+++ b/lib/eal/common/eal_common_tailqs.c
@@ -73,9 +73,10 @@ rte_eal_tailq_create(const char *name)
 		strlcpy(head->name, name, sizeof(head->name) - 1);
 		TAILQ_INIT(&head->tailq_head);
 		rte_tailqs_count++;
+		return head;
 	}
 
-	return head;
+	return rte_eal_tailq_lookup(name);
 }
 
 /* local register, used to store "early" tailqs before rte_eal_init() and to
@@ -99,7 +100,9 @@ rte_eal_tailq_update(struct rte_tailq_elem *t)
 {
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		/* primary process is the only one that creates */
-		t->head = rte_eal_tailq_create(t->name);
+		t->head = rte_eal_tailq_lookup(t->name);
+		if (t->head == NULL)
+			t->head = rte_eal_tailq_create(t->name);
 	} else {
 		t->head = rte_eal_tailq_lookup(t->name);
 	}
@@ -108,15 +111,13 @@ rte_eal_tailq_update(struct rte_tailq_elem *t)
 int
 rte_eal_tailq_register(struct rte_tailq_elem *t)
 {
-	if (rte_eal_tailq_local_register(t) < 0) {
-		RTE_LOG(ERR, EAL,
-			"%s tailq is already registered\n", t->name);
-		goto error;
-	}
+	rte_eal_tailq_local_register(t);
 
 	/* if a register happens after rte_eal_tailqs_init(), then we can update
 	 * tailq head */
 	if (rte_tailqs_count >= 0) {
+		RTE_LOG(INFO, EAL,
+			"%s tailq is registered\n", t->name);
 		rte_eal_tailq_update(t);
 		if (t->head == NULL) {
 			RTE_LOG(ERR, EAL,
@@ -138,6 +139,11 @@ rte_eal_tailqs_init(void)
 {
 	struct rte_tailq_elem *t;
 
+	if (rte_tailqs_count > 0) {
+		RTE_LOG(INFO, EAL, "tailq already initialized\n");
+		return 0;
+	}
+
 	rte_tailqs_count = 0;
 
 	TAILQ_FOREACH(t, &rte_tailq_elem_head, next) {
-- 
2.25.1


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

* [PATCH v4 4/8] eal_memzone: bail out on initialized
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (2 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 3/8] tailq: skip init if already initialized okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 14:50 ` [PATCH v4 5/8] memseg: init once okaya
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Initialize memzone once and bail out if someone calls init
multiple times.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memzone.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/eal/common/eal_common_memzone.c b/lib/eal/common/eal_common_memzone.c
index 1f3e701499..6645ccfe83 100644
--- a/lib/eal/common/eal_common_memzone.c
+++ b/lib/eal/common/eal_common_memzone.c
@@ -22,6 +22,8 @@
 #include "eal_private.h"
 #include "eal_memcfg.h"
 
+static bool memzone_initialized;
+
 /* Default count used until rte_memzone_max_set() is called */
 #define DEFAULT_MAX_MEMZONE_COUNT 2560
 
@@ -426,6 +428,9 @@ rte_eal_memzone_init(void)
 	struct rte_mem_config *mcfg;
 	int ret = 0;
 
+	if (memzone_initialized)
+		return 0;
+
 	/* get pointer to global configuration */
 	mcfg = rte_eal_get_configuration()->mem_config;
 
@@ -444,6 +449,8 @@ rte_eal_memzone_init(void)
 
 	rte_rwlock_write_unlock(&mcfg->mlock);
 
+	memzone_initialized = true;
+
 	return ret;
 }
 
-- 
2.25.1


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

* [PATCH v4 5/8] memseg: init once
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (3 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 4/8] eal_memzone: bail out on initialized okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 14:50 ` [PATCH v4 6/8] eal_memory: skip initialization okaya
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Initialize memory segments just once and bail out if called
multiple times.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal_memory.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
index 9b6f08fba8..df0aa9ccc7 100644
--- a/lib/eal/linux/eal_memory.c
+++ b/lib/eal/linux/eal_memory.c
@@ -57,6 +57,7 @@
  */
 
 static int phys_addrs_available = -1;
+static bool memseg_initialized;
 
 #define RANDOMIZE_VA_SPACE_FILE "/proc/sys/kernel/randomize_va_space"
 
@@ -1920,6 +1921,10 @@ rte_eal_memseg_init(void)
 {
 	/* increase rlimit to maximum */
 	struct rlimit lim;
+	int ret;
+
+	if (memseg_initialized)
+		return 0;
 
 #ifndef RTE_EAL_NUMA_AWARE_HUGEPAGES
 	const struct internal_config *internal_conf =
@@ -1948,11 +1953,16 @@ rte_eal_memseg_init(void)
 	}
 #endif
 
-	return rte_eal_process_type() == RTE_PROC_PRIMARY ?
+	ret = rte_eal_process_type() == RTE_PROC_PRIMARY ?
 #ifndef RTE_ARCH_64
 			memseg_primary_init_32() :
 #else
 			memseg_primary_init() :
 #endif
 			memseg_secondary_init();
+
+	if (!ret)
+		memseg_initialized = true;
+
+	return ret;
 }
-- 
2.25.1


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

* [PATCH v4 6/8] eal_memory: skip initialization
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (4 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 5/8] memseg: init once okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 14:50 ` [PATCH v4 7/8] eal_interrupts: don't reinitialize threads okaya
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  To: Anatoly Burakov; +Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Initialize heap area just once.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/common/eal_common_memory.c | 5 +++++
 lib/eal/common/malloc_heap.c       | 7 +++++++
 2 files changed, 12 insertions(+)

diff --git a/lib/eal/common/eal_common_memory.c b/lib/eal/common/eal_common_memory.c
index d9433db623..4c68de1054 100644
--- a/lib/eal/common/eal_common_memory.c
+++ b/lib/eal/common/eal_common_memory.c
@@ -41,6 +41,7 @@
 
 static void *next_baseaddr;
 static uint64_t system_page_sz;
+static bool memory_initialized;
 
 #define MAX_MMAP_WITH_DEFINED_ADDR_TRIES 5
 void *
@@ -1084,6 +1085,9 @@ rte_eal_memory_init(void)
 		eal_get_internal_configuration();
 	int retval;
 
+	if (memory_initialized)
+		return 0;
+
 	RTE_LOG(DEBUG, EAL, "Setting up physically contiguous memory...\n");
 
 	if (rte_eal_memseg_init() < 0)
@@ -1101,6 +1105,7 @@ rte_eal_memory_init(void)
 	if (internal_conf->no_shconf == 0 && rte_eal_memdevice_init() < 0)
 		goto fail;
 
+	memory_initialized = true;
 	return 0;
 fail:
 	return -1;
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index 6b6cf9174c..8d05a2ab2e 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -31,6 +31,8 @@
 #define CONST_MAX(a, b) (a > b ? a : b) /* RTE_MAX is not a constant */
 #define EXTERNAL_HEAP_MIN_SOCKET_ID (CONST_MAX((1 << 8), RTE_MAX_NUMA_NODES))
 
+static bool heap_initialized;
+
 static unsigned
 check_hugepage_sz(unsigned flags, uint64_t hugepage_sz)
 {
@@ -1410,6 +1412,9 @@ rte_eal_malloc_heap_init(void)
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 
+	if (heap_initialized)
+		return 0;
+
 	if (internal_conf->match_allocations)
 		RTE_LOG(DEBUG, EAL, "Hugepages will be freed exactly as allocated.\n");
 
@@ -1449,6 +1454,8 @@ int rte_eal_malloc_heap_populate(void)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	heap_initialized = true;
+
 	/* add all IOVA-contiguous areas to the heap */
 	return rte_memseg_contig_walk(malloc_add_seg, NULL);
 }
-- 
2.25.1


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

* [PATCH v4 7/8] eal_interrupts: don't reinitialize threads
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (5 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 6/8] eal_memory: skip initialization okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 17:47   ` Stephen Hemminger
  2023-08-15 14:50 ` [PATCH v4 8/8] eal: initialize worker threads once okaya
  2023-08-15 17:45 ` [PATCH v4 0/8] support reinit flow Stephen Hemminger
  8 siblings, 1 reply; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  To: Harman Kalra; +Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Initialize interrupt thread once and keep a flag
for further init.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal_interrupts.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index c9881143be..6a35b4aebd 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -91,6 +91,9 @@ static struct rte_intr_source_list intr_sources;
 /* interrupt handling thread */
 static pthread_t intr_thread;
 
+/* flag for initialization */
+static bool intr_initialized;
+
 /* VFIO interrupts */
 #ifdef VFIO_PRESENT
 
@@ -1175,6 +1178,9 @@ rte_eal_intr_init(void)
 {
 	int ret = 0;
 
+	if (intr_initialized)
+		return 0;
+
 	/* init the global interrupt source head */
 	TAILQ_INIT(&intr_sources);
 
@@ -1196,6 +1202,7 @@ rte_eal_intr_init(void)
 			"Failed to create thread for interrupt handling\n");
 	}
 
+	intr_initialized = true;
 	return ret;
 }
 
-- 
2.25.1


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

* [PATCH v4 8/8] eal: initialize worker threads once
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (6 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 7/8] eal_interrupts: don't reinitialize threads okaya
@ 2023-08-15 14:50 ` okaya
  2023-08-15 17:46   ` Stephen Hemminger
  2023-08-15 17:45 ` [PATCH v4 0/8] support reinit flow Stephen Hemminger
  8 siblings, 1 reply; 17+ messages in thread
From: okaya @ 2023-08-15 14:50 UTC (permalink / raw)
  Cc: dev, Sinan Kaya

From: Sinan Kaya <okaya@kernel.org>

Initialize worker threads once and keep a flag
for other init calls.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 lib/eal/linux/eal.c | 60 ++++++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 584c78e640..1305e1df54 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -80,6 +80,7 @@ int rte_cycles_vmware_tsc_map;
 
 
 static uint32_t run_once;
+static bool worker_initialized;
 
 int
 eal_clean_runtime_dir(void)
@@ -1254,42 +1255,45 @@ rte_eal_init(int argc, char **argv)
 		config->main_lcore, (uintptr_t)pthread_self(), cpuset,
 		ret == 0 ? "" : "...");
 
-	RTE_LCORE_FOREACH_WORKER(i) {
+	if (worker_initialized == false) {
+		RTE_LCORE_FOREACH_WORKER(i) {
 
 		/*
 		 * create communication pipes between main thread
 		 * and children
 		 */
-		if (pipe(lcore_config[i].pipe_main2worker) < 0)
-			rte_panic("Cannot create pipe\n");
-		if (pipe(lcore_config[i].pipe_worker2main) < 0)
-			rte_panic("Cannot create pipe\n");
-
-		lcore_config[i].state = WAIT;
-
-		/* create a thread for each lcore */
-		ret = eal_worker_thread_create(i);
-		if (ret != 0)
-			rte_panic("Cannot create thread\n");
-
-		/* Set thread_name for aid in debugging. */
-		snprintf(thread_name, sizeof(thread_name),
-			"rte-worker-%d", i);
-		rte_thread_set_name(lcore_config[i].thread_id, thread_name);
+			if (pipe(lcore_config[i].pipe_main2worker) < 0)
+				rte_panic("Cannot create pipe\n");
+			if (pipe(lcore_config[i].pipe_worker2main) < 0)
+				rte_panic("Cannot create pipe\n");
+
+			lcore_config[i].state = WAIT;
+
+			/* create a thread for each lcore */
+			ret = eal_worker_thread_create(i);
+			if (ret != 0)
+				rte_panic("Cannot create thread\n");
+
+			/* Set thread_name for aid in debugging. */
+			snprintf(thread_name, sizeof(thread_name),
+				"rte-worker-%d", i);
+			rte_thread_set_name(lcore_config[i].thread_id, thread_name);
+
+			ret = rte_thread_set_affinity_by_id(lcore_config[i].thread_id,
+				&lcore_config[i].cpuset);
+			if (ret != 0)
+				rte_panic("Cannot set affinity\n");
+		}
 
-		ret = rte_thread_set_affinity_by_id(lcore_config[i].thread_id,
-			&lcore_config[i].cpuset);
-		if (ret != 0)
-			rte_panic("Cannot set affinity\n");
+		/*
+		 * Launch a dummy function on all worker lcores, so that main lcore
+		 * knows they are all ready when this function returns.
+		 */
+		rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MAIN);
+		rte_eal_mp_wait_lcore();
+		worker_initialized = true;
 	}
 
-	/*
-	 * Launch a dummy function on all worker lcores, so that main lcore
-	 * knows they are all ready when this function returns.
-	 */
-	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MAIN);
-	rte_eal_mp_wait_lcore();
-
 	/* initialize services so vdevs register service during bus_probe. */
 	ret = rte_service_init();
 	if (ret) {
-- 
2.25.1


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

* Re: [PATCH v4 0/8] support reinit flow
  2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
                   ` (7 preceding siblings ...)
  2023-08-15 14:50 ` [PATCH v4 8/8] eal: initialize worker threads once okaya
@ 2023-08-15 17:45 ` Stephen Hemminger
  2023-08-15 18:58   ` Sinan Kaya
  8 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2023-08-15 17:45 UTC (permalink / raw)
  To: okaya; +Cc: dev

On Tue, 15 Aug 2023 10:50:15 -0400
okaya@kernel.org wrote:

> From: Sinan Kaya <okaya@kernel.org>
> 
> We want to be able to call rte_eal_init() and rte_eal_cleanup()
> APIs back to back for maintanance reasons.
> 
> Here is a summary of the code we have seen so far:
> 
> 1. some code support getting called multiple times by keeping
> a static variable.
> 2. some code initializes once but never clean up after them and
> don't have a cleanup API.
> 3. some code assumes that they only get called once during the
> lifecycle of the process.
> 
> Most changes in this patch center around following the #1 design
> principle.
> 
> Why?
> 
> It is not always ideal to reinitialize a DPDK process. Memory needs
> to be reinitialized, hugetables need to warm up etc.


I am familiar with the backstory of why this is desirable in your case.
But others may not be. It will work for you, but for the wider the
range of libraries and drivers it probably won't.

As a compromise, can this restart be officially tagged as unsupported.
I.e. it may work for some drivers and libraries but not all of them.
If nothing else many parts of DPDK still do leak memory on cleanup
and currently this is harmless.

This has enough impact that it probably needs to wait past 23.11 release.

Could you add a test for restart into standalone tests and test-pmd?


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

* Re: [PATCH v4 8/8] eal: initialize worker threads once
  2023-08-15 14:50 ` [PATCH v4 8/8] eal: initialize worker threads once okaya
@ 2023-08-15 17:46   ` Stephen Hemminger
  2023-08-15 18:43     ` Sinan Kaya
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2023-08-15 17:46 UTC (permalink / raw)
  To: okaya; +Cc: dev

On Tue, 15 Aug 2023 10:50:23 -0400
okaya@kernel.org wrote:

> From: Sinan Kaya <okaya@kernel.org>
> 
> Initialize worker threads once and keep a flag
> for other init calls.
> 
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  lib/eal/linux/eal.c | 60 ++++++++++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 584c78e640..1305e1df54 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -80,6 +80,7 @@ int rte_cycles_vmware_tsc_map;
>  
>  
>  static uint32_t run_once;
> +static bool worker_initialized;
>  

I see a pattern here. Many places are using static to
only initialize once. Could you name these variables the
same; suggest using run_once everywhere.

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

* Re: [PATCH v4 7/8] eal_interrupts: don't reinitialize threads
  2023-08-15 14:50 ` [PATCH v4 7/8] eal_interrupts: don't reinitialize threads okaya
@ 2023-08-15 17:47   ` Stephen Hemminger
  0 siblings, 0 replies; 17+ messages in thread
From: Stephen Hemminger @ 2023-08-15 17:47 UTC (permalink / raw)
  To: okaya; +Cc: Harman Kalra, dev

On Tue, 15 Aug 2023 10:50:22 -0400
okaya@kernel.org wrote:

>  
> +/* flag for initialization */
> +static bool intr_initialized;

Could be local to function like other places with same pattern.

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

* Re: [PATCH v4 2/8] eal: fixes for re-initialization issues
  2023-08-15 14:50 ` [PATCH v4 2/8] eal: fixes for re-initialization issues okaya
@ 2023-08-15 17:49   ` Stephen Hemminger
  2023-08-15 18:35     ` Sinan Kaya
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2023-08-15 17:49 UTC (permalink / raw)
  To: okaya; +Cc: dev, Graham Whyte

On Tue, 15 Aug 2023 10:50:17 -0400
okaya@kernel.org wrote:

> +static uint32_t run_once;
> +
>  int
>  eal_clean_runtime_dir(void)
>  {
> @@ -505,6 +507,7 @@ eal_parse_socket_arg(char *strval, volatile uint64_t *socket_arg)
>  		socket_arg[i] = val;
>  	}
>  
> +	__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>  	return 0;

Interesting, other flags don't use atomic. Why here?

And is already set elsewhere?

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

* Re: [PATCH v4 2/8] eal: fixes for re-initialization issues
  2023-08-15 17:49   ` Stephen Hemminger
@ 2023-08-15 18:35     ` Sinan Kaya
  0 siblings, 0 replies; 17+ messages in thread
From: Sinan Kaya @ 2023-08-15 18:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Graham Whyte

On Tue, 2023-08-15 at 10:49 -0700, Stephen Hemminger wrote:
> >   
> > +     __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >        return 0;
> 
> 
> Interesting, other flags don't use atomic. Why here?
> 
> 
> 
> And is already set elsewhere?

Looking at the history, this variable used to be an atomic
variable. Later, it got replaced with a uint32_t and write
is done with __atomic_store_n instead. I followed other
examples in the same source code file.


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

* Re: [PATCH v4 8/8] eal: initialize worker threads once
  2023-08-15 17:46   ` Stephen Hemminger
@ 2023-08-15 18:43     ` Sinan Kaya
  0 siblings, 0 replies; 17+ messages in thread
From: Sinan Kaya @ 2023-08-15 18:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Tue, 2023-08-15 at 10:46 -0700, Stephen Hemminger wrote:
> >   static uint32_t run_once;
> > +static bool worker_initialized;
> >   
> 
> 
> I see a pattern here. Many places are using static to
> 
> only initialize once. Could you name these variables the
> 
> same; suggest using run_once everywhere.

sure, will do.


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

* Re: [PATCH v4 0/8] support reinit flow
  2023-08-15 17:45 ` [PATCH v4 0/8] support reinit flow Stephen Hemminger
@ 2023-08-15 18:58   ` Sinan Kaya
  2023-08-15 19:47     ` Stephen Hemminger
  0 siblings, 1 reply; 17+ messages in thread
From: Sinan Kaya @ 2023-08-15 18:58 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Tue, 2023-08-15 at 10:45 -0700, Stephen Hemminger wrote:
> > Why?
> > It is not always ideal to reinitialize a DPDK process. Memory needs
> > to be reinitialized, hugetables need to warm up etc.
> 
> 
> 
> 
> I am familiar with the backstory of why this is desirable in your
> case.
> 
> But others may not be. It will work for you, but for the wider the
> 
> range of libraries and drivers it probably won't.
> 
> 

Fair enough.

> 
> As a compromise, can this restart be officially tagged as
> unsupported.

any pointers how to do this?

I have no idea how to mark something unsupported in code.
If this is acceptable in cover letter, I'm happy to do that too.

> 
> I.e. it may work for some drivers and libraries but not all of them.
> 
> If nothing else many parts of DPDK still do leak memory on cleanup
> 
> and currently this is harmless.
> 
> 
> 
> This has enough impact that it probably needs to wait past 23.11
> release.
> 
> 

Sure, no rush. Happy to wait for time slot and accumulate review
feedbacks.

> 
> Could you add a test for restart into standalone tests and test-pmd?

Will do.



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

* Re: [PATCH v4 0/8] support reinit flow
  2023-08-15 18:58   ` Sinan Kaya
@ 2023-08-15 19:47     ` Stephen Hemminger
  0 siblings, 0 replies; 17+ messages in thread
From: Stephen Hemminger @ 2023-08-15 19:47 UTC (permalink / raw)
  To: Sinan Kaya; +Cc: dev

On Tue, 15 Aug 2023 14:58:03 -0400
Sinan Kaya <okaya@kernel.org> wrote:

> > 
> > As a compromise, can this restart be officially tagged as
> > unsupported.  
> 
> any pointers how to do this?
> 
> I have no idea how to mark something unsupported in code.
> If this is acceptable in cover letter, I'm happy to do that too.

Put some additional notes in the rte_eal.h like:

diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 53c4a5519e61..348b340f006d 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -67,6 +67,11 @@ int rte_eal_iopl_init(void);
  * as possible in the application's main() function.
  * It puts the WORKER lcores in the WAIT state.
  *
+ * @warning
+ * It maybe possisble to call it again after rte_eal_cleanup().
+ * But this usage is dependent on libraries and drivers support which
+ * may not work. Use at your own risk.
+ *
  * @param argc
  *   A non-negative value.  If it is greater than 0, the array members
  *   for argv[0] through argv[argc] (non-inclusive) shall contain pointers

Or maybe in the Shutdown and Cleanup section of:
 doc/guides/prog_guide/env_abstraction_layer.rst


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

end of thread, other threads:[~2023-08-15 19:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-15 14:50 [PATCH v4 0/8] support reinit flow okaya
2023-08-15 14:50 ` [PATCH v4 1/8] eal: cleanup plugins data okaya
2023-08-15 14:50 ` [PATCH v4 2/8] eal: fixes for re-initialization issues okaya
2023-08-15 17:49   ` Stephen Hemminger
2023-08-15 18:35     ` Sinan Kaya
2023-08-15 14:50 ` [PATCH v4 3/8] tailq: skip init if already initialized okaya
2023-08-15 14:50 ` [PATCH v4 4/8] eal_memzone: bail out on initialized okaya
2023-08-15 14:50 ` [PATCH v4 5/8] memseg: init once okaya
2023-08-15 14:50 ` [PATCH v4 6/8] eal_memory: skip initialization okaya
2023-08-15 14:50 ` [PATCH v4 7/8] eal_interrupts: don't reinitialize threads okaya
2023-08-15 17:47   ` Stephen Hemminger
2023-08-15 14:50 ` [PATCH v4 8/8] eal: initialize worker threads once okaya
2023-08-15 17:46   ` Stephen Hemminger
2023-08-15 18:43     ` Sinan Kaya
2023-08-15 17:45 ` [PATCH v4 0/8] support reinit flow Stephen Hemminger
2023-08-15 18:58   ` Sinan Kaya
2023-08-15 19:47     ` Stephen Hemminger

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).