[PATCH v2] mbuf: avoid temporary array for bulk free

[Konstantin Ananyev]

> rte_pktmbuf_free_bulk() currently stages freeable mbufs in a
> temporary array before returning them to their mempool. For flat
> packet arrays, this copies pointers even when the input array
> already contains contiguous freeable mbufs.
>
> Track contiguous same-pool runs in the input array and pass them
> directly to rte_mbuf_raw_free_bulk(). Flush at 64 entries to
> preserve the existing mempool-cache behavior. Also flush on NULL
> entries, retained mbufs, and pool changes. Keep the array-based
> fallback for chained packets.
>
> Add coverage for NULL entries, mixed mempools, shared and indirect
> mbufs, and a flat run followed by a chained packet.
>
> Signed-off-by: Nam Tran <hoangnamtran18122005 at gmail.com>
> Reviewed-by: Morten Brørup <mb at smartsharesystems.com>
> ---
> v2:
> - Cap same-pool runs at 64 to retain mempool-cache behavior.
> - Move the sanity check after the chained-packet fallback decision.
> - Add mixed flat/chained and indirect-mbuf regression cases.
> - Remove the earlier timing claim; the capped version has not yet
>    been benchmarked on x86 or with a cached mempool.
>
> Tested: ARM64 Linux debug build and mbuf_autotest passed.
>
>   app/test/test_mbuf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
>   lib/mbuf/rte_mbuf.c  | 74 ++++++++++++++++++++++++++++++++--
>   2 files changed, 165 insertions(+), 3 deletions(-)
>
> diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
> index db23259745..249e541173 100644
> --- a/app/test/test_mbuf.c
> +++ b/app/test/test_mbuf.c
> @@ -833,6 +833,100 @@ test_pktmbuf_pool_bulk(void)
>   		goto err;
>   	}
>   
> +	printf("Test bulk free with NULL entries.\n");
> +
> +	ret = rte_pktmbuf_alloc_bulk(pool, mbufs, 4);
> +	if (ret != 0)
> +		goto err;
> +
> +	m = mbufs[1];
> +	mbufs[1] = NULL;
> +	rte_pktmbuf_free_bulk(mbufs, 4);
> +	rte_pktmbuf_free(m);
> +
> +	if (!rte_mempool_full(pool)) {
> +		printf("mempool not full after NULL-entry bulk free\n");
> +		goto err;
> +	}
> +
> +	printf("Test bulk free with multiple pools.\n");
> +
> +	for (i = 0; i < 4; i++) {
> +		mbufs[i] = rte_pktmbuf_alloc((i & 1) ? pool2 : pool);
> +		if (mbufs[i] == NULL)
> +			goto err;
> +	}
> +
> +	rte_pktmbuf_free_bulk(mbufs, 4);
> +
> +	if (!(rte_mempool_full(pool) && rte_mempool_full(pool2))) {
> +		printf("mempools not full after mixed-pool bulk free\n");
> +		goto err;
> +	}
> +
> +	printf("Test bulk free with shared mbuf.\n");
> +
> +	ret = rte_pktmbuf_alloc_bulk(pool, mbufs, 4);
> +	if (ret != 0)
> +		goto err;
> +
> +	m = mbufs[1];
> +	rte_mbuf_refcnt_update(m, 1);
> +	rte_pktmbuf_free_bulk(mbufs, 4);
> +
> +	if (rte_mbuf_refcnt_read(m) != 1) {
> +		printf("shared mbuf reference count incorrect\n");
> +		goto err;
> +	}
> +
> +	rte_pktmbuf_free(m);
> +
> +	if (!rte_mempool_full(pool)) {
> +		printf("mempool not full after shared mbuf free\n");
> +		goto err;
> +	}
> +
> +	printf("Test bulk free of flat mbufs around a chain.\n");
> +
> +	ret = rte_pktmbuf_alloc_bulk(pool, mbufs, 5);
> +	if (ret != 0)
> +		goto err;
> +
> +	ret = rte_pktmbuf_chain(mbufs[1], mbufs[2]);
> +	if (ret != 0)
> +		goto err;
> +	mbufs[2] = mbufs[3];
> +	mbufs[3] = mbufs[4];
> +	rte_pktmbuf_free_bulk(mbufs, 4);
> +
> +	if (!rte_mempool_full(pool)) {
> +		printf("mempool not full after mixed flat and chain bulk free\n");
> +		goto err;
> +	}
> +
> +	printf("Test bulk free with an indirect mbuf.\n");
> +
> +	m = rte_pktmbuf_alloc(pool);
> +	if (m == NULL)
> +		goto err;
> +	mbufs[0] = rte_pktmbuf_alloc(pool);
> +	mbufs[1] = rte_pktmbuf_clone(m, pool);
> +	mbufs[2] = rte_pktmbuf_alloc(pool);
> +	if (mbufs[0] == NULL || mbufs[1] == NULL || mbufs[2] == NULL)
> +		goto err;
> +
> +	rte_pktmbuf_free_bulk(mbufs, 3);
> +	if (rte_mbuf_refcnt_read(m) != 1) {
> +		printf("indirect mbuf reference count incorrect\n");
> +		goto err;
> +	}
> +	rte_pktmbuf_free(m);
> +
> +	if (!rte_mempool_full(pool)) {
> +		printf("mempool not full after indirect mbuf bulk free\n");
> +		goto err;
> +	}
> +
>   	printf("Test bulk free of single long chain.\n");
>   
>   	/* Bulk allocate all mbufs in the pool, in one go. */
> diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> index 005bfaa573..99b2ea07cf 100644
> --- a/lib/mbuf/rte_mbuf.c
> +++ b/lib/mbuf/rte_mbuf.c
> @@ -555,9 +555,8 @@ __rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
>    */
>   #define RTE_PKTMBUF_FREE_PENDING_SZ 64
>   
> -/* Free a bulk of packet mbufs back into their original mempools. */
> -RTE_EXPORT_SYMBOL(rte_pktmbuf_free_bulk)
> -void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
> +static void
> +__rte_pktmbuf_free_bulk_fallback(struct rte_mbuf **mbufs, unsigned int count)
>   {
>   	struct rte_mbuf *m, *m_next, *pending[RTE_PKTMBUF_FREE_PENDING_SZ];
>   	unsigned int idx, nb_pending = 0;
> @@ -582,6 +581,75 @@ void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
>   		rte_mbuf_raw_free_bulk(pending[0]->pool, pending, nb_pending);
>   }
>   
> +/* Free a bulk of packet mbufs back into their original mempools. */
> +RTE_EXPORT_SYMBOL(rte_pktmbuf_free_bulk)
> +void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int count)
> +{
> +	struct rte_mempool *run_pool = NULL;
> +	unsigned int run_start = 0;
> +	unsigned int run_count = 0;
> +	unsigned int idx;
> +
> +	for (idx = 0; idx < count; idx++) {
> +		struct rte_mbuf *m = mbufs[idx];
> +
> +		if (unlikely(m == NULL)) {
> +			if (run_count != 0) {
> +				rte_mbuf_raw_free_bulk(run_pool,
> +						&mbufs[run_start], run_count);
> +				run_count = 0;
here (and in other similar places), I think you also has to update 
run_start also.
BTW, I don't think you need both run_count and run_start.
Knowing the run_count and idx is enough to deduce run_start (or 
visa-versa).
> +			}
> +			continue;
> +		}
> +
> +		/*
> +		 * Preserve the generic path for chained packets. No mbuf in
> +		 * this suffix has been modified yet.
> +		 */
> +		if (unlikely(m->next != NULL)) {
> +			if (run_count != 0)
> +				rte_mbuf_raw_free_bulk(run_pool,
> +						&mbufs[run_start], run_count);
> +
> +			__rte_pktmbuf_free_bulk_fallback(&mbufs[idx],
> +					count - idx);
> +			return;
> +		}
> +
> +		__rte_mbuf_sanity_check(m, 1);
> +		m = rte_pktmbuf_prefree_seg(m);
> +		if (unlikely(m == NULL)) {
> +			if (run_count != 0) {
> +				rte_mbuf_raw_free_bulk(run_pool,
> +						&mbufs[run_start], run_count);
> +				run_count = 0;
> +			}
> +			continue;
> +		}
> +
> +		if (run_count != 0 && m->pool != run_pool) {
> +			rte_mbuf_raw_free_bulk(run_pool,
> +					&mbufs[run_start], run_count);
> +			run_count = 0;
> +		}
> +
> +		if (run_count == 0) {
> +			run_pool = m->pool;
> +			run_start = idx;
> +		}
> +
> +		run_count++;
> +		if (run_count == RTE_PKTMBUF_FREE_PENDING_SZ) {
> +			rte_mbuf_raw_free_bulk(run_pool,
> +					&mbufs[run_start], run_count);
> +			run_count = 0;
> +		}
> +	}
> +
> +	if (run_count != 0)
> +		rte_mbuf_raw_free_bulk(run_pool, &mbufs[run_start], run_count);
> +}
> +
>   /* Creates a shallow copy of mbuf */
>   RTE_EXPORT_SYMBOL(rte_pktmbuf_clone)
>   struct rte_mbuf *