[PATCH v3] eal: fix core_index for non-EAL registered threads

Maxime Peim <[email protected]>
Newsgroups org.dpdk.dev
Message-ID <[email protected]>
Threads registered via rte_thread_register() are assigned a valid
lcore_id by eal_lcore_non_eal_allocate(), but their core_index in
lcore_config is left at -1. This value was set during rte_eal_cpu_init()
for lcores with ROLE_OFF (undetected CPUs) and is never updated when the
lcore is later allocated to a non-EAL thread.

As a result, rte_lcore_index() returns -1 for registered non-EAL
threads. Libraries that use rte_lcore_index() to select per-lcore
caches fall back to a shared global path when it returns -1, causing
severe contention under concurrent access from multiple registered
threads.

A concrete example is the mlx5 indexed memory pool (mlx5_ipool), which
uses rte_lcore_index() in mlx5_ipool_malloc_cache() to select a per-core
cache slot. When core_index is -1, all registered threads are funneled
into a single shared slot protected by a spinlock. In testing with VPP
(which registers worker threads via rte_thread_register()), this caused
async flow rule insertion throughput to drop from ~6.4M rules/sec to
~1.2M rules/sec with 4 workers -- a 5x regression attributable entirely
to spinlock contention in the ipool allocator.

Fix by setting core_index to the next sequential index (cfg->lcore_count)
in eal_lcore_non_eal_allocate() before incrementing the count. Also reset
core_index back to -1 on the error rollback path and in
eal_lcore_non_eal_release() for correctness.

Fixes: 5c307ba2a5 ("eal: register non-EAL threads as lcores")
Fixes: 99c05ce5ef ("eal: parse coremask as cpuset")
Signed-off-by: Maxime Peim <[email protected]>
---
v2:
  - Track allocated core_index values with a bitset instead of deriving
    the next non-EAL index from lcore_count, avoiding duplicate indices
    after non-EAL lcore release.
  - Keep the bitset in sync when default EAL lcores are discovered, when
    EAL lcore options remap the active set, and when non-EAL lcore
    registration rolls back or releases an lcore.
v3:
  - Fix commit message to track also 99c05ce5ef

 lib/eal/common/eal_common_lcore.c   | 19 ++++++++++++++++++-
 lib/eal/common/eal_common_options.c |  5 +++++
 lib/eal/common/eal_private.h        |  3 +++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
index 39411f9370..b5f59a6380 100644
--- a/lib/eal/common/eal_common_lcore.c
+++ b/lib/eal/common/eal_common_lcore.c
@@ -6,8 +6,9 @@
 #include <stdlib.h>
 #include <string.h>

-#include <rte_common.h>
+#include <rte_bitset.h>
 #include <rte_branch_prediction.h>
+#include <rte_common.h>
 #include <rte_errno.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
@@ -184,6 +185,9 @@ rte_eal_cpu_init(void)
 		/* By default, lcore 1:1 map to cpu id */
 		CPU_SET(lcore_id, &lcore_config[lcore_id].cpuset);

+		/* This is the first time we discover the lcores, so the bitset should be zeroed */
+		rte_bitset_set(config->core_indices, count);
+
 		/* By default, each detected core is enabled */
 		config->lcore_role[lcore_id] = ROLE_RTE;
 		lcore_config[lcore_id].core_role = ROLE_RTE;
@@ -373,11 +377,20 @@ eal_lcore_non_eal_allocate(void)
 	struct lcore_callback *callback;
 	struct lcore_callback *prev;
 	unsigned int lcore_id;
+	int core_index = -1;

 	rte_rwlock_write_lock(&lcore_lock);
+	core_index = rte_bitset_find_first_clear(cfg->core_indices, RTE_MAX_LCORE);
+	if (core_index == -1) {
+		EAL_LOG(DEBUG, "No core_index available.");
+		lcore_id = RTE_MAX_LCORE;
+		goto out;
+	}
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		if (cfg->lcore_role[lcore_id] != ROLE_OFF)
 			continue;
