Re: [PATCH v2 17/24] page_pool: Convert page_pool_return_skb_page() to use netmem

[Jesper Dangaard Brouer <jbrouer@redhat.com>]

On 05/01/2023 22.46, Matthew Wilcox (Oracle) wrote:
> This function accesses the pagepool members of struct page directly,
> so it needs to become netmem.  Add page_pool_put_full_netmem() and
> page_pool_recycle_netmem().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   include/net/page_pool.h | 14 +++++++++++++-
>   net/core/page_pool.c    | 13 ++++++-------
>   2 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index fbb653c9f1da..126c04315929 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -464,10 +464,16 @@ static inline void page_pool_put_page(struct page_pool *pool,
>   }
>   
>   /* Same as above but will try to sync the entire area pool->max_len */
> +static inline void page_pool_put_full_netmem(struct page_pool *pool,
> +		struct netmem *nmem, bool allow_direct)
> +{
> +	page_pool_put_netmem(pool, nmem, -1, allow_direct);
> +}
> +
>   static inline void page_pool_put_full_page(struct page_pool *pool,
>   					   struct page *page, bool allow_direct)
>   {
> -	page_pool_put_page(pool, page, -1, allow_direct);
> +	page_pool_put_full_netmem(pool, page_netmem(page), allow_direct);
>   }
>   
>   /* Same as above but the caller must guarantee safe context. e.g NAPI */
> @@ -477,6 +483,12 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>   	page_pool_put_full_page(pool, page, true);
>   }
>   
> +static inline void page_pool_recycle_netmem(struct page_pool *pool,
> +					    struct netmem *nmem)
> +{
> +	page_pool_put_full_netmem(pool, nmem, true);
                                               ^^^^

It is not clear in what context page_pool_recycle_netmem() will be used,
but I think the 'true' (allow_direct=true) might be wrong here.

It is only in limited special cases (RX-NAPI context) we can allow
direct return to the RX-alloc-cache.

--Jesper
(cut rest of patch which looked fine)