> From: Konstantin Ananyev [mailto:[email protected]] > Sent: Friday, 18 September 2026 12.02 > > > > Swapped the location of the private data and the local cache, > > so the private data is located immediately after the mempool header, > > and the local cache after that. > > This way, getting the address of the private data is as simple as > > adding a constant to the address of the mempool. > > > > The local cache is accessed by dereferencing a pointer to it anyway, > so > > the performance of accessing it is not affected by moving its > location. > > In theory, if we keep current layout, we can access local cache by just > pointer > arithmetic, without actual pointer de-referencing.
Yes, I explored that option in a previous patch [1], but self-rejected it. The downside to that option is that memory for local cache (RTE_MAX_LCORE * 512 pointers) is always allocated, also for mempools without cache. I prefer this series with variable size cache; the cache is no longer limited to 512 objects, but can be any size. And since we have to fetch the size multiplier anyway, fetching the pointer does not require additional memory load operations (they are in the same cache line). Also, mbuf mempools use the mempool private data for information about the mbuf buffer size (fetched when resetting mbufs), and for mbufs with external buffer. So direct access to the mempool private data is beneficial for the mbuf library. (Although the actual performance difference is probably insignificant between the two ways of getting the address of the mempool private data.) [1]: https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/ > I am not oppose to that change - it does make sense to me, > and I don't know would it make any real difference in terms of > performance > (my guess - it wouldn't). > Just another option to consider. In theory pointer arithmetic should be faster (assuming fixed-size cache). But when I experimented with accessing the local cache by pointer arithmetic instead of pointer dereferencing, I didn't observe any performance difference. I guess the pointer is sufficiently hot in the cache, so the cost of dereferencing it ("cache = mp->local_cache_ptr") is similar to the cost of adding the constant offset to the mempool cache ("cache = &mp->local_cache_array"). So the performance benefit turned out to be insignificant, and I didn't feel bad about abandoning my "Access local cache without first accessing the mempool header struct" optimization. > > > > > Note: > > The mempool private data API describes the private data as following > the > > mempool header; but it has been implemented differently for a long > time > > without causing problems, so this is considered an optimization, > > not a bugfix. > > > > Signed-off-by: Morten Brørup <[email protected]> > > --- > > Supersedes: patch-169235 > > ("[v2] mempool: no cache size limit") > > --- > > app/test/test_mempool.c | 3 +-- > > lib/mempool/rte_mempool.c | 17 +++++++++++++---- > > lib/mempool/rte_mempool.h | 20 ++++++-------------- > > 3 files changed, 20 insertions(+), 20 deletions(-) > > > > diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c > > index 0bb051cb31..6ff8746474 100644 > > --- a/app/test/test_mempool.c > > +++ b/app/test/test_mempool.c > > @@ -112,8 +112,7 @@ test_mempool_basic(struct rte_mempool *mp, int > > use_external_cache) > > GOTO_ERR(ret, out); > > > > printf("get private data\n"); > > - if (rte_mempool_get_priv(mp) != (char *)mp + > > - RTE_MEMPOOL_HEADER_SIZE(mp, mp->cache_size)) > > + if (rte_mempool_get_priv(mp) != (char *)mp + sizeof(struct[] > > I'd say - remove that completely. > From my perspective it is wrong to make assumptions on internal > structure layout. Agree. The size is fixed now, so this test has become superfluous anyway. But it's harmless, so I'll leave it there (for now). > > > rte_mempool)) > > GOTO_ERR(ret, out); > > > > #ifndef RTE_EXEC_ENV_FREEBSD /* rte_mem_virt2iova() not supported on > bsd > > */ > > diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c > > index 04766f55d6..211763aced 100644 > > --- a/lib/mempool/rte_mempool.c > > +++ b/lib/mempool/rte_mempool.c > > @@ -873,6 +873,13 @@ rte_mempool_create_empty(const char *name, > > unsigned n, unsigned elt_size, > > * cache-aligned > > */ > > private_data_size = RTE_CACHE_LINE_ROUNDUP(private_data_size); > > + /* > > + * If any private data, add padding, to guard against false > sharing-like > > + * effects on systems with a next-N-lines hardware prefetcher, > when > > + * accessing private data. > > + */ > > + if (private_data_size != 0) > > + private_data_size += RTE_CACHE_GUARD_LINES * > > RTE_CACHE_LINE_SIZE; > > > > /* try to allocate tailq entry */ > > te = rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0); > > @@ -881,8 +888,10 @@ rte_mempool_create_empty(const char *name, > > unsigned n, unsigned elt_size, > > goto exit_unlock; > > } > > > > - mempool_size = RTE_MEMPOOL_HEADER_SIZE(mp, cache_size); > > + mempool_size = sizeof(struct rte_mempool); > > mempool_size += private_data_size; > > + if (cache_size != 0) > > + mempool_size += RTE_MAX_LCORE * sizeof(struct > > rte_mempool_cache); > > > > ret = snprintf(mz_name, sizeof(mz_name), > > RTE_MEMPOOL_MZ_FORMAT, name); > > if (ret < 0 || ret >= (int)sizeof(mz_name)) { > > @@ -896,7 +905,7 @@ rte_mempool_create_empty(const char *name, > > unsigned n, unsigned elt_size, > > > > /* init the mempool structure */ > > mp = mz->addr; > > - memset(mp, 0, RTE_MEMPOOL_HEADER_SIZE(mp, cache_size)); > > + memset(mp, 0, mempool_size); > > ret = strlcpy(mp->name, name, sizeof(mp->name)); > > if (ret < 0 || ret >= (int)sizeof(mp->name)) { > > rte_errno = ENAMETOOLONG; > > @@ -935,10 +944,10 @@ rte_mempool_create_empty(const char *name, > > unsigned n, unsigned elt_size, > > > > /* > > * local_cache pointer is set even if cache_size is zero. > > - * The local_cache points to just past the elt_pa[] array. > > + * The local_cache points to just past the private data. > > */ > > mp->local_cache = (struct rte_mempool_cache *) > > - RTE_PTR_ADD(mp, RTE_MEMPOOL_HEADER_SIZE(mp, 0)); > > + RTE_PTR_ADD(mp, sizeof(struct rte_mempool) + > > private_data_size); > > > > /* Init all default caches. */ > > if (cache_size != 0) { > > diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h > > index 4cdbb3f778..5a81e53a9f 100644 > > --- a/lib/mempool/rte_mempool.h > > +++ b/lib/mempool/rte_mempool.h > > @@ -278,6 +278,11 @@ struct __rte_cache_aligned rte_mempool { > > */ > > struct rte_mempool_debug_stats stats[RTE_MAX_LCORE + 1]; > > #endif > > + > > + /* > > + * Private data, if any, is located after the mempool header. > > + * Per-lcore local cache, if any, is located after the private > data. > > + */ > > }; > > > > /** Spreading among memory channels not required. */ > > @@ -369,18 +374,6 @@ struct __rte_cache_aligned rte_mempool { > > #define RTE_MEMPOOL_CACHE_STAT_ADD(cache, name, n) do {} while (0) > > #endif > > > > -/** > > - * @internal Calculate the size of the mempool header. > > - * > > - * @param mp > > - * Pointer to the memory pool. > > - * @param cs > > - * Size of the per-lcore cache. > > - */ > > -#define RTE_MEMPOOL_HEADER_SIZE(mp, cs) \ > > - (sizeof(*(mp)) + (((cs) == 0) ? 0 : \ > > - (sizeof(struct rte_mempool_cache) * RTE_MAX_LCORE))) > > - > > /* return the header of a mempool object (internal) */ > > static inline struct rte_mempool_objhdr * > > rte_mempool_get_header(void *obj) > > @@ -1902,8 +1895,7 @@ void rte_mempool_audit(struct rte_mempool *mp); > > */ > > static inline void *rte_mempool_get_priv(struct rte_mempool *mp) > > { > > - return (char *)mp + > > - RTE_MEMPOOL_HEADER_SIZE(mp, mp->cache_size); > > + return (void *)(mp + 1); > > } > > > > /** > > -- > > 2.43.0
