[PATCH 0/6] mempool: cleanup, fixes, improvements and optimizations

[PATCH 0/6] mempool: cleanup, fixes, improvements and optimizations

[Morten Brørup]

Consolidated mempool patches previously submitted individually into one
series, and added a couple more.

Patch 1
Fixed a minor bug regarding the private data size alignment.

Patch 2 and 3
Removed the deprecated cache flush threshold field.
Supersedes: patch-167311
("[v3] mempool: remove cache flush threshold field")

Patch 4
Annotated private cache alloc/free functions as such,
improved the ops registration and lookup functions, and
improved the audit functions, and
updated some comments.

Patch 5
Swapped the location of the private data and local cache.
Supersedes: patch-169235
("[v2] mempool: no cache size limit")

Patch 6
Rightsized the memory allocated for cache, and
eliminated the cache size max limit.
Supersedes: patch-169235
("[v2] mempool: no cache size limit")

Morten Brørup (6):
  mempool: fix private data area size alignment
  net/sxe2: do not use deprecated mempool cache field
  mempool: remove deprecated cache flush threshold field
  mempool: various cleanups
  mempool: optimize access to private data
  mempool: no cache size limit

 app/test/test_mempool.c                 |  17 ++-
 doc/guides/rel_notes/release_26_11.rst  |  16 +++
 drivers/net/sxe2/sxe2_txrx_vec_avx512.c |   2 +-
 lib/mempool/mempool_trace.h             |   1 -
 lib/mempool/rte_mempool.c               | 151 +++++++++++++-----------
 lib/mempool/rte_mempool.h               |  91 +++++++-------
 lib/mempool/rte_mempool_ops.c           |  30 +++--
 7 files changed, 178 insertions(+), 130 deletions(-)

-- 
2.43.0

[PATCH 1/6] mempool: fix private data area size alignment

[Morten Brørup]

Object size alignment should not affect alignment of mempool header size
or private data size, which are both cache aligned.
Reverted the parts of the patch that incorrectly changed this.

Fixes: 9f34c5a7abbd ("mempool: allow config override on element alignment")

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/mempool/rte_mempool.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 817e2b8dc1..925d21570a 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -873,9 +873,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	 * reserve a memory zone for this mempool: private data is
 	 * cache-aligned
 	 */
-	private_data_size = (private_data_size +
-			     RTE_MEMPOOL_ALIGN_MASK) & (~RTE_MEMPOOL_ALIGN_MASK);
-
+	private_data_size = RTE_CACHE_LINE_ROUNDUP(private_data_size);
 
 	/* try to allocate tailq entry */
 	te = rte_zmalloc("MEMPOOL_TAILQ_ENTRY", sizeof(*te), 0);
@@ -886,7 +884,6 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 
 	mempool_size = RTE_MEMPOOL_HEADER_SIZE(mp, cache_size);
 	mempool_size += private_data_size;
-	mempool_size = RTE_ALIGN_CEIL(mempool_size, RTE_MEMPOOL_ALIGN);
 
 	ret = snprintf(mz_name, sizeof(mz_name), RTE_MEMPOOL_MZ_FORMAT, name);
 	if (ret < 0 || ret >= (int)sizeof(mz_name)) {
-- 
2.43.0

[PATCH 2/6] net/sxe2: do not use deprecated mempool cache field

[Morten Brørup]

The driver was updated to use the mempool cache ``size`` field instead
of the deprecated ``flushthresh`` field; it contains the same value.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
Note:
This driver patch may be omitted if another driver patch eliminates
the use of the deprecated ``flushthresh`` field.

Supersedes: patch-167311
("[v3] mempool: remove cache flush threshold field")
---
 drivers/net/sxe2/sxe2_txrx_vec_avx512.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/sxe2/sxe2_txrx_vec_avx512.c b/drivers/net/sxe2/sxe2_txrx_vec_avx512.c
index a830c7a33b..2e680f4027 100644
--- a/drivers/net/sxe2/sxe2_txrx_vec_avx512.c
+++ b/drivers/net/sxe2/sxe2_txrx_vec_avx512.c
@@ -67,7 +67,7 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 		}
 		cache->len += rs_thresh;
 
