From: Praveen Kaligineedi pkaligineedi@google.com
gve_tx_timeout was calculating missed completions in a way that is only relevant in the GQ queue format. Additionally, it was attempting to disable device interrupts, which is not needed in either GQ or DQ queue formats.
As a result, TX timeouts with the DQ queue format likely would have triggered early resets without kicking the queue at all.
This patch drops the check for pending work altogether and always kicks the queue after validating the queue has not seen a TX timeout too recently.
Fixes: 87a7f321bb6a ("gve: Recover from queue stall due to missed IRQ") Co-developed-by: Tim Hostetler thostet@google.com Signed-off-by: Tim Hostetler thostet@google.com Signed-off-by: Praveen Kaligineedi pkaligineedi@google.com Signed-off-by: Harshitha Ramamurthy hramamurthy@google.com --- drivers/net/ethernet/google/gve/gve_main.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c index c3791cf..0c6328b 100644 --- a/drivers/net/ethernet/google/gve/gve_main.c +++ b/drivers/net/ethernet/google/gve/gve_main.c @@ -1921,7 +1921,6 @@ static void gve_tx_timeout(struct net_device *dev, unsigned int txqueue) struct gve_notify_block *block; struct gve_tx_ring *tx = NULL; struct gve_priv *priv; - u32 last_nic_done; u32 current_time; u32 ntfy_idx;
@@ -1941,17 +1940,10 @@ static void gve_tx_timeout(struct net_device *dev, unsigned int txqueue) if (tx->last_kick_msec + MIN_TX_TIMEOUT_GAP > current_time) goto reset;
- /* Check to see if there are missed completions, which will allow us to - * kick the queue. - */ - last_nic_done = gve_tx_load_event_counter(priv, tx); - if (last_nic_done - tx->done) { - netdev_info(dev, "Kicking queue %d", txqueue); - iowrite32be(GVE_IRQ_MASK, gve_irq_doorbell(priv, block)); - napi_schedule(&block->napi); - tx->last_kick_msec = current_time; - goto out; - } // Else reset. + netdev_info(dev, "Kicking queue %d", txqueue); + napi_schedule(&block->napi); + tx->last_kick_msec = current_time; + goto out;
reset: gve_schedule_reset(priv);
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH net] gve: Fix stuck TX queue for DQ queue format Link: https://lore.kernel.org/stable/20250604201938.1409219-1-hramamurthy%40google...
On Wed, 4 Jun 2025 20:19:38 +0000 Harshitha Ramamurthy wrote:
- netdev_info(dev, "Kicking queue %d", txqueue);
- napi_schedule(&block->napi);
- tx->last_kick_msec = current_time;
- goto out;
reset: gve_schedule_reset(priv);
gotos at the base level of the function are too ugly to exit.
Please refactor this first to move the logic that decides whether reset should happen to a separate helper, then you can avoid both gotos/labels.
goto reset should turn into return true goto out should turn into return false
On Thu, Jun 5, 2025 at 8:13 AM Jakub Kicinski kuba@kernel.org wrote:
On Wed, 4 Jun 2025 20:19:38 +0000 Harshitha Ramamurthy wrote:
netdev_info(dev, "Kicking queue %d", txqueue);
napi_schedule(&block->napi);
tx->last_kick_msec = current_time;
goto out;
reset: gve_schedule_reset(priv);
gotos at the base level of the function are too ugly to exit.
Please refactor this first to move the logic that decides whether reset should happen to a separate helper, then you can avoid both gotos/labels.
goto reset should turn into return true goto out should turn into return false
That makes sense to me, I'll refactor this in v2.
linux-stable-mirror@lists.linaro.org