Re: [PATCH iwl-next v2 1/2] libeth: add libeth_rx_alloc_gfp() to specify allocation flags

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Luigi Rizzo <lrizzo@google.com>
Date: Mon, 19 Oct 2026 21:06:03 +0000

> libeth_rx_alloc() calls page_pool_dev_alloc_netmem(), which always
> allocates with GFP_ATOMIC | __GFP_NOWARN. Drivers also use it to fill
> their Rx rings at open or on queue reconfiguration, which runs in
> process context before the queues' NAPI is enabled. There, the caller
> may sleep and GFP_ATOMIC is needlessly restrictive: it can neither
> reclaim nor compact, while the initial fill can allocate tens of
> thousands of buffers across all queues.
> 
> Add libeth_rx_alloc_gfp(), which takes a gfp_t parameter and passes
> gfp | __GFP_NOWARN to page_pool_alloc_netmem(), and make
> libeth_rx_alloc() its GFP_ATOMIC wrapper.

If you'd really like to go ahead (I'm still not really convinced, even
with "O(100k)" which is not that realistic), here's a couple style
wishes from me:

> 
> Signed-off-by: Luigi Rizzo <lrizzo@google.com>
> ---
>  include/net/libeth/rx.h | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/libeth/rx.h b/include/net/libeth/rx.h
> index 0e736846c5e8a..14a528ea8a075 100644
> --- a/include/net/libeth/rx.h
> +++ b/include/net/libeth/rx.h
> @@ -96,20 +96,23 @@ int libeth_rx_fq_create(struct libeth_fq *fq, void *napi_dev);
>  void libeth_rx_fq_destroy(struct libeth_fq *fq);
>  
>  /**
> - * libeth_rx_alloc - allocate a new Rx buffer
> + * libeth_rx_alloc_gfp - allocate a new Rx buffer
>   * @fq: fill queue to allocate for
>   * @i: index of the buffer within the queue
> + * @gfp: allocation flags: GFP_KERNEL where the caller may sleep (e.g. the
> + *	 initial fill of the ring), GFP_ATOMIC from NAPI
>   *
>   * Return: DMA address to be passed to HW for Rx on successful allocation,
>   * ```DMA_MAPPING_ERROR``` otherwise.
>   */
> -static inline dma_addr_t libeth_rx_alloc(const struct libeth_fq_fp *fq, u32 i)
> +static inline dma_addr_t libeth_rx_alloc_gfp(const struct libeth_fq_fp *fq,
> +					     u32 i, gfp_t gfp)

Mayb just __libeth_rx_alloc()?

>  {
>  	struct libeth_fqe *buf = &fq->fqes[i];
>  
>  	buf->truesize = fq->truesize;
> -	buf->netmem = page_pool_dev_alloc_netmem(fq->pp, &buf->offset,
> -						 &buf->truesize);
> +	buf->netmem = page_pool_alloc_netmem(fq->pp, &buf->offset,
> +					     &buf->truesize, gfp | __GFP_NOWARN);

And here I'd also place the gfp argument on a separate line.

>  	if (unlikely(!buf->netmem))
>  		return DMA_MAPPING_ERROR;
>  
> @@ -117,6 +120,18 @@ static inline dma_addr_t libeth_rx_alloc(const struct libeth_fq_fp *fq, u32 i)
>  	       fq->pp->p.offset;
>  }
>  
> +/**
> + * libeth_rx_alloc - allocate a new Rx buffer from NAPI context
> + * @fq: fill queue to allocate for
> + * @i: index of the buffer within the queue
> + *
> + * Return: as libeth_rx_alloc_gfp().
> + */
> +static inline dma_addr_t libeth_rx_alloc(const struct libeth_fq_fp *fq, u32 i)
> +{
> +	return libeth_rx_alloc_gfp(fq, i, GFP_ATOMIC);
> +}
> +
>  void libeth_rx_recycle_slow(netmem_ref netmem);
>  
>  /**

Thanks,
Olek