Re: [dpdk-dev] [PATCH 26/36] mempool: introduce a function to create an empty mempool

["Wiles, Keith" <keith.wiles@intel.com>]

>
>
>On 04/14/2016 05:57 PM, Wiles, Keith wrote:
>>> Introduce a new function rte_mempool_create_empty()
>>> that allocates a mempool that is not populated.
>>>
>>> The functions rte_mempool_create() and rte_mempool_xmem_create()
>>> now make use of it, making their code much easier to read.
>>> Currently, they are the only users of rte_mempool_create_empty()
>>> but the function will be made public in next commits.
>>>
>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>> +/* create an empty mempool */
>>> +static struct rte_mempool *
>>> +rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>>> +	unsigned cache_size, unsigned private_data_size,
>>> +	int socket_id, unsigned flags)
>>> {
>> 
>> When two processes need to use the same mempool, do we have a race condition with one doing a rte_mempool_create_empty() and the other process tries to use it when it finds that mempool before being fully initialized by the first process?
>> 
>
>I'm not an expert of the dpdk multiprocess model. But I would
>say that there are a lot of possible race conditions like this
>(ex: a port is created but not started), and I assume that
>applications doing multiprocess have their synchronization.
>
>If we really want a solution in mempool, we could:
>
>- remove the TAILQ_INSERT_TAIL() from rte_mempool_create()
>- create a new function rte_mempool_share() that adds the
>  mempool in the tailq for multiprocess. This function would
>  be called at the end of rte_mempool_create(), or by the
>  user if using rte_mempool_create_empty().
>
>I may be mistaking but I don't feel it's really required. Any
>comment from a multiprocess expert is welcome though.

Yes, I agree we should have the developer handle the multiprocess synchronization. The only thing I think we can do is provide a sync point API, but that is all I can think of to do.

Maybe instead of adding a fix for each place in DPDK, we require the developer to add the sync up when he has done all of the inits in his code or we provide one. Maybe a minor issue and we can ignore my comments for now.
>
>
>Regards,
>Olivier
>


Regards,
Keith