Re: [PATCH iwl-next v3 0/2] libeth, idpf: use GFP_KERNEL for the initial Rx ring fill

[Alexander Lobakin]

From: Luigi Rizzo <lrizzo@xxxxxxxxxx>
Date: Mon, 9 Nov 2026 15:53:55 +0100

> On Thu, Nov 5, 2026 at 7:08 PM Alexander Lobakin
> <aleksander.lobakin@xxxxxxxxx> wrote:
>>
>> From: Luigi Rizzo <lrizzo@xxxxxxxxxx>
>> Date: Tue,  3 Nov 2026 16:23:23 +0000
>>
>>> libeth_rx_alloc() always allocates with GFP_ATOMIC, including when a
>>> driver fills its Rx rings at open or on queue reconfiguration. That
>>> fill runs in process context, right after page_pool_create() and before
>>> the queues' NAPI is enabled, so it may sleep, and GFP_ATOMIC is
>>> needlessly restrictive there: it can neither reclaim nor compact, while
>>> the initial fill can allocate tens of thousands of buffers across all
>>> queues.
>>>
>>> Patch 1 adds __libeth_rx_alloc() taking a gfp_t parameter and turns
>>> libeth_rx_alloc() into its GFP_ATOMIC wrapper.
>>> Patch 2 switches idpf's initial Rx ring fill (both splitq and singleq)
>>> to GFP_KERNEL.
>>>
>>> v3:
>>>   - rename libeth_rx_alloc_gfp() to __libeth_rx_alloc() and put the gfp
>>>     argument on its own line in page_pool_alloc_netmem() (Alexander Lobakin)
>>> v2: https://lore.kernel.org/r/20261019210604.2433620-1-lrizzo@xxxxxxxxxx/
>>>   - split into two patches (libeth helper and idpf consumer)
>>> v1: https://lore.kernel.org/r/20261016151835.2500807-1-lrizzo@xxxxxxxxxx/
>>>
>>> Luigi Rizzo (2):
>>>   libeth: add __libeth_rx_alloc() to specify allocation flags
>>>   idpf: use GFP_KERNEL for the initial Rx ring fill
>>
>> For the series:
>>
>> Reviewed-by: Alexander Lobakin <aleksander.lobakin@xxxxxxxxx>
> 
> Looking further at the problem, gemini pointed out that libeth_rx_alloc()
> is called in a way that makes it easy and robust to tell when it
> can use GFP_KERNEL. This would greatly simplify the patch.
> 
> Alexander, do you like this version better perhaps ?
> 
> cheers
> luigi
> 
> diff --git a/include/net/libeth/rx.h b/include/net/libeth/rx.h
> index 5d991404845e6..38c0e5066f0ef 100644
> --- a/include/net/libeth/rx.h
> +++ b/include/net/libeth/rx.h
> @@ -104,10 +104,17 @@ void libeth_rx_fq_destroy(struct libeth_fq *fq);
>  static inline dma_addr_t libeth_rx_alloc(const struct libeth_fq_fp *fq, u32 i)
>  {
>         struct libeth_fqe *buf = &fq->fqes[i];
> +       /* fq->fqes is zero-allocated and pre-filled in process context for
> +        * indices [0 .. fq->count - 2]; slot fq->count - 1 is first allocated
> +        * in NAPI. Use GFP_KERNEL for initial pre-fill and GFP_ATOMIC after.
> +        */
> +       gfp_t gfp = (buf->truesize || i == fq->count - 1) ?
> +                   GFP_ATOMIC : GFP_KERNEL;

The fact that `fq->count - 1` is only allocated from the NAPI context is
idpf's detail that doesn't belong to libeth.
Even though this is way shorter diffstat than the original series, I
prefer the original as it doesn't change anything for drivers other than
idpf and allows specifying GFP flags manually without such guesses. This
change introduces new branches on hotpath for each driver that uses libeth.
> 
>         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);
>         if (unlikely(!buf->netmem))
>                 return DMA_MAPPING_ERROR;

Thanks,
Olek