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.zh...@intel.com>
> > Signed-off-by: Xiaolong Ye <xiaolong...@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
