Re: [PATCH net] gve: fix Rx queue stall on alloc failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Updating u64_stats_sync counters from process context without disabling preemption causes a seqcount deadlock on 32-bit systems.
--

commit 1e50350a60c2a580ad86412a4cff5327452afdc7
Author: Eddie Phillips <eddiephillips@google.com>

gve: fix Rx queue stall on alloc failure

This commit introduces a starvation recovery timer for each Rx queue to
prevent permanent queue stalls when page allocations fail during the refill
loop. It also adds an ethtool statistic to track when this starvation recovery
timer is triggered.

> diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> index 02cba280d81a8..303db4fa677b3 100644
> --- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c

[ ... ]

> @@ -400,6 +415,23 @@ void gve_rx_post_buffers_dqo(struct gve_rx_ring *rx)
>  	}
>  
>  	rx->fill_cnt += num_posted;
> +
> +	/* If the queue has fewer than GVE_RX_BUF_THRESH_DQO descriptors
> +	 * visible to the hardware, and no doorbell was written, the hardware
> +	 * is in danger of starving and cannot trigger interrupts. Start the
> +	 * timer to periodically reschedule NAPI and recover from starvation.
> +	 */
> +	num_bufs_avail_to_hw =
> +		((bufq->tail & ~(GVE_RX_BUF_THRESH_DQO - 1)) -
> +		 bufq->head) & bufq->mask;
> +
> +	if (num_bufs_avail_to_hw < GVE_RX_BUF_THRESH_DQO) {
> +		u64_stats_update_begin(&rx->statss);
> +		rx->rx_critical_low_bufs++;
> +		u64_stats_update_end(&rx->statss);

[Severity: High]
This isn't a bug introduced by this patch, but it adds a new instance of a
pre-existing issue.

Could this lead to a seqcount deadlock on 32-bit systems?

If gve_rx_post_buffers_dqo() is called from a process context where preemption
is enabled, a reader thread (like a netlink get_queue_stats_rx request) could
preempt the writer thread while the seqcount is odd.

On 32-bit architectures, this would cause the reader to spin infinitely in
u64_stats_fetch_retry() waiting for the seqcount to become even, deadlocking
the CPU since it won't yield back to the writer.

Would it be safer to disable preemption or use
u64_stats_update_begin_irqsave() here and in the existing rx_buf_alloc_fail
stat update higher up in this same function?

> +		mod_timer(&rx->starvation_timer,
> +			  jiffies + msecs_to_jiffies(GVE_RX_NAPI_RESCHED_MS));
> +	}
>  }
>  
>  static void gve_rx_skb_csum(struct sk_buff *skb,

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