Re: [PATCH net] gve: Add adminq lock for creating and destroying multiple queues

On 4/17/25 22:43, Harshitha Ramamurthy wrote:

From: Ziwei Xiao ziweixiao@google.com

The original adminq lock is only protecting the gve_adminq_execute_cmd which is aimed for sending out single adminq command. However, there are other callers of gve_adminq_kick_and_wait and gve_adminq_issue_cmd that need to take the mutex lock for mutual exclusion between them, which are creating and destroying rx/tx queues. Add the adminq lock for those unprotected callers.

Also this patch cleans up the error handling code of gve_adminq_destroy_tx_queue.

Cc: stable@vger.kernel.org Fixes: 1108566ca509 ("gve: Add adminq mutex lock")

This looks like a correct fix, it is also nice that you have added lockdep annotations. Reviewed-by: Przemek Kitszel przemyslaw.kitszel@intel.com

Reviewed-by: Willem de Bruijn willemb@google.com Signed-off-by: Ziwei Xiao ziweixiao@google.com Signed-off-by: Harshitha Ramamurthy hramamurthy@google.com

---

drivers/net/ethernet/google/gve/gve_adminq.c | 54 ++++++++++++++------ 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c index 3e8fc33cc11f..659460812276 100644 --- a/drivers/net/ethernet/google/gve/gve_adminq.c +++ b/drivers/net/ethernet/google/gve/gve_adminq.c @@ -442,6 +442,8 @@ static int gve_adminq_kick_and_wait(struct gve_priv *priv) int tail, head; int i;

• lockdep_assert_held(&priv->adminq_lock);
• tail = ioread32be(&priv->reg_bar0->adminq_event_counter); head = priv->adminq_prod_cnt;

@@ -467,9 +469,6 @@ static int gve_adminq_kick_and_wait(struct gve_priv *priv) return 0; } -/* This function is not threadsafe - the caller is responsible for any

• • necessary locks.
• */ static int gve_adminq_issue_cmd(struct gve_priv *priv, union gve_adminq_command *cmd_orig) {

@@ -477,6 +476,8 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv, u32 opcode; u32 tail;

• lockdep_assert_held(&priv->adminq_lock);
• tail = ioread32be(&priv->reg_bar0->adminq_event_counter);

// Check if next command will overflow the buffer. @@ -709,13 +710,19 @@ int gve_adminq_create_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_que int err; int i;

• mutex_lock(&priv->adminq_lock);
• for (i = start_id; i < start_id + num_queues; i++) { err = gve_adminq_create_tx_queue(priv, i); if (err)

• 	return err;
  

• 	goto out;
  

  }

• return gve_adminq_kick_and_wait(priv);

• err = gve_adminq_kick_and_wait(priv);

+out:

• mutex_unlock(&priv->adminq_lock);
• return err; }

static void gve_adminq_get_create_rx_queue_cmd(struct gve_priv *priv, @@ -788,19 +795,24 @@ int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues) int err; int i;

• mutex_lock(&priv->adminq_lock);
• for (i = 0; i < num_queues; i++) { err = gve_adminq_create_rx_queue(priv, i); if (err)

• 	return err;
  

• 	goto out;
  

  }

• return gve_adminq_kick_and_wait(priv);

• err = gve_adminq_kick_and_wait(priv);

+out:

• mutex_unlock(&priv->adminq_lock);
• return err; }

static int gve_adminq_destroy_tx_queue(struct gve_priv *priv, u32 queue_index) { union gve_adminq_command cmd;

• int err;

memset(&cmd, 0, sizeof(cmd)); cmd.opcode = cpu_to_be32(GVE_ADMINQ_DESTROY_TX_QUEUE); @@ -808,11 +820,7 @@ static int gve_adminq_destroy_tx_queue(struct gve_priv *priv, u32 queue_index) .queue_id = cpu_to_be32(queue_index), };

• err = gve_adminq_issue_cmd(priv, &cmd);
• if (err)

• return err;
  

• return 0;

• return gve_adminq_issue_cmd(priv, &cmd); }

int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_queues) @@ -820,13 +828,19 @@ int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 start_id, u32 num_qu int err; int i;

• mutex_lock(&priv->adminq_lock);
• for (i = start_id; i < start_id + num_queues; i++) { err = gve_adminq_destroy_tx_queue(priv, i); if (err)

• 	return err;
  

• 	goto out;
  

  }

• return gve_adminq_kick_and_wait(priv);

• err = gve_adminq_kick_and_wait(priv);

+out:

• mutex_unlock(&priv->adminq_lock);
• return err; }

static void gve_adminq_make_destroy_rx_queue_cmd(union gve_adminq_command *cmd, @@ -861,13 +875,19 @@ int gve_adminq_destroy_rx_queues(struct gve_priv *priv, u32 num_queues) int err; int i;

• mutex_lock(&priv->adminq_lock);
• for (i = 0; i < num_queues; i++) { err = gve_adminq_destroy_rx_queue(priv, i); if (err)

• 	return err;
  

• 	goto out;
  

  }

• return gve_adminq_kick_and_wait(priv);

• err = gve_adminq_kick_and_wait(priv);

+out:

• mutex_unlock(&priv->adminq_lock);
• return err; }

static void gve_set_default_desc_cnt(struct gve_priv *priv,