|SUCCESS| [v5] mempool: improve cache behaviour and performance

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163183

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-19

Commit: (git SHA needed)
Author: Morten Brorup <mb at smartsharesystems.com>
Subject: [PATCH v5] mempool: improve cache behaviour and performance

This patch refactors the mempool cache to eliminate unexpected behavior
and reduce cache miss rate by changing flush/refill levels and fixing
capacity checks.

> diff --git a/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c b/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c
> index 9af275cd9d..dd2263b8d7 100644
> --- a/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c
> +++ b/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c
> @@ -148,15 +148,31 @@ idpf_singleq_rearm(struct idpf_rx_queue *rxq)
>  	/* Can this be satisfied from the cache? */
>  	if (cache->len < IDPF_RXQ_REARM_THRESH) {
>  		/* No. Backfill the cache first, and then fill from it */
> -		uint32_t req = IDPF_RXQ_REARM_THRESH + (cache->size -
> -							cache->len);
>  
> -		/* How many do we require i.e. number to fill the cache + the request */
> +		/* Backfill would exceed the cache bounce buffer limit? */
> +		__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
> +		if (unlikely(cache->size / 2 < IDPF_RXQ_REARM_THRESH)) {
> +			idpf_singleq_rearm_common(rxq);
> +			return;
> +		}
> +
> +		/*
> +		 * Backfill the cache from the backend;
> +		 * move up the hot objects in the cache to the top half of the cache,
> +		 * and fetch (size / 2) objects to the bottom of the cache.
> +		 */
> +		__rte_assume(cache->len < cache->size / 2);
> +		rte_memcpy(&cache->objs[cache->size / 2], &cache->objs[0],
> +				sizeof(void *) * cache->len);

Does this code have incorrect whitespace on the third line? The
alignment doesn't match the opening paren. The coding style guide
requires that continuation lines align with the opening delimiter.

> +		int ret = rte_mempool_ops_dequeue_bulk
> +				(rxq->mp, &cache->objs[0], cache->size / 2);

Does this code have incorrect whitespace in the function call? The
opening paren should be on the same line as the function name according
to the coding style guide.

> +		if (ret == 0) {
> +			cache->len += cache->size / 2;
> +		} else {
> +			/*
> +			 * No further action is required for roll back, as the objects moved
> +			 * in the cache were actually copied, and the cache remains intact.
> +			 */

Does this comment line exceed 79 characters? The first line of this
multi-line comment appears to be too long.

[ ... ]

> @@ -565,15 +581,31 @@ idpf_splitq_rearm(struct idpf_rx_queue *rx_bufq)
>  	/* Can this be satisfied from the cache? */
>  	if (cache->len < IDPF_RXQ_REARM_THRESH) {
>  		/* No. Backfill the cache first, and then fill from it */
> -		uint32_t req = IDPF_RXQ_REARM_THRESH + (cache->size -
> -							cache->len);
>  
> -		/* How many do we require i.e. number to fill the cache + the request */
> +		/* Backfill would exceed the cache bounce buffer limit? */
> +		__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
> +		if (unlikely(cache->size / 2 < IDPF_RXQ_REARM_THRESH)) {
> +			idpf_splitq_rearm_common(rx_bufq);
> +			return;
> +		}
> +
> +		/*
> +		 * Backfill the cache from the backend;
> +		 * move up the hot objects in the cache to the top half of the cache,
> +		 * and fetch (size / 2) objects to the bottom of the cache.
> +		 */
> +		__rte_assume(cache->len < cache->size / 2);
> +		rte_memcpy(&cache->objs[cache->size / 2], &cache->objs[0],
> +				sizeof(void *) * cache->len);

Does this code have incorrect whitespace in the rte_memcpy call?

> +		int ret = rte_mempool_ops_dequeue_bulk
> +				(rx_bufq->mp, &cache->objs[0], cache->size / 2);

Does this code have incorrect whitespace in the function call?

> +		if (ret == 0) {
> +			cache->len += cache->size / 2;
> +		} else {
> +			/*
> +			 * No further action is required for roll back, as the objects moved
> +			 * in the cache were actually copied, and the cache remains intact.
> +			 */

Does this comment line exceed 79 characters?

[ ... ]

> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index 3042d94c14..805b52cc58 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c

[ ... ]

> diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
> index 2e54fc4466..432c43ab15 100644
> --- a/lib/mempool/rte_mempool.h
> +++ b/lib/mempool/rte_mempool.h

[ ... ]

> @@ -1390,24 +1397,32 @@ rte_mempool_do_generic_put(struct rte_mempool *mp, void * const *obj_table,
>  	RTE_MEMPOOL_CACHE_STAT_ADD(cache, put_bulk, 1);
>  	RTE_MEMPOOL_CACHE_STAT_ADD(cache, put_objs, n);
>  
> -	__rte_assume(cache->flushthresh <= RTE_MEMPOOL_CACHE_MAX_SIZE * 2);
> -	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE * 2);
> -	__rte_assume(cache->len <= cache->flushthresh);
> -	if (likely(cache->len + n <= cache->flushthresh)) {
> +	__rte_assume(cache->size <= RTE_MEMPOOL_CACHE_MAX_SIZE);
> +	__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
> +	__rte_assume(cache->len <= RTE_MEMPOOL_CACHE_MAX_SIZE);
> +	__rte_assume(cache->len <= cache->size);
> +	if (likely(cache->len + n <= cache->size)) {
>  		/* Sufficient room in the cache for the objects. */
>  		cache_objs = &cache->objs[cache->len];
>  		cache->len += n;
> -	} else if (n <= cache->flushthresh) {
> +	} else if (n <= cache->size / 2) {
>  		/*
> -		 * The cache is big enough for the objects, but - as detected by
> -		 * the comparison above - has insufficient room for them.
> -		 * Flush the cache to make room for the objects.
> +		 * The number of objects is within the cache bounce buffer limit,
> +		 * but - as detected by the comparison above - the cache has
> +		 * insufficient room for them.
> +		 * Flush the cache to the backend to make room for the objects;
> +		 * flush (size / 2) objects from the bottom of the cache, where
> +		 * objects are less hot, and move down the remaining objects, which
> +		 * are more hot, from the upper half of the cache.

Does this comment line exceed 79 characters? The second to last line
appears too long.

>  		 */
> -		cache_objs = &cache->objs[0];
> -		rte_mempool_ops_enqueue_bulk(mp, cache_objs, cache->len);
> -		cache->len = n;
> +		__rte_assume(cache->len > cache->size / 2);
> +		rte_mempool_ops_enqueue_bulk(mp, &cache->objs[0], cache->size / 2);
> +		rte_memcpy(&cache->objs[0], &cache->objs[cache->size / 2],
> +				sizeof(void *) * (cache->len - cache->size / 2));

Does this code have incorrect whitespace in the rte_memcpy call?

> +		cache_objs = &cache->objs[cache->len - cache->size / 2];
> +		cache->len = cache->len - cache->size / 2 + n;
>  	} else {
> -		/* The request itself is too big for the cache. */
> +		/* The request itself is too big. */
>  		goto driver_enqueue_stats_incremented;
>  	}

[ ... ]