[dpdk-dev] [PATCH v7 3/5] lib/mempool: allow page size aligned mempool

Olivier Matz olivier.matz at 6wind.com
Fri Mar 29 18:42:48 CET 2019


Hi,

On Fri, Mar 29, 2019 at 01:37:17PM +0300, Andrew Rybchenko wrote:
> On 3/27/19 12:00 PM, Xiaolong Ye wrote:
> > Allow create a mempool with page size aligned base address.
> > 
> > Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> > Signed-off-by: Xiaolong Ye <xiaolong.ye at intel.com>
> > ---
> >   lib/librte_mempool/rte_mempool.c | 3 +++
> >   lib/librte_mempool/rte_mempool.h | 1 +
> >   2 files changed, 4 insertions(+)
> > 
> > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> > index 683b216f9..cfbb49ea5 100644
> > --- a/lib/librte_mempool/rte_mempool.c
> > +++ b/lib/librte_mempool/rte_mempool.c
> > @@ -543,6 +543,9 @@ rte_mempool_populate_default(struct rte_mempool *mp)
> >   		if (try_contig)
> >   			flags |= RTE_MEMZONE_IOVA_CONTIG;
> > +		if (mp->flags & MEMPOOL_CHUNK_F_PAGE_ALIGN)
> > +			align = RTE_MAX(align, (size_t)getpagesize());
> > +
> >   		mz = rte_memzone_reserve_aligned(mz_name, mem_size,
> >   				mp->socket_id, flags, align);
> > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h
> > index 7c9cd9a2f..47729f7c9 100644
> > --- a/lib/librte_mempool/rte_mempool.h
> > +++ b/lib/librte_mempool/rte_mempool.h
> > @@ -264,6 +264,7 @@ struct rte_mempool {
> >   #define MEMPOOL_F_POOL_CREATED   0x0010 /**< Internal: pool is created. */
> >   #define MEMPOOL_F_NO_IOVA_CONTIG 0x0020 /**< Don't need IOVA contiguous objs. */
> >   #define MEMPOOL_F_NO_PHYS_CONTIG MEMPOOL_F_NO_IOVA_CONTIG /* deprecated */
> > +#define MEMPOOL_CHUNK_F_PAGE_ALIGN     0x0040 /**< Chunk's base address is page aligned */
> 
> Now the define name better explains what happens. I would name it
> MEMPOOL_F_CHUNK_PAGE_ALIGN to have MEMPOOL_F_ prefix for all flags,
> but it is minor. More important is what is behind and I have doubts that it
> is
> a right way to go.
> 
> I have already asked what is the final goal (but agree that question is
> unclear).
> Personally I doubt that the final goal is just having chunk page aligned.
> 
> It looks like the patch makes an assumption on how chunk is sliced into
> objects/elements and the property of chunk address allows to achieve some
> goal with elements start address. It looks very fragile and easily breakable
> with other flags (or no flags).
> 
> Also prefix should be just "mempool:", not "lib/mempool".

+1
I agree with Andrew. Please see my comment in patch 4.


Regards,
Olivier


More information about the dev mailing list