Hi Keith, Looks good, thanks. Please find some comments below.
> [PATCH] mempool: Reduce rte_mempool structure size nit: we usually avoid uppercase letters in title On 02/03/2016 12:02 AM, Keith Wiles wrote: > diff --git a/lib/librte_mempool/rte_mempool.c > b/lib/librte_mempool/rte_mempool.c > index aff5f6d..bdf8e2e 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -450,15 +450,11 @@ rte_mempool_xmem_create(const char *name, unsigned n, > unsigned elt_size, > int page_size = getpagesize(); > > /* compilation-time checks */ > +#ifdef RTE_LIBRTE_MEMPOOL_DEBUG > RTE_BUILD_BUG_ON((sizeof(struct rte_mempool) & > RTE_CACHE_LINE_MASK) != 0); > -#if RTE_MEMPOOL_CACHE_MAX_SIZE > 0 > RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_cache) & > RTE_CACHE_LINE_MASK) != 0); > - RTE_BUILD_BUG_ON((offsetof(struct rte_mempool, local_cache) & > - RTE_CACHE_LINE_MASK) != 0); > -#endif > -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG I don't think the #ifdef RTE_LIBRTE_MEMPOOL_DEBUG should be moved. It should only protects the checks on stats which are enabled in debug mode. > @@ -194,10 +192,7 @@ struct rte_mempool { > > unsigned private_data_size; /**< Size of private data. */ > > -#if RTE_MEMPOOL_CACHE_MAX_SIZE > 0 > - /** Per-lcore local cache. */ > - struct rte_mempool_cache local_cache[RTE_MAX_LCORE]; > -#endif > + struct rte_mempool_cache *local_cache; /**< Per-lcore local cache */ > > #ifdef RTE_LIBRTE_MEMPOOL_DEBUG > /** Per-lcore statistics. */ As you noticed it in your initial mail, this changes the ABI. I think your patch justifies the ABI change, so I think it should follow the ABI change process described in dpdk/doc/guides/contributing/versioning.rst. >From what I understand of versioning.rst, these kind of changes requires a deprecation notice first, and will be integrated in next version. I don't think it's easy to keep a backward compat in this case, especially because the rte_mempool structure is used by several inlined functions. Regards, Olivier