Hi Cyril, > -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cyril Chemparathy > Sent: Monday, June 22, 2015 7:59 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 08/12] mempool: allow config override on > element alignment > > On TILE-Gx and TILE-Mx platforms, the buffers fed into the hardware > buffer manager require a 128-byte alignment. With this change, we > allow configuration based override of the element alignment, and > default to RTE_CACHE_LINE_SIZE if left unspecified. > > Change-Id: I9cd789d92b0bc9c8f44a633de59bb04d45d927a7 > Signed-off-by: Cyril Chemparathy <cchemparathy at ezchip.com> > --- > lib/librte_mempool/rte_mempool.c | 16 +++++++++------- > lib/librte_mempool/rte_mempool.h | 6 ++++++ > 2 files changed, 15 insertions(+), 7 deletions(-) > > diff --git a/lib/librte_mempool/rte_mempool.c > b/lib/librte_mempool/rte_mempool.c > index 002d3a8..7656b0f 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -120,10 +120,10 @@ static unsigned optimize_object_size(unsigned obj_size) > nrank = 1; > > /* process new object size */ > - new_obj_size = (obj_size + RTE_CACHE_LINE_MASK) / RTE_CACHE_LINE_SIZE; > + new_obj_size = (obj_size + RTE_MEMPOOL_ALIGN_MASK) / RTE_MEMPOOL_ALIGN; > while (get_gcd(new_obj_size, nrank * nchan) != 1) > new_obj_size++; > - return new_obj_size * RTE_CACHE_LINE_SIZE; > + return new_obj_size * RTE_MEMPOOL_ALIGN; > } > > static void > @@ -267,7 +267,7 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t > flags, > #endif > if ((flags & MEMPOOL_F_NO_CACHE_ALIGN) == 0) > sz->header_size = RTE_ALIGN_CEIL(sz->header_size, > - RTE_CACHE_LINE_SIZE); > + RTE_MEMPOOL_ALIGN); > > /* trailer contains the cookie in debug mode */ > sz->trailer_size = 0; > @@ -281,9 +281,9 @@ rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t > flags, > if ((flags & MEMPOOL_F_NO_CACHE_ALIGN) == 0) { > sz->total_size = sz->header_size + sz->elt_size + > sz->trailer_size; > - sz->trailer_size += ((RTE_CACHE_LINE_SIZE - > - (sz->total_size & RTE_CACHE_LINE_MASK)) & > - RTE_CACHE_LINE_MASK); > + sz->trailer_size += ((RTE_MEMPOOL_ALIGN - > + (sz->total_size & RTE_MEMPOOL_ALIGN_MASK)) & > + RTE_MEMPOOL_ALIGN_MASK); > } > > /* > @@ -498,7 +498,7 @@ rte_mempool_xmem_create(const char *name, unsigned n, > unsigned elt_size, > * cache-aligned > */ > private_data_size = (private_data_size + > - RTE_CACHE_LINE_MASK) & (~RTE_CACHE_LINE_MASK); > + RTE_MEMPOOL_ALIGN_MASK) & > (~RTE_MEMPOOL_ALIGN_MASK); > > if (! rte_eal_has_hugepages()) { > /* > @@ -525,6 +525,7 @@ rte_mempool_xmem_create(const char *name, unsigned n, > unsigned elt_size, > * enough to hold mempool header and metadata plus mempool objects. > */ > mempool_size = MEMPOOL_HEADER_SIZE(mp, pg_num) + private_data_size; > + mempool_size = RTE_ALIGN_CEIL(mempool_size, RTE_MEMPOOL_ALIGN); > if (vaddr == NULL) > mempool_size += (size_t)objsz.total_size * n; > > @@ -580,6 +581,7 @@ rte_mempool_xmem_create(const char *name, unsigned n, > unsigned elt_size, > /* calculate address of the first element for continuous mempool. */ > obj = (char *)mp + MEMPOOL_HEADER_SIZE(mp, pg_num) + > private_data_size; > + obj = RTE_PTR_ALIGN_CEIL(obj, RTE_MEMPOOL_ALIGN); > > /* populate address translation fields. */ > mp->pg_num = pg_num; > diff --git a/lib/librte_mempool/rte_mempool.h > b/lib/librte_mempool/rte_mempool.h > index 380d60b..9321b86 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -142,6 +142,12 @@ struct rte_mempool_objsz { > /** Mempool over one chunk of physically continuous memory */ > #define MEMPOOL_PG_NUM_DEFAULT 1 > > +#ifndef RTE_MEMPOOL_ALIGN > +#define RTE_MEMPOOL_ALIGN RTE_CACHE_LINE_SIZE > +#endif > + > +#define RTE_MEMPOOL_ALIGN_MASK (RTE_MEMPOOL_ALIGN - 1)
I am probably a bit late with my comments, but why not make it a runtime decision then? I know we can't add a new parameter to mempool_xmem_create() without ABI breakage, but we can make some global variable for now, that could be setup at init time or something similar. > + > /** > * Mempool object header structure > * > -- > 2.1.2