Hello Konstantin,

On Mon, Jun 22, 2020 at 5:49 PM Ananyev, Konstantin
[off-list ref] wrote:

quoted

diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
index 86d32a3dd7..7db05428e7 100644
--- a/lib/librte_eal/common/eal_common_lcore.c
+++ b/lib/librte_eal/common/eal_common_lcore.c

@@ -220,3 +221,38 @@ rte_socket_id_by_idx(unsigned int idx)
      }
      return config->numa_nodes[idx];
 }
+
+static rte_spinlock_t lcore_lock = RTE_SPINLOCK_INITIALIZER;
+
+unsigned int
+eal_lcore_non_eal_allocate(void)
+{
+     struct rte_config *cfg = rte_eal_get_configuration();
+     unsigned int lcore_id;
+
+     rte_spinlock_lock(&lcore_lock);

I think it will break current DPDK MP modes.
The problem here - rte_config (and lcore_role[]) is in shared memory,
while the lock is local.
Simplest way probably to move lcore_lock to rte_config.

Even before this series, MP has no protection on lcore placing between
primary and secondary processes.
Personally, I have no use for DPDK MP and marking MP as not supporting
this new feature is tempting for a first phase.
If this is a strong requirement, I can look at it in a second phase.
What do you think?

quoted

+     for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+             if (cfg->lcore_role[lcore_id] != ROLE_OFF)
+                     continue;
+             cfg->lcore_role[lcore_id] = ROLE_NON_EAL;
+             cfg->lcore_count++;
+             break;
+     }
+     if (lcore_id == RTE_MAX_LCORE)
+             RTE_LOG(DEBUG, EAL, "No lcore available.\n");
+     rte_spinlock_unlock(&lcore_lock);
+     return lcore_id;
+}
+
+void
+eal_lcore_non_eal_release(unsigned int lcore_id)
+{
+     struct rte_config *cfg = rte_eal_get_configuration();
+
+     rte_spinlock_lock(&lcore_lock);
+     if (cfg->lcore_role[lcore_id] == ROLE_NON_EAL) {
+             cfg->lcore_role[lcore_id] = ROLE_OFF;
+             cfg->lcore_count--;
+     }
+     rte_spinlock_unlock(&lcore_lock);
+}

diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c
index a7ae0691bf..1cbddc4b5b 100644
--- a/lib/librte_eal/common/eal_common_thread.c
+++ b/lib/librte_eal/common/eal_common_thread.c

@@ -236,3 +236,36 @@ rte_ctrl_thread_create(pthread_t *thread, const char *name,
      pthread_join(*thread, NULL);
      return -ret;
 }
+
+void
+rte_thread_register(void)
+{
+     unsigned int lcore_id;
+     rte_cpuset_t cpuset;
+
+     /* EAL init flushes all lcores, we can't register before. */
+     assert(internal_config.init_complete == 1);
+     if (pthread_getaffinity_np(pthread_self(), sizeof(cpuset),
+                     &cpuset) != 0)
+             CPU_ZERO(&cpuset);
+     lcore_id = eal_lcore_non_eal_allocate();
+     if (lcore_id >= RTE_MAX_LCORE)
+             lcore_id = LCORE_ID_ANY;
+     rte_thread_init(lcore_id, &cpuset);

So we just setting affinity to the same value, right?
Not a big deal, but might be easier to allow rte_thread_init()
to accept cpuset==NULL (and just don't change thread affinity in that case)

rte_thread_init does not change the thread cpu affinity, it handles
per thread (TLS included) variables initialization.

So do you mean accepting cpuset == NULL and do the getaffinity in this case?
rte_thread_init is EAL private for now.
That saves us some code in this function, but we will call with a !=
NULL cpuset in all other EAL code.


-- 
David Marchand