diff mbox series

[v2,1/2] eal: cleanup lcore ID hand-over

Message ID 20220401084419.21150-1-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [v2,1/2] eal: cleanup lcore ID hand-over |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand April 1, 2022, 8:44 a.m. UTC 
  So far, a worker thread has been using its thread_id to discover which
lcore has been assigned to it.

On the other hand, as noted by Tyler, the pthread API does not strictly
guarantee that a new thread won't start running eal_thread_loop before
pthread_create writes to &lcore_config[xx].thread_id.

Though all OS implementations supported in DPDK (recently) ensure this
property, it is more robust to have the main thread directly pass
the worker thread lcore.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/common/eal_thread.h  |  4 ++--
 lib/eal/freebsd/eal.c        |  2 +-
 lib/eal/freebsd/eal_thread.c | 16 +++-------------
 lib/eal/linux/eal.c          |  2 +-
 lib/eal/linux/eal_thread.c   | 17 ++++-------------
 lib/eal/windows/eal.c        |  2 +-
 lib/eal/windows/eal_thread.c | 23 +++++++----------------
 7 files changed, 19 insertions(+), 47 deletions(-)
diff mbox series

Patch

diff --git a/lib/eal/common/eal_thread.h b/lib/eal/common/eal_thread.h
index 4a49117be8..3c84efd553 100644
--- a/lib/eal/common/eal_thread.h
+++ b/lib/eal/common/eal_thread.h
@@ -8,10 +8,10 @@ 
 #include <rte_lcore.h>
 
 /**
- * basic loop of thread, called for each thread by eal_init().
+ * Basic loop of EAL thread, called for each worker thread by rte_eal_init().
  *
  * @param arg
- *   opaque pointer
+ *   The lcore_id (passed as an integer) of this worker thread.
  */
 __rte_noreturn void *eal_thread_loop(void *arg);
 
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 71993fe25b..e233e57184 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -813,7 +813,7 @@  rte_eal_init(int argc, char **argv)
 
 		/* create a thread for each lcore */
 		ret = pthread_create(&lcore_config[i].thread_id, NULL,
-				     eal_thread_loop, NULL);
+				     eal_thread_loop, (void *)(uintptr_t)i);
 		if (ret != 0)
 			rte_panic("Cannot create thread\n");
 
diff --git a/lib/eal/freebsd/eal_thread.c b/lib/eal/freebsd/eal_thread.c
index 3b18030d73..9044d70ef7 100644
--- a/lib/eal/freebsd/eal_thread.c
+++ b/lib/eal/freebsd/eal_thread.c
@@ -76,25 +76,15 @@  rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned worker_id)
 
 /* main loop of threads */
 __rte_noreturn void *
-eal_thread_loop(__rte_unused void *arg)
+eal_thread_loop(void *arg)
 {
+	unsigned int lcore_id = (uintptr_t)arg;
 	char c;
 	int n, ret;
 	unsigned lcore_id;
-	pthread_t thread_id;
 	int m2w, w2m;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 
-	thread_id = pthread_self();
-
-	/* retrieve our lcore_id from the configuration structure */
-	RTE_LCORE_FOREACH_WORKER(lcore_id) {
-		if (thread_id == lcore_config[lcore_id].thread_id)
-			break;
-	}
-	if (lcore_id == RTE_MAX_LCORE)
-		rte_panic("cannot retrieve lcore id\n");
-
 	m2w = lcore_config[lcore_id].pipe_main2worker[0];
 	w2m = lcore_config[lcore_id].pipe_worker2main[1];
 
@@ -102,7 +92,7 @@  eal_thread_loop(__rte_unused void *arg)
 
 	ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset));
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%p;cpuset=[%s%s])\n",
-		lcore_id, thread_id, cpuset, ret == 0 ? "" : "...");
+		lcore_id, pthread_self(), cpuset, ret == 0 ? "" : "...");
 
 	rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
 
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 025e5cc10d..98c6838026 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1147,7 +1147,7 @@  rte_eal_init(int argc, char **argv)
 
 		/* create a thread for each lcore */
 		ret = pthread_create(&lcore_config[i].thread_id, NULL,
-				     eal_thread_loop, NULL);
+				     eal_thread_loop, (void *)(uintptr_t)i);
 		if (ret != 0)
 			rte_panic("Cannot create thread\n");
 
