Re: [RFC v4 03/18] page_pool: use netmem alloc/put APIs in __page_pool_alloc_page_order()

Pavel Begunkov
2025-06-05T10:25:42Z

On 6/4/25 03:52, Byungchul Park wrote:
> Use netmem alloc/put APIs instead of page alloc/put APIs and make it
> return netmem_ref instead of struct page * in
> __page_pool_alloc_page_order().
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> ---
> net/core/page_pool.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 4011eb305cee..523354f2db1c 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -518,29 +518,29 @@ static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t g
> return false;
> }
>
> -static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
> - gfp_t gfp)
> +static netmem_ref __page_pool_alloc_page_order(struct page_pool *pool,
> + gfp_t gfp)
> {
> - struct page *page;
> + netmem_ref netmem;
>
> gfp |= __GFP_COMP;
> - page = alloc_pages_node(pool->p.nid, gfp, pool->p.order);
> - if (unlikely(!page))
> - return NULL;
> + netmem = alloc_netmems_node(pool->p.nid, gfp, pool->p.order);
> + if (unlikely(!netmem))
> + return 0;
>
> - if (pool->dma_map && unlikely(!page_pool_dma_map(pool, page_to_netmem(page), gfp))) {
> - put_page(page);
> - return NULL;
> + if (pool->dma_map && unlikely(!page_pool_dma_map(pool, netmem, gfp))) {
> + put_netmem(netmem);

It's a bad idea to have {put,get}_netmem in page pool's code, it has a
different semantics from what page pool expects for net_iov. I.e.
instead of releasing the netmem and allowing it to be reallocated by
page pool, put_netmem(niov) will drop a memory provider reference and
leak the net_iov. Depending on implementation it might even underflow
mp refs if a net_iov is ever passed here.

The second problem is that you pass it page_pool_dma_map(), which
works only with struct page and not net_iov, and so it just
unconditionally casts it back to struct page. Which, to be fair,
a pre-existent issue.

This function deals with pages, can we just use pages instead and
cast to netmem when needed? Sth like in this pseudo code:

netmem_ref __page_pool_alloc_page_order() {
struct page *p = alloc_pages_order();

netmem = page_to_netmem(p);
if (!page_pool_dma_map(netmem)) {
put_page(p);
return 0;
}
return netmem;
}

--
Pavel Begunkov