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.andr...@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.andr...@oktetlabs.ru>
> > > Signed-off-by: Andrew Rybchenko <arybche...@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
