On Tue, Jun 23, 2020 at 11:14 AM Bruce Richardson
<bruce.richard...@intel.com> wrote:
>
> On Tue, Jun 23, 2020 at 09:49:18AM +0200, David Marchand wrote:
> > Hello Konstantin,
> >
> > On Mon, Jun 22, 2020 at 5:49 PM Ananyev, Konstantin
> > <konstantin.anan...@intel.com> wrote:
> > > > 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?
> >
> I think that is reasonable for a new feature. I suspect those wanting to
> dynamically manage their own threads probably do not care about
> multi-process mode.
>
> However, this limitation probably needs to be clearly called out in the
> docs.
Again, *disclaimer* I am not a user of the MP feature.
But I suppose users of such a feature are relying on DPDK init and
threads management, and I would not expect them to use this new API.
I will add a note in rte_thread_register() doxygen.
But I wonder if adding some check when a secondary attaches would make sense...
Like how we have a version check, I could "taint" the dpdk primary
process: a variable in shared memory could do the trick.
--
David Marchand
