Hi David,
> > > 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.
Agree, it is not a new problem, it has been there for a while.
Though making lcore assignment dynamic will make it more noticeable and harder
to avoid.
With static only lcore distribution it is much easier to control things.
> 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?
In theory it is possible to mark this new API as not supported for MP.
At least for now. Though I think it is sort of temporal solution.
AFAIK, MP is used by customers, so sooner or later someone will hit that
problem.
Let say, we do have pdump app/library in our mainline.
As I can see - it will be affected when users will start using this new dynamic
lcore API
inside their apps.
>
> >
> > > + 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.
Right, didn't read the code properly.
Please scratch that comment.
>
> 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.
