Re: [dpdk-dev] [PATCH v3 6/9] eal: register non-EAL threads as lcores

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> 
> 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
> > @@ -6,12 +6,13 @@
> >  #include <limits.h>
> >  #include <string.h>
> >
> > -#include <rte_errno.h>
> > -#include <rte_log.h>
> > -#include <rte_eal.h>
> > -#include <rte_lcore.h>
> >  #include <rte_common.h>
> >  #include <rte_debug.h>
> > +#include <rte_eal.h>
> > +#include <rte_errno.h>
> > +#include <rte_lcore.h>
> > +#include <rte_log.h>
> > +#include <rte_spinlock.h>
> >
> >  #include "eal_private.h"
> >  #include "eal_thread.h"
> > @@ -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.

Actually sorry, I was wrong - rte_config is local.
So having lcore_lock local seems ok here.
Though then, I think another issue arises:
For MP case 2 processes might get the same lcore_id via this function. 
And, as I remember, rte_mempool cache is by default located in shared memory.
So two threads might end-up racing for the same mempool cache slot.
Same story probably about some other shared data that uses lcore_id as an index

> 
> > +	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);
> > +}