-		if (cache->len >= cache->flushthresh) {
+		if (cache->len >= cache->size) {
 			(void)rte_mempool_ops_enqueue_bulk(mp,
 					&cache->objs[cache->size], cache->len - cache->size);
 			cache->len = cache->size;
-- 
2.43.0

[PATCH 3/6] mempool: remove deprecated cache flush threshold field

[Morten Brørup]

Removed the mempool cache flush threshold field (``flushthresh``),
which was obsolete and marked for removal.

Reduced the size of the mempool cache objects array; it was double the
required size.

Updated release notes accordingly.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
Supersedes: patch-167311
("[v3] mempool: remove cache flush threshold field")
---
 doc/guides/rel_notes/release_26_11.rst |  6 ++++++
 lib/mempool/mempool_trace.h            |  1 -
 lib/mempool/rte_mempool.c              |  1 -
 lib/mempool/rte_mempool.h              | 16 +++++-----------
 4 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 87c7e81bde..59b8514360 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -79,6 +79,8 @@ Removed Items
     ``rte_rib6_is_equal``
   * table: ``RTE_LPM_IPV6_ADDR_SIZE``
 
+* mempool: Removed the deprecated and obsolete ``flushthresh`` field from the ``rte_mempool_cache`` structure.
+
 
 API Changes
 -----------
@@ -95,6 +97,10 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* mempool: Updated the ``rte_mempool_cache`` structure as follows:
+  - Removed the deprecated and obsolete ``flushthresh`` field.
+  - Removed the ``unused`` field.
+  - Reduced the size of the ``objs`` array from ``RTE_MEMPOOL_CACHE_MAX_SIZE`` * 2 to ``RTE_MEMPOOL_CACHE_MAX_SIZE``.
 
 ABI Changes
 -----------
diff --git a/lib/mempool/mempool_trace.h b/lib/mempool/mempool_trace.h
index 23cda1473c..60e47cf67b 100644
--- a/lib/mempool/mempool_trace.h
+++ b/lib/mempool/mempool_trace.h
@@ -119,7 +119,6 @@ RTE_TRACE_POINT(
 	rte_trace_point_emit_i32(socket_id);
 	rte_trace_point_emit_ptr(cache);
 	rte_trace_point_emit_u32(cache->len);
-	rte_trace_point_emit_u32(cache->flushthresh);
 )
 
 RTE_TRACE_POINT(
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 925d21570a..03ecd01560 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -753,7 +753,6 @@ static void
 mempool_cache_init(struct rte_mempool_cache *cache, uint32_t size)
 {
 	cache->size = size;
-	cache->flushthresh = size; /* Obsolete; for API/ABI compatibility purposes only */
 	cache->len = 0;
 }
 
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 50d958c7c6..2fa70812d5 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -89,14 +89,14 @@ struct __rte_cache_aligned rte_mempool_debug_stats {
  */
 struct __rte_cache_aligned rte_mempool_cache {
 	uint32_t size;	      /**< Size of the cache */
-	uint32_t flushthresh; /**< Obsolete; for API/ABI compatibility purposes only */
 	uint32_t len;	      /**< Current cache count */
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
-	uint32_t unused;
 	/*
 	 * Alternative location for the most frequently updated mempool statistics (per-lcore),
 	 * providing faster update access when using a mempool cache.
+	 * Note: 16-byte aligned for optimal SIMD access, when updating pairs of counters.
 	 */
+	alignas(16)
 	struct {
 		uint64_t put_bulk;          /**< Number of puts. */
 		uint64_t put_objs;          /**< Number of objects successfully put. */
@@ -104,15 +104,9 @@ struct __rte_cache_aligned rte_mempool_cache {
 		uint64_t get_success_objs;  /**< Objects successfully allocated. */
 	} stats;                        /**< Statistics */
 #endif
-	/**
-	 * Cache objects
-	 *
-	 * Note:
-	 * Cache is allocated at double size for API/ABI compatibility purposes only.
-	 * When reducing its size at an API/ABI breaking release,
-	 * remember to add a cache guard after it.
-	 */
-	alignas(RTE_CACHE_LINE_SIZE) void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE * 2];
+	/** Cache objects */
+	alignas(RTE_CACHE_LINE_SIZE) void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE];
+	RTE_CACHE_GUARD;
 };
 
 /**
-- 
2.43.0

[PATCH 4/6] mempool: various cleanups

[Morten Brørup]

Removed audit call from dump function, where it doesn't belong.

Added malloc/dealloc attribute to private cache alloc/free functions.

Improved checks in mempool ops registration function.

Improved cache audit checks.

Added detailed description to definition of mempool name length.

Updated some comments, and moved around some misplaced comments.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
 app/test/test_mempool.c       | 12 +++++-----
 lib/mempool/rte_mempool.c     | 20 ++++++++---------
 lib/mempool/rte_mempool.h     | 42 ++++++++++++++++++++++++-----------
 lib/mempool/rte_mempool_ops.c | 30 +++++++++++++++++++------
 4 files changed, 67 insertions(+), 37 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index e54249ce61..0bb051cb31 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -191,7 +191,7 @@ static int test_mempool_creation_with_exceeded_cache_size(void)
 {
 	struct rte_mempool *mp_cov;
 
-	mp_cov = rte_mempool_create("test_mempool_cache_too_big",
+	mp_cov = rte_mempool_create("test_cache_too_big",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
 		RTE_MEMPOOL_CACHE_MAX_SIZE + 32, 0,
@@ -211,7 +211,7 @@ static int test_mempool_creation_with_invalid_flags(void)
 {
 	struct rte_mempool *mp_cov;
 
-	mp_cov = rte_mempool_create("test_mempool_invalid_flags", MEMPOOL_SIZE,
+	mp_cov = rte_mempool_create("test_invalid_flags", MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE, 0, 0,
 		NULL, NULL,
 		NULL, NULL,
@@ -333,7 +333,7 @@ test_mempool_sp_sc(void)
 
 	/* create a mempool with single producer/consumer ring */
 	if (mp_spsc == NULL) {
-		mp_spsc = rte_mempool_create("test_mempool_sp_sc", MEMPOOL_SIZE,
+		mp_spsc = rte_mempool_create("test_sp_sc", MEMPOOL_SIZE,
 			MEMPOOL_ELT_SIZE, 0, 0,
 			my_mp_init, NULL,
 			my_obj_init, NULL,
@@ -343,7 +343,7 @@ test_mempool_sp_sc(void)
 		if (mp_spsc == NULL)
 			RET_ERR();
 	}
-	if (rte_mempool_lookup("test_mempool_sp_sc") != mp_spsc) {
+	if (rte_mempool_lookup("test_sp_sc") != mp_spsc) {
 		printf("Cannot lookup mempool from its name\n");
 		ret = -1;
 		goto err;
@@ -440,7 +440,7 @@ test_mempool_same_name_twice_creation(void)
 {
 	struct rte_mempool *mp_tc, *mp_tc2;
 
-	mp_tc = rte_mempool_create("test_mempool_same_name", MEMPOOL_SIZE,
+	mp_tc = rte_mempool_create("test_same_name", MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE, 0, 0,
 		NULL, NULL,
 		NULL, NULL,
@@ -449,7 +449,7 @@ test_mempool_same_name_twice_creation(void)
 	if (mp_tc == NULL)
 		RET_ERR();
 
-	mp_tc2 = rte_mempool_create("test_mempool_same_name", MEMPOOL_SIZE,
+	mp_tc2 = rte_mempool_create("test_same_name", MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE, 0, 0,
 		NULL, NULL,
 		NULL, NULL,
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 03ecd01560..04766f55d6 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -1193,6 +1193,7 @@ mempool_obj_audit(struct rte_mempool *mp, __rte_unused void *opaque,
 	RTE_MEMPOOL_CHECK_COOKIES(mp, &obj, 1, 2);
 }
 
+/* check cookies before and after objects */
 static void
 mempool_audit_cookies(struct rte_mempool *mp)
 {
@@ -1209,11 +1210,10 @@ mempool_audit_cookies(struct rte_mempool *mp)
 #define mempool_audit_cookies(mp) do {} while(0)
 #endif
 
-/* check cookies before and after objects */
+/* check cache size consistency */
 static void
 mempool_audit_cache(const struct rte_mempool *mp)
 {
-	/* check cache size consistency */
 	unsigned lcore_id;
 
 	if (mp->cache_size == 0)
@@ -1222,10 +1222,13 @@ mempool_audit_cache(const struct rte_mempool *mp)
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		const struct rte_mempool_cache *cache;
 		cache = &mp->local_cache[lcore_id];
-		if (cache->len > RTE_DIM(cache->objs)) {
-			RTE_MEMPOOL_LOG(CRIT, "badness on cache[%u]",
-				lcore_id);
-			rte_panic("MEMPOOL: invalid cache len\n");
+		if (cache->size > RTE_DIM(cache->objs)) {
+			RTE_MEMPOOL_LOG(CRIT, "badness on cache[%u] size", lcore_id);
+			rte_panic("MEMPOOL: invalid cache[%u] size\n", lcore_id);
+		}
+		if (cache->len > cache->size) {
+			RTE_MEMPOOL_LOG(CRIT, "badness on cache[%u] len", lcore_id);
+			rte_panic("MEMPOOL: invalid cache[%u] len\n", lcore_id);
 		}
 	}
 }
@@ -1237,9 +1240,6 @@ rte_mempool_audit(struct rte_mempool *mp)
 {
 	mempool_audit_cache(mp);
 	mempool_audit_cookies(mp);
-
-	/* For case where mempool DEBUG is not set, and cache size is 0 */
-	RTE_SET_USED(mp);
 }
 
 /* dump the status of the mempool on the console */
@@ -1367,8 +1367,6 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 #else
 	fprintf(f, "  no statistics available\n");
 #endif
-
-	rte_mempool_audit(mp);
 }
 
 /* dump the status of all mempools on the console */
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 2fa70812d5..4cdbb3f778 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -120,7 +120,21 @@ struct rte_mempool_objsz {
 	/**< Total size of an object (header + elt + trailer). */
 };
 
-/**< Maximum length of a memory pool's name. */
+/**
+ * Maximum length of a memory pool's name.
+ *
+ * Note:
+ * Needs room for memzone prefix indicating "mempool" type:
+ * "MP_<name>"
+ * Furthermore, the mempool driver needs additional room for its own memzone prefix, e.g.:
+ * "RG_MP_<name>" (ring driver) or "STK_MP_<name>" (stack driver).
+ * In order to fail early on too long names, the length of the memzone name reserved
+ * by the default mempool driver (ring) is considered when creating the mempool.
+ * The length of the memzone name reserved by various other mempool drivers is
+ * not considered when creating the mempool.
+ * If the name eventually turns out to be too long for the chosen mempool driver,
+ * populating the mempool will fail.
+ */
 #define RTE_MEMPOOL_NAMESIZE (RTE_RING_NAMESIZE - \
 			      sizeof(RTE_MEMPOOL_MZ_PREFIX) + 1)
 #define RTE_MEMPOOL_MZ_PREFIX "MP_"
@@ -234,8 +248,7 @@ struct __rte_cache_aligned rte_mempool {
 	unsigned int flags;              /**< Flags of the mempool. */
 	int socket_id;                   /**< Socket id passed at create. */
 	uint32_t size;                   /**< Max size of the mempool. */
-	uint32_t cache_size;
-	/**< Size of per-lcore default local cache. */
+	uint32_t cache_size;             /**< Size of per-lcore default local cache. */
 
 	uint32_t elt_size;               /**< Size of an element. */
 	uint32_t header_size;            /**< Size of header (before elt). */
@@ -973,6 +986,8 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name,
  *   - >=0: Success; return the index of the ops struct in the table.
  *   - -EINVAL - some missing callbacks while registering ops struct.
  *   - -ENOSPC - the maximum number of ops structs has been reached.
+ *   - -ENAMETOOLONG - the name of the ops is too long.
+ *   - -EEXIST - the name of the ops is already registered.
  */
 int rte_mempool_register_ops(const struct rte_mempool_ops *ops);
 
@@ -1322,6 +1337,15 @@ void rte_mempool_stats_reset(struct rte_mempool *mp);
  */
 void rte_mempool_dump(FILE *f, struct rte_mempool *mp);
 
+/**
+ * Free a user-owned mempool cache.
+ *
+ * @param cache
+ *   A pointer to the mempool cache.
+ */
+void
+rte_mempool_cache_free(struct rte_mempool_cache *cache);
+
 /**
  * Create a user-owned mempool cache.
  *
@@ -1337,16 +1361,8 @@ void rte_mempool_dump(FILE *f, struct rte_mempool *mp);
  *   SOCKET_ID_ANY if there is no NUMA constraint for the reserved zone.
  */
 struct rte_mempool_cache *
-rte_mempool_cache_create(uint32_t size, int socket_id);
-
-/**
- * Free a user-owned mempool cache.
- *
- * @param cache
- *   A pointer to the mempool cache.
- */
-void
-rte_mempool_cache_free(struct rte_mempool_cache *cache);
+rte_mempool_cache_create(uint32_t size, int socket_id)
+	__rte_malloc __rte_dealloc(rte_mempool_cache_free, 1);
 
 /**
  * Get a pointer to the per-lcore default mempool cache.
diff --git a/lib/mempool/rte_mempool_ops.c b/lib/mempool/rte_mempool_ops.c
index 066bec36fc..6d99c7a25c 100644
--- a/lib/mempool/rte_mempool_ops.c
+++ b/lib/mempool/rte_mempool_ops.c
@@ -27,7 +27,7 @@ int
 rte_mempool_register_ops(const struct rte_mempool_ops *h)
 {
 	struct rte_mempool_ops *ops;
-	int16_t ops_index;
+	unsigned int ops_index;
 
 	rte_spinlock_lock(&rte_mempool_ops_table.sl);
 
@@ -47,12 +47,22 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
 		return -EINVAL;
 	}
 
-	if (strlen(h->name) >= sizeof(ops->name) - 1) {
+	if (strnlen(h->name, sizeof(h->name)) > sizeof(ops->name) - 1) {
 		rte_spinlock_unlock(&rte_mempool_ops_table.sl);
-		RTE_MEMPOOL_LOG(DEBUG, "%s(): mempool_ops <%s>: name too long",
-				__func__, h->name);
-		rte_errno = EEXIST;
-		return -EEXIST;
+		char name[sizeof(h->name) + 1];
+		strlcpy(name, h->name, sizeof(name));
+		RTE_MEMPOOL_LOG(ERR, "%s(): mempool_ops <%s>: name too long",
+				__func__, name);
+		return -ENAMETOOLONG;
+	}
+
+	for (ops_index = 0; ops_index < rte_mempool_ops_table.num_ops; ops_index++) {
+		if (!strcmp(h->name, rte_mempool_ops_table.ops[ops_index].name)) {
+			rte_spinlock_unlock(&rte_mempool_ops_table.sl);
+			RTE_MEMPOOL_LOG(ERR, "%s(): mempool_ops <%s>: name exists",
+					__func__, h->name);
+			return -EEXIST;
+		}
 	}
 
 	ops_index = rte_mempool_ops_table.num_ops++;
@@ -70,6 +80,8 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h)
 
 	rte_spinlock_unlock(&rte_mempool_ops_table.sl);
 
+	RTE_MEMPOOL_LOG(DEBUG, "Registered mempool ops <%s> at index %u", ops->name, ops_index);
+
 	return ops_index;
 }
 
@@ -185,8 +197,12 @@ rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name,
 		}
 	}
 
-	if (ops == NULL)
+	if (ops == NULL) {
+		RTE_MEMPOOL_LOG(DEBUG,
+				"Cannot set unknown mempool ops <%s>, of %u ops registered",
+				name, i);
 		return -EINVAL;
+	}
 
 	mp->ops_index = i;
 	mp->pool_config = pool_config;
-- 
2.43.0

[PATCH 5/6] mempool: optimize access to private data

[Morten Brørup]

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.

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 <mb@smartsharesystems.com>
---
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 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

[PATCH 6/6] mempool: no cache size limit

[Morten Brørup]

Replaced the fixed size object array in the per-lcore local cache
with a variable size array, allocated at mempool creation.
For faster indexing into the per-lcore array of caches, pre-calculate
the size (in bytes) of the per-lcore local cache.

Using a variable size array makes the RTE_MEMPOOL_CACHE_MAX_SIZE build
time configuration parameter superfluous,
but it was kept for compatibility purposes, as it is often used for
the cache_size parameter when creating mempools.

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
Supersedes: patch-169235
("[v2] mempool: no cache size limit")
---
 app/test/test_mempool.c                |   2 +-
 doc/guides/rel_notes/release_26_11.rst |  12 ++-
 lib/mempool/rte_mempool.c              | 118 ++++++++++++++-----------
 lib/mempool/rte_mempool.h              |  17 ++--
 4 files changed, 86 insertions(+), 63 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 6ff8746474..8a21b385d9 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -193,7 +193,7 @@ static int test_mempool_creation_with_exceeded_cache_size(void)
 	mp_cov = rte_mempool_create("test_cache_too_big",
 		MEMPOOL_SIZE,
 		MEMPOOL_ELT_SIZE,
-		RTE_MEMPOOL_CACHE_MAX_SIZE + 32, 0,
+		MEMPOOL_SIZE + 32, 0,
 		NULL, NULL,
 		my_obj_init, NULL,
 		SOCKET_ID_ANY, 0);
diff --git a/doc/guides/rel_notes/release_26_11.rst b/doc/guides/rel_notes/release_26_11.rst
index 59b8514360..64f6802e58 100644
--- a/doc/guides/rel_notes/release_26_11.rst
+++ b/doc/guides/rel_notes/release_26_11.rst
@@ -97,10 +97,20 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* mempool: When creating a mempool, the cache size can be freely specified
+  (although still not exceed the number of elements),
+  and is no longer limited by the ``RTE_MEMPOOL_CACHE_MAX_SIZE`` build time configuration parameter.
+  Although ``RTE_MEMPOOL_CACHE_MAX_SIZE`` has lost its original meaning,
+  it was kept for compatibility purposes,
+  as it is often used for the ``cache_size`` parameter when creating mempools.
+
+* mempool: Updated the ``rte_mempool`` structure as follows:
+  - Added the ``sizeof_cache_per_lcore`` field, for indexing into the per-lcore local cache.
+
 * mempool: Updated the ``rte_mempool_cache`` structure as follows:
   - Removed the deprecated and obsolete ``flushthresh`` field.
   - Removed the ``unused`` field.
-  - Reduced the size of the ``objs`` array from ``RTE_MEMPOOL_CACHE_MAX_SIZE`` * 2 to ``RTE_MEMPOOL_CACHE_MAX_SIZE``.
+  - Changed the ``objs`` array from fixed size to variable size.
 
 ABI Changes
 -----------
diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index 211763aced..069aabaa70 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -759,20 +759,21 @@ mempool_cache_init(struct rte_mempool_cache *cache, uint32_t size)
 /*
  * Create and initialize a cache for objects that are retrieved from and
  * returned to an underlying mempool. This structure is identical to the
- * local_cache[lcore_id] pointed to by the mempool structure.
+ * local_cache entry pointed to by the mempool structure.
  */
 RTE_EXPORT_SYMBOL(rte_mempool_cache_create)
 struct rte_mempool_cache *
 rte_mempool_cache_create(uint32_t size, int socket_id)
 {
 	struct rte_mempool_cache *cache;
+	size_t sizeof_cache = sizeof(struct rte_mempool_cache) + size * sizeof(void *);
 
-	if (size == 0 || size > RTE_MEMPOOL_CACHE_MAX_SIZE) {
+	if (size == 0 || sizeof_cache > UINT32_MAX) {
 		rte_errno = EINVAL;
 		return NULL;
 	}
 
-	cache = rte_zmalloc_socket("MEMPOOL_CACHE", sizeof(*cache),
+	cache = rte_zmalloc_socket("MEMPOOL_CACHE", sizeof_cache,
 				  RTE_CACHE_LINE_SIZE, socket_id);
 	if (cache == NULL) {
 		RTE_MEMPOOL_LOG(ERR, "Cannot allocate mempool cache.");
@@ -811,10 +812,9 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	struct rte_mempool *mp = NULL;
 	struct rte_tailq_entry *te = NULL;
 	const struct rte_memzone *mz = NULL;
-	size_t mempool_size;
+	size_t mempool_size, sizeof_cache_per_lcore;
 	unsigned int mz_flags = RTE_MEMZONE_1GB|RTE_MEMZONE_SIZE_HINT_ONLY;
 	struct rte_mempool_objsz objsz;
-	unsigned lcore_id;
 	int ret;
 
 	/* compilation-time checks */
@@ -822,6 +822,8 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 			  RTE_CACHE_LINE_MASK) != 0);
 	RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_cache) &
 			  RTE_CACHE_LINE_MASK) != 0);
+	RTE_BUILD_BUG_ON(offsetof(struct rte_mempool_cache, objs) !=
+			  sizeof(struct rte_mempool_cache));
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
 	RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_debug_stats) &
 			  RTE_CACHE_LINE_MASK) != 0);
@@ -838,7 +840,18 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	}
 
 	/* asked cache too big */
-	if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
+	sizeof_cache_per_lcore = 0;
+	if (cache_size != 0) {
+		sizeof_cache_per_lcore = sizeof(struct rte_mempool_cache);
+		sizeof_cache_per_lcore += RTE_CACHE_LINE_ROUNDUP(cache_size * sizeof(void *));
+		/*
+		 * Add padding, to guard against false sharing-like effects
+		 * on systems with a next-N-lines hardware prefetcher, when
+		 * accessing objects at the end of the cache.
+		 */
+		sizeof_cache_per_lcore += RTE_CACHE_GUARD_LINES * RTE_CACHE_LINE_SIZE;
+	}
+	if (sizeof_cache_per_lcore > UINT32_MAX ||
 	    cache_size > n) {
 		rte_errno = EINVAL;
 		return NULL;
@@ -890,8 +903,7 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_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);
+	mempool_size += RTE_MAX_LCORE * sizeof_cache_per_lcore;
 
 	ret = snprintf(mz_name, sizeof(mz_name), RTE_MEMPOOL_MZ_FORMAT, name);
 	if (ret < 0 || ret >= (int)sizeof(mz_name)) {
@@ -918,7 +930,6 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 	mp->elt_size = objsz.elt_size;
 	mp->header_size = objsz.header_size;
 	mp->trailer_size = objsz.trailer_size;
-	/* Size of default caches, zero means disabled. */
 	mp->cache_size = cache_size;
 	mp->private_data_size = private_data_size;
 	STAILQ_INIT(&mp->elt_list);
@@ -942,18 +953,17 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
 		goto exit_unlock;
 	}
 
-	/*
-	 * local_cache pointer is set even if cache_size is zero.
-	 * The local_cache points to just past the private data.
-	 */
-	mp->local_cache = (struct rte_mempool_cache *)
-		RTE_PTR_ADD(mp, sizeof(struct rte_mempool) + private_data_size);
-
-	/* Init all default caches. */
+	/* local_cache pointer is only set if per-lcore local cache is present */
 	if (cache_size != 0) {
-		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
-			mempool_cache_init(&mp->local_cache[lcore_id],
-					   cache_size);
+		mp->local_cache = (struct rte_mempool_cache *)
+			RTE_PTR_ADD(mp, sizeof(struct rte_mempool) + private_data_size);
+		mp->sizeof_cache_per_lcore = sizeof_cache_per_lcore;
+
+		/* Init all default caches. */
+		struct rte_mempool_cache *cache = mp->local_cache;
+		for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+				lcore_id++, cache = RTE_PTR_ADD(cache, sizeof_cache_per_lcore))
+			mempool_cache_init(cache, cache_size);
 	}
 
 	te->data = mp;
@@ -1016,16 +1026,17 @@ RTE_EXPORT_SYMBOL(rte_mempool_avail_count)
 unsigned int
 rte_mempool_avail_count(const struct rte_mempool *mp)
 {
-	unsigned count;
-	unsigned lcore_id;
+	unsigned int count;
 
 	count = rte_mempool_ops_get_count(mp);
 
-	if (mp->cache_size == 0)
+	if (mp->local_cache == NULL)
 		return count;
 
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
-		count += mp->local_cache[lcore_id].len;
+	const struct rte_mempool_cache *cache = mp->local_cache;
+	for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+			lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore))
+		count += cache->len;
 
 	/*
 	 * due to race condition (access to len is not locked), the
@@ -1053,11 +1064,11 @@ rte_mempool_stats_reset(struct rte_mempool *mp)
 
 #ifdef RTE_LIBRTE_MEMPOOL_STATS
 	memset(&mp->stats, 0, sizeof(mp->stats));
-	if (mp->cache_size != 0) {
-		for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-			memset(&mp->local_cache[lcore_id].stats, 0,
-					sizeof(mp->local_cache[lcore_id].stats));
-		}
+	if (mp->local_cache != NULL) {
+		struct rte_mempool_cache *cache = mp->local_cache;
+		for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+				lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore))
+			memset(&cache->stats, 0, sizeof(cache->stats));
 	}
 
 	RTE_MEMPOOL_LOG(DEBUG, "<%s>@%p: statistics reset", mp->name, mp);
@@ -1071,18 +1082,18 @@ rte_mempool_stats_reset(struct rte_mempool *mp)
 static unsigned
 rte_mempool_dump_cache(FILE *f, const struct rte_mempool *mp)
 {
-	unsigned lcore_id;
-	unsigned count = 0;
-	unsigned cache_count;
+	unsigned int count = 0;
 
 	fprintf(f, "  internal cache infos (hide zero value items):\n");
 	fprintf(f, "    cache_size=%"PRIu32"\n", mp->cache_size);
 
-	if (mp->cache_size == 0)
+	if (mp->local_cache == NULL)
 		return count;
 
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-		cache_count = mp->local_cache[lcore_id].len;
+	const struct rte_mempool_cache *cache = mp->local_cache;
+	for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+			lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore)) {
+		unsigned int cache_count = cache->len;
 		if (cache_count == 0)
 			continue;
 		fprintf(f, "    cache_count[%u]=%"PRIu32"\n",
@@ -1223,15 +1234,13 @@ mempool_audit_cookies(struct rte_mempool *mp)
 static void
 mempool_audit_cache(const struct rte_mempool *mp)
 {
-	unsigned lcore_id;
-
-	if (mp->cache_size == 0)
+	if (mp->local_cache == NULL)
 		return;
 
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-		const struct rte_mempool_cache *cache;
-		cache = &mp->local_cache[lcore_id];
-		if (cache->size > RTE_DIM(cache->objs)) {
+	const struct rte_mempool_cache *cache = mp->local_cache;
+	for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+			lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore)) {
+		if (cache->size > mp->size) {
 			RTE_MEMPOOL_LOG(CRIT, "badness on cache[%u] size", lcore_id);
 			rte_panic("MEMPOOL: invalid cache[%u] size\n", lcore_id);
 		}
@@ -1324,13 +1333,15 @@ rte_mempool_dump(FILE *f, struct rte_mempool *mp)
 		sum.get_success_blks += mp->stats[lcore_id].get_success_blks;
 		sum.get_fail_blks += mp->stats[lcore_id].get_fail_blks;
 	}
-	if (mp->cache_size != 0) {
+	if (mp->local_cache != NULL) {
 		/* Add the statistics stored in the mempool caches. */
-		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-			sum.put_bulk += mp->local_cache[lcore_id].stats.put_bulk;
-			sum.put_objs += mp->local_cache[lcore_id].stats.put_objs;
-			sum.get_success_bulk += mp->local_cache[lcore_id].stats.get_success_bulk;
-			sum.get_success_objs += mp->local_cache[lcore_id].stats.get_success_objs;
+		const struct rte_mempool_cache *cache = mp->local_cache;
+		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+			lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore)) {
+			sum.put_bulk += cache->stats.put_bulk;
+			sum.put_objs += cache->stats.put_objs;
+			sum.get_success_bulk += cache->stats.get_success_bulk;
+			sum.get_success_objs += cache->stats.get_success_objs;
 		}
 	}
 	fprintf(f, "  stats:\n");
@@ -1627,10 +1638,11 @@ mempool_info_cb(struct rte_mempool *mp, void *arg)
 				  mp->populated_size);
 
 	cache_count = 0;
-	if (mp->cache_size > 0) {
-		int lcore_id;
-		for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++)
-			cache_count += mp->local_cache[lcore_id].len;
+	if (mp->local_cache != NULL) {
+		const struct rte_mempool_cache *cache = mp->local_cache;
+		for (unsigned int lcore_id = 0; lcore_id < RTE_MAX_LCORE;
+				lcore_id++, cache = RTE_PTR_ADD(cache, mp->sizeof_cache_per_lcore))
+			cache_count += cache->len;
 	}
 	rte_tel_data_add_dict_uint(info->d, "total_cache_count", cache_count);
 	common_count = rte_mempool_ops_get_count(mp);
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 5a81e53a9f..5fc7cdb79c 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -105,8 +105,7 @@ struct __rte_cache_aligned rte_mempool_cache {
 	} stats;                        /**< Statistics */
 #endif
 	/** Cache objects */
-	alignas(RTE_CACHE_LINE_SIZE) void *objs[RTE_MEMPOOL_CACHE_MAX_SIZE];
-	RTE_CACHE_GUARD;
+	alignas(RTE_CACHE_LINE_SIZE) void *objs[];
 };
 
 /**
@@ -265,6 +264,7 @@ struct __rte_cache_aligned rte_mempool {
 	int32_t ops_index;
 
 	struct rte_mempool_cache *local_cache; /**< Per-lcore local cache */
+	uint32_t sizeof_cache_per_lcore; /**< Multiplier for indexing into the local cache. */
 
 	uint32_t populated_size;         /**< Number of populated objects. */
 	struct rte_mempool_objhdr_list elt_list; /**< List of objects in pool */
@@ -1050,8 +1050,7 @@ rte_mempool_free(struct rte_mempool *mp);
  * @param cache_size
  *   If cache_size is non-zero, the rte_mempool library will try to
  *   limit the accesses to the common lockless pool, by maintaining a
- *   per-lcore object cache. This argument must be lower or equal to
- *   RTE_MEMPOOL_CACHE_MAX_SIZE and n.
+ *   per-lcore object cache. This argument must be lower or equal to n.
  *   The access to the per-lcore table is of course
  *   faster than the multi-producer/consumer pool. The cache can be
  *   disabled if the cache_size argument is set to 0; it can be useful to
@@ -1371,15 +1370,17 @@ rte_mempool_cache_create(uint32_t size, int socket_id)
 static __rte_always_inline struct rte_mempool_cache *
 rte_mempool_default_cache(struct rte_mempool *mp, unsigned lcore_id)
 {
-	if (unlikely(mp->cache_size == 0))
+	if (unlikely(mp->local_cache == NULL))
 		return NULL;
 
 	if (unlikely(lcore_id == LCORE_ID_ANY))
 		return NULL;
 
-	rte_mempool_trace_default_cache(mp, lcore_id,
-		&mp->local_cache[lcore_id]);
-	return &mp->local_cache[lcore_id];
+	struct rte_mempool_cache *cache = (struct rte_mempool_cache *)RTE_PTR_ADD(mp->local_cache,
+			lcore_id * (size_t)mp->sizeof_cache_per_lcore);
+	rte_mempool_trace_default_cache(mp, lcore_id, cache);
+	__rte_assume(cache != NULL);
+	return cache;
 }
 
 /**
-- 
2.43.0