[dpdk-dev] [PATCH 1/2] mempool: indicate the usages of multi memzones

Olivier MATZ olivier.matz at 6wind.com
Tue Dec 19 12:02:06 CET 2017


On Tue, Dec 19, 2017 at 04:16:33PM +0530, Hemant Agrawal wrote:
> Hi Olivier,
> 
> On 12/19/2017 3:54 PM, Olivier MATZ wrote:
> > Hi Hemant,
> > 
> > On Wed, Dec 06, 2017 at 06:01:12PM +0530, Hemant Agrawal wrote:
> > > This is required for the optimizations w.r.t hw mempools.
> > > They will use different kind of optimizations if the buffers
> > > are from single contiguous memzone.
> > > 
> > > Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
> > > ---
> > >  lib/librte_mempool/rte_mempool.c | 7 +++++--
> > >  lib/librte_mempool/rte_mempool.h | 5 +++++
> > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> > > index d50dba4..9d3737c 100644
> > > --- a/lib/librte_mempool/rte_mempool.c
> > > +++ b/lib/librte_mempool/rte_mempool.c
> > > @@ -387,13 +387,16 @@ rte_mempool_populate_iova(struct rte_mempool *mp, char *vaddr,
> > >  	total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size;
> > > 
> > >  	/* Detect pool area has sufficient space for elements */
> > > -	if (mp->flags & MEMPOOL_F_CAPA_PHYS_CONTIG) {
> > > -		if (len < total_elt_sz * mp->size) {
> > > +	if (len < total_elt_sz * mp->size) {
> > > +		if (mp->flags & MEMPOOL_F_CAPA_PHYS_CONTIG) {
> > >  			RTE_LOG(ERR, MEMPOOL,
> > >  				"pool area %" PRIx64 " not enough\n",
> > >  				(uint64_t)len);
> > >  			return -ENOSPC;
> > >  		}
> > > +	} else {
> > > +		/* Memory will be allocated from multiple memzones */
> > > +		mp->flags |= MEMPOOL_F_MULTI_MEMZONE;
> > >  	}
> > > 
> > >  	memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0);
> > > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> > > index 721227f..394a4fe 100644
> > > --- a/lib/librte_mempool/rte_mempool.h
> > > +++ b/lib/librte_mempool/rte_mempool.h
> > > @@ -292,6 +292,11 @@ struct rte_mempool {
> > >   */
> > >  #define MEMPOOL_F_CAPA_BLK_ALIGNED_OBJECTS 0x0080
> > > 
> > > +/* Indicates that the mempool buffers are allocated from multiple memzones
> > > + * the buffer may or may not be physically contiguous.
> > > + */
> > > +#define MEMPOOL_F_MULTI_MEMZONE 0x0100
> > > +
> > >  /**
> > >   * @internal When debug is enabled, store some statistics.
> > >   *
> > > --
> > > 2.7.4
> > > 
> > 
> > I'm not confortable with adding more and more flags, as I explained
> > here: http://dpdk.org/ml/archives/dev/2017-December/083909.html
> 
> This particular flag is not about how to populate mempool. This is just
> indicating how the mempool was populated - a status flag. This information
> is just helpful for the PMDs.
> 
> At least I am not able to see that this particular flag is being very driver
> specific.

That's true, I commented too fast :)
And what about using mp->nb_mem_chunks instead? Would it do the job
in your use-case?


> > It makes the generic code very complex, and probably buggy (many
> > flags are incompatible with other flags).
> > 
> > I'm thinking about moving the populate_* functions in the drivers
> > (this is described a bit more in the link above). What do you think
> > about this approach?
> > 
> 
> The idea is good and it will give fine control to the individual mempools to
> populate the memory the way they want. However, on the downside, it will
> also lead to lot of duplicate code or similar code. It may also lead to a
> maintenance issue for the mempool PMD owner.

Yes, that will be the drawback. If we do this, we should try to keep some
common helpers in the mempool lib.



More information about the dev mailing list