netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Harshitha Ramamurthy <hramamurthy@google.com>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	 pabeni@redhat.com, jeroendb@google.com, hramamurthy@google.com,
	 andrew+netdev@lunn.ch, willemb@google.com, ziweixiao@google.com,
	 pkaligineedi@google.com, yyd@google.com, joshwash@google.com,
	 shailend@google.com, linux@treblig.org, thostet@google.com,
	 jfraker@google.com, richardcochran@gmail.com,
	jdamato@fastly.com,  vadim.fedorenko@linux.dev, horms@kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH net-next v4 4/8] gve: Add adminq lock for queues creation and destruction
Date: Mon,  9 Jun 2025 18:40:25 +0000	[thread overview]
Message-ID: <20250609184029.2634345-5-hramamurthy@google.com> (raw)
In-Reply-To: <20250609184029.2634345-1-hramamurthy@google.com>

From: Ziwei Xiao <ziweixiao@google.com>

Adminq commands for queues creation and destruction were not
consistently protected by the driver's adminq_lock. This was previously
benign as these operations were always initiated from contexts holding
kernel-level locks (e.g., rtnl_lock, netdev_lock), which provided
serialization.

Upcoming PTP aux_work will issue adminq commands directly from the
driver to read the NIC clock, without such kernel lock protection.
To prevent race conditions with this new PTP work, this patch ensures
the adminq_lock is held during queues creation and destruction.

Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
 Changes in v2:
 - Send this patch together with the rx timestamping patches to net-next
   instead of sending it to net (Jakub Kicinski)
 - Remove the unnecessary cleanup (Jakub Kicinski)
---
 drivers/net/ethernet/google/gve/gve_adminq.c | 47 +++++++++++++++-----
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
index f57913a673b4..a0cc05a9eefc 100644
--- a/drivers/net/ethernet/google/gve/gve_adminq.c
+++ b/drivers/net/ethernet/google/gve/gve_adminq.c
@@ -463,6 +463,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;
 
@@ -488,9 +490,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)
 {
@@ -498,6 +497,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.
@@ -733,13 +734,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,
@@ -812,13 +819,19 @@ 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)
@@ -844,13 +857,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,
@@ -885,13 +904,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,
-- 
2.50.0.rc0.604.gd4ff7b7c86-goog


  parent reply	other threads:[~2025-06-09 18:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-09 18:40 [PATCH net-next v4 0/8] gve: Add Rx HW timestamping support Harshitha Ramamurthy
2025-06-09 18:40 ` [PATCH net-next v4 1/8] gve: Add device option for nic clock synchronization Harshitha Ramamurthy
2025-06-09 18:40 ` [PATCH net-next v4 2/8] gve: Add adminq command to report nic timestamp Harshitha Ramamurthy
2025-06-09 18:40 ` [PATCH net-next v4 3/8] gve: Add initial PTP device support Harshitha Ramamurthy
2025-06-09 18:40 ` Harshitha Ramamurthy [this message]
2025-06-09 18:40 ` [PATCH net-next v4 5/8] gve: Add support to query the nic clock Harshitha Ramamurthy
2025-06-10  9:47   ` Vadim Fedorenko
2025-06-11  1:25   ` Jakub Kicinski
2025-06-11 23:41     ` Ziwei Xiao
2025-06-09 18:40 ` [PATCH net-next v4 6/8] gve: Add rx hardware timestamp expansion Harshitha Ramamurthy
2025-06-10  9:49   ` Vadim Fedorenko
2025-06-09 18:40 ` [PATCH net-next v4 7/8] gve: Implement ndo_hwtstamp_get/set for RX timestamping Harshitha Ramamurthy
2025-06-09 18:40 ` [PATCH net-next v4 8/8] gve: Advertise support for rx hardware timestamping Harshitha Ramamurthy
2025-06-11  1:23 ` [PATCH net-next v4 0/8] gve: Add Rx HW timestamping support Jakub Kicinski
2025-06-11 22:05   ` Ziwei Xiao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250609184029.2634345-5-hramamurthy@google.com \
    --to=hramamurthy@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jdamato@fastly.com \
    --cc=jeroendb@google.com \
    --cc=jfraker@google.com \
    --cc=joshwash@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@treblig.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pkaligineedi@google.com \
    --cc=richardcochran@gmail.com \
    --cc=shailend@google.com \
    --cc=thostet@google.com \
    --cc=vadim.fedorenko@linux.dev \
    --cc=willemb@google.com \
    --cc=yyd@google.com \
    --cc=ziweixiao@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).