diff --git a/lib/eal/linux/eal_thread.c b/lib/eal/linux/eal_thread.c
index fa6cd7e2c4..26b0a7d48a 100644
--- a/lib/eal/linux/eal_thread.c
+++ b/lib/eal/linux/eal_thread.c
@@ -70,24 +70,14 @@  rte_eal_remote_launch(int (*f)(void *), void *arg, unsigned int worker_id)
 
 /* main loop of threads */
 __rte_noreturn void *
-eal_thread_loop(__rte_unused void *arg)
+eal_thread_loop(void *arg)
 {
+	unsigned int lcore_id = (uintptr_t)arg;
 	char c;
 	int n, ret;
-	unsigned lcore_id;
-	pthread_t thread_id;
 	int m2w, w2m;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 
-	thread_id = pthread_self();
-
-	/* retrieve our lcore_id from the configuration structure */
-	RTE_LCORE_FOREACH_WORKER(lcore_id) {
-		if (thread_id == lcore_config[lcore_id].thread_id)
-			break;
-	}
-	if (lcore_id == RTE_MAX_LCORE)
-		rte_panic("cannot retrieve lcore id\n");
 
 	m2w = lcore_config[lcore_id].pipe_main2worker[0];
 	w2m = lcore_config[lcore_id].pipe_worker2main[1];
@@ -96,7 +86,8 @@  eal_thread_loop(__rte_unused void *arg)
 
 	ret = eal_thread_dump_current_affinity(cpuset, sizeof(cpuset));
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s%s])\n",
-		lcore_id, (uintptr_t)thread_id, cpuset, ret == 0 ? "" : "...");
+		lcore_id, (uintptr_t)pthread_self(), cpuset,
+		ret == 0 ? "" : "...");
 
 	rte_eal_trace_thread_lcore_ready(lcore_id, cpuset);
 
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index ca3c41aaa7..1874f9f6d7 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -420,7 +420,7 @@  rte_eal_init(int argc, char **argv)
 		lcore_config[i].state = WAIT;
 
 		/* create a thread for each lcore */
-		if (eal_thread_create(&lcore_config[i].thread_id) != 0)
+		if (eal_thread_create(&lcore_config[i].thread_id, i) != 0)
 			rte_panic("Cannot create thread\n");
 		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
 			sizeof(rte_cpuset_t), &lcore_config[i].cpuset);
diff --git a/lib/eal/windows/eal_thread.c b/lib/eal/windows/eal_thread.c
index ff84cb42af..95e96548de 100644
--- a/lib/eal/windows/eal_thread.c
+++ b/lib/eal/windows/eal_thread.c
@@ -63,32 +63,21 @@  rte_eal_remote_launch(lcore_function_t *f, void *arg, unsigned int worker_id)
 
 /* main loop of threads */
 void *
-eal_thread_loop(void *arg __rte_unused)
+eal_thread_loop(void *arg)
 {
+	unsigned int lcore_id = (uintptr_t)arg;
 	char c;
 	int n, ret;
-	unsigned int lcore_id;
-	pthread_t thread_id;
 	int m2w, w2m;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 
-	thread_id = pthread_self();
-
-	/* retrieve our lcore_id from the configuration structure */
-	RTE_LCORE_FOREACH_WORKER(lcore_id) {
-		if (thread_id == lcore_config[lcore_id].thread_id)
-			break;
-	}
-	if (lcore_id == RTE_MAX_LCORE)
-		rte_panic("cannot retrieve lcore id\n");
-
 	m2w = lcore_config[lcore_id].pipe_main2worker[0];
 	w2m = lcore_config[lcore_id].pipe_worker2main[1];
 
 	__rte_thread_init(lcore_id, &lcore_config[lcore_id].cpuset);
 
 	RTE_LOG(DEBUG, EAL, "lcore %u is ready (tid=%zx;cpuset=[%s])\n",
-		lcore_id, (uintptr_t)thread_id, cpuset);
+		lcore_id, (uintptr_t)pthread_self(), cpuset);
 
 	/* read on our pipe to get commands */
 	while (1) {
@@ -144,13 +133,15 @@  eal_thread_loop(void *arg __rte_unused)
 
 /* function to create threads */
 int
-eal_thread_create(pthread_t *thread)
+eal_thread_create(pthread_t *thread, unsigned int lcore_id)
 {
 	HANDLE th;
 
 	th = CreateThread(NULL, 0,
 		(LPTHREAD_START_ROUTINE)(ULONG_PTR)eal_thread_loop,
-						NULL, CREATE_SUSPENDED, (LPDWORD)thread);
+						(LPVOID)(uintptr_t)lcore_id,
+						CREATE_SUSPENDED,
+						(LPDWORD)thread);
 	if (!th)
 		return -1;