+		rte_bitset_set(cfg->core_indices, core_index);
+		lcore_config[lcore_id].core_index = core_index;
 		cfg->lcore_role[lcore_id] = ROLE_NON_EAL;
 		cfg->lcore_count++;
 		break;
@@ -399,6 +412,8 @@ eal_lcore_non_eal_allocate(void)
 		}
 		EAL_LOG(DEBUG, "Initialization refused for lcore %u.",
 			lcore_id);
+		rte_bitset_clear(cfg->core_indices, lcore_config[lcore_id].core_index);
+		lcore_config[lcore_id].core_index = -1;
 		cfg->lcore_role[lcore_id] = ROLE_OFF;
 		cfg->lcore_count--;
 		lcore_id = RTE_MAX_LCORE;
@@ -420,6 +435,8 @@ eal_lcore_non_eal_release(unsigned int lcore_id)
 		goto out;
 	TAILQ_FOREACH(callback, &lcore_callbacks, next)
 		callback_uninit(callback, lcore_id);
+	rte_bitset_clear(cfg->core_indices, lcore_config[lcore_id].core_index);
+	lcore_config[lcore_id].core_index = -1;
 	cfg->lcore_role[lcore_id] = ROLE_OFF;
 	cfg->lcore_count--;
 out:
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 1049838d73..42cdef632f 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -911,6 +911,7 @@ eal_parse_service_coremask(const char *coremask)
 		if (coremask[i] != '0')
 			return -1;

+	rte_bitset_clear_all(cfg->core_indices, RTE_MAX_LCORE);
 	for (; idx < RTE_MAX_LCORE; idx++)
 		lcore_config[idx].core_index = -1;

@@ -937,6 +938,7 @@ update_lcore_config(const rte_cpuset_t *cpuset, bool remap, uint16_t remap_base)
 	int ret = 0;

 	/* set everything to disabled first, then set up values */
+	rte_bitset_clear_all(cfg->core_indices, RTE_MAX_LCORE);
 	for (i = 0; i < RTE_MAX_LCORE; i++) {
 		cfg->lcore_role[i] = ROLE_OFF;
 		lcore_config[i].core_index = -1;
@@ -966,6 +968,7 @@ update_lcore_config(const rte_cpuset_t *cpuset, bool remap, uint16_t remap_base)
 				continue;
 			}

+			rte_bitset_set(cfg->core_indices, count);
 			cfg->lcore_role[lcore_id] = ROLE_RTE;
 			lcore_config[lcore_id].core_index = count;
 			CPU_ZERO(&lcore_config[lcore_id].cpuset);
@@ -1388,6 +1391,7 @@ eal_parse_lcores(const char *lcores)
 	CPU_ZERO(&cpuset);

 	/* Reset lcore config */
+	rte_bitset_clear_all(cfg->core_indices, RTE_MAX_LCORE);
 	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
 		cfg->lcore_role[idx] = ROLE_OFF;
 		lcore_config[idx].core_index = -1;
@@ -1452,6 +1456,7 @@ eal_parse_lcores(const char *lcores)
 			set_count--;

 			if (cfg->lcore_role[idx] != ROLE_RTE) {
+				rte_bitset_set(cfg->core_indices, count);
 				lcore_config[idx].core_index = count;
 				cfg->lcore_role[idx] = ROLE_RTE;
 				count++;
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 0c0544beaf..6340bab8be 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -11,6 +11,7 @@
 #include <sys/queue.h>

 #include <dev_driver.h>
+#include <rte_bitset.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
 #include <rte_memory.h>
@@ -44,6 +45,8 @@ extern struct lcore_config lcore_config[RTE_MAX_LCORE];
  * The global RTE configuration structure.
  */
 struct rte_config {
+	RTE_BITSET_DECLARE(core_indices,
+			   RTE_MAX_LCORE); /**< bitset of currently allocated core_indices */
 	uint32_t main_lcore;         /**< Id of the main lcore */
 	uint32_t lcore_count;        /**< Number of available logical cores. */
 	uint32_t numa_node_count;    /**< Number of detected NUMA nodes. */
--
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.