Re: [PATCH net-next v1 1/2] page_pool: reintroduce page_pool_unlink_napi()

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 25 Jun 2024 12:55:21 -0700 David Wei wrote:
>  #ifdef CONFIG_PAGE_POOL
> +void page_pool_unlink_napi(struct page_pool *pool);
>  void page_pool_destroy(struct page_pool *pool);
>  void page_pool_use_xdp_mem(struct page_pool *pool, void (*disconnect)(void *),
>  			   const struct xdp_mem_info *mem);
>  void page_pool_put_page_bulk(struct page_pool *pool, void **data,
>  			     int count);
>  #else
> +static inline void page_pool_unlink_napi(struct page_pool *pool)
> +{
> +}

All callers must select PAGE_POOL, I don't think we need the empty
static inline in this particular case.

>  static inline void page_pool_destroy(struct page_pool *pool)
>  {
>  }
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 3927a0a7fa9a..ec274dde0e32 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -1021,6 +1021,11 @@ static void page_pool_disable_direct_recycling(struct page_pool *pool)
>  	 */
>  	WRITE_ONCE(pool->cpuid, -1);
>  
> +	page_pool_unlink_napi(pool);

No need to split page_pool_disable_direct_recycling()
into two, we can write cpuid, it won't hurt.

The purpose of the function didn't really change when Olek
renamed it. Unlinking NAPI is also precisely to prevent recycling.
So you can either export page_pool_disable_direct_recycling()
add a wrapper called page_pool_unlink_napi(), or come up with
another name... But there's no need to split it.

> +}
> +
> +void page_pool_unlink_napi(struct page_pool *pool)
> +{
>  	if (!pool->p.napi)
>  		return;
>  
> @@ -1032,6 +1037,7 @@ static void p
-- 
pw-bot: cr