Re: [RFC PATCH v2 21/25] net/gve: Use GFP_KERNEL when pre-filling RX rings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] net/gve: sleep-in-atomic GFP_KERNEL alloc in NAPI poll during
  gve_turnup()
- [Medium] net/gve: gve_rx_alloc_buffer() uses GFP_ATOMIC on live config
  pre-fill
--

commit 1b07e24554dc827c263c1efbdbd4b2e0d6ee11bf
Author: Luigi Rizzo <lrizzo@google.com>

net/gve: Use GFP_KERNEL when pre-filling RX rings

This commit uses GFP_KERNEL instead of GFP_ATOMIC when allocating RX ring
buffers in gve_alloc_from_page_pool() and gve_rx_alloc_buffer() while
gve_get_napi_enabled() is false.

> diff --git a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> index 6880d1531ba5e..3b13aa4999ae5 100644
> --- a/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c
> @@ -223,13 +223,18 @@ void gve_free_to_page_pool(struct gve_rx_ring *rx,
>  static int gve_alloc_from_page_pool(struct gve_rx_ring *rx,
>  				    struct gve_rx_buf_state_dqo *buf_state)
>  {
> +	/*
> +	 * Use GFP_KERNEL when pre-filling RX rings in process context (e.g.
> +	 * gve_open) so PISA metadata init and CoCo page decryption can sleep.
> +	 */
> +	gfp_t gfp = gve_get_napi_enabled(rx->gve) ? GFP_ATOMIC : GFP_KERNEL;
>  	netmem_ref netmem;
>  
>  	buf_state->page_info.buf_size = rx->packet_buffer_truesize;
>  	netmem = page_pool_alloc_netmem(rx->dqo.page_pool,
>  					&buf_state->page_info.page_offset,
>  					&buf_state->page_info.buf_size,
> -					GFP_ATOMIC);
> +					gfp);

[Severity: High]
Can gve_alloc_from_page_pool() (and gve_rx_alloc_buffer() below) be called
from NAPI poll with GFP_KERNEL during gve_turnup()?

In gve_turnup(), each RX queue's NAPI is enabled, its IRQ doorbell is
unmasked, and napi_schedule() is called inside the per-queue loop before
GVE_PRIV_FLAGS_NAPI_ENABLED is set at the end of the function:

drivers/net/ethernet/google/gve/gve_main.c:gve_turnup() {
    ...
	for (i = 0; i < priv->rx_cfg.num_queues; i++) {
        ...
		napi_enable_locked(&block->napi);
        ...
		mb();
		napi_schedule(&block->napi);
	}
    ...
	gve_set_napi_enabled(priv);
}

CPU0                              CPU1 (<softirq>)
gve_turnup()
  napi_enable_locked(&block->napi)
  napi_schedule(&block->napi)
                                  gve_napi_poll_dqo()
                                    gve_rx_poll_dqo()
                                      ...
                                        gve_alloc_from_page_pool()
                                          gve_get_napi_enabled() -> false
                                          page_pool_alloc_netmem(..., GFP_KERNEL)
                                            <-- sleep in atomic context
  gve_set_napi_enabled(priv)

If NAPI runs on another CPU or on softirq exit before gve_turnup() finishes
the loop and calls gve_set_napi_enabled(priv), refilling buffers from
softirq context will use GFP_KERNEL and sleep in atomic context.

> diff --git a/drivers/net/ethernet/google/gve/gve_rx.c b/drivers/net/ethernet/google/gve/gve_rx.c
> index 81ea800e66e94..89f287b678bf0 100644
> --- a/drivers/net/ethernet/google/gve/gve_rx.c
> +++ b/drivers/net/ethernet/google/gve/gve_rx.c
> @@ -161,12 +161,16 @@ static int gve_rx_alloc_buffer(struct gve_priv *priv, struct device *dev,
>  			       union gve_rx_data_slot *data_slot,
>  			       struct gve_rx_ring *rx)
>  {
> +	/*
> +	 * Use GFP_KERNEL when pre-filling RX rings in process context (e.g.
> +	 * gve_open) so PISA metadata init and CoCo page decryption can sleep.
> +	 */
> +	gfp_t gfp = gve_get_napi_enabled(priv) ? GFP_ATOMIC : GFP_KERNEL;
>  	struct page *page;
>  	dma_addr_t dma;
>  	int err;
>  
> -	err = gve_alloc_page(priv, dev, &page, &dma, DMA_FROM_DEVICE,
> -			     GFP_ATOMIC);
> +	err = gve_alloc_page(priv, dev, &page, &dma, DMA_FROM_DEVICE, gfp);

[Severity: Medium]
Does gve_rx_alloc_buffer() still select GFP_ATOMIC when pre-filling GQI RX
rings on a running interface?

When reconfiguring queues or ring sizes on a live device,
gve_adjust_config() calls gve_queues_mem_alloc() -> gve_rx_alloc_rings_gqi()
-> gve_rx_alloc_ring_gqi() -> gve_rx_prefill_pages() before calling
gve_close() to turn down the device and clear GVE_PRIV_FLAGS_NAPI_ENABLED
(and similarly in gve_rx_queue_mem_alloc()). Because gve_get_napi_enabled()
is still true during this process-context pre-fill, gve_rx_alloc_buffer()
falls back to GFP_ATOMIC and can still exhaust atomic reserves or fail
allocation. Passing a gfp_t parameter explicitly from the pre-fill vs NAPI
refill callers would avoid depending on GVE_PRIV_FLAGS_NAPI_ENABLED for
both issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109182533.3594984-1-lrizzo@google.com?part=21