[dpdk-dev] [PATCH v1 8/9] mempool: ensure the mempool is initialized before populating

[Olivier Matz]

On Tue, Mar 20, 2018 at 04:32:04PM +0300, Andrew Rybchenko wrote:
> On 03/19/2018 08:06 PM, Olivier Matz wrote:
> > On Sat, Mar 10, 2018 at 03:39:41PM +0000, Andrew Rybchenko wrote:
> > > From: "Artem V. Andreev" <Artem.Andreev at oktetlabs.ru>
> > > 
> > > Callback to calculate required memory area size may require mempool
> > > driver data to be already allocated and initialized.
> > > 
> > > Signed-off-by: Artem V. Andreev <Artem.Andreev at oktetlabs.ru>
> > > Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
> > > ---
> > > RFCv2 -> v1:
> > >   - rename helper function as mempool_ops_alloc_once()
> > > 
> > >   lib/librte_mempool/rte_mempool.c | 29 ++++++++++++++++++++++-------
> > >   1 file changed, 22 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> > > index 844d907..12085cd 100644
> > > --- a/lib/librte_mempool/rte_mempool.c
> > > +++ b/lib/librte_mempool/rte_mempool.c
> > > @@ -322,6 +322,21 @@ rte_mempool_free_memchunks(struct rte_mempool *mp)
> > >   	}
> > >   }
> > > +static int
> > > +mempool_ops_alloc_once(struct rte_mempool *mp)
> > > +{
> > > +	int ret;
> > > +
> > > +	/* create the internal ring if not already done */
> > > +	if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
> > > +		ret = rte_mempool_ops_alloc(mp);
> > > +		if (ret != 0)
> > > +			return ret;
> > > +		mp->flags |= MEMPOOL_F_POOL_CREATED;
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > >   /* Add objects in the pool, using a physically contiguous memory
> > >    * zone. Return the number of objects added, or a negative value
> > >    * on error.
> > > @@ -336,13 +351,9 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
> > >   	struct rte_mempool_memhdr *memhdr;
> > >   	int ret;
> > > -	/* create the internal ring if not already done */
> > > -	if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
> > > -		ret = rte_mempool_ops_alloc(mp);
> > > -		if (ret != 0)
> > > -			return ret;
> > > -		mp->flags |= MEMPOOL_F_POOL_CREATED;
> > > -	}
> > > +	ret = mempool_ops_alloc_once(mp);
> > > +	if (ret != 0)
> > > +		return ret;
> > >   	/* mempool is already populated */
> > >   	if (mp->populated_size >= mp->size)
> > > @@ -515,6 +526,10 @@ rte_mempool_populate_default(struct rte_mempool *mp)
> > >   	unsigned mz_id, n;
> > >   	int ret;
> > > +	ret = mempool_ops_alloc_once(mp);
> > > +	if (ret != 0)
> > > +		return ret;
> > > +
> > >   	/* mempool must not be populated */
> > >   	if (mp->nb_mem_chunks != 0)
> > >   		return -EEXIST;
> > 
> > Is there a reason why we need to add it in
> > rte_mempool_populate_default() but not in rte_mempool_populate_virt() and
> > rte_mempool_populate_iova_tab()?
> 
> The reason is rte_mempool_ops_calc_mem_size() call
> from rte_mempool_populate_default(). rte_mempool_ops_*() are not
> called directly from rte_mempool_populate_virt() and
> rte_mempool_populate_iova_tab().
> 
> In fact I've found out that rte_mempool_ops_calc_mem_size() is called
> from get_anon_size() which is called from rte_mempool_populate_anon().
> So, we need to add to get_anon_size() as well.
> 
> May be it is even better to make the patch the first in series to make
> sure that it is already OK when rte_mempool_ops_calc_mem_size()
> is added. What do you think?

Yes, sounds good.


Olivier