gve: Add adminq mutex lock
authorZiwei Xiao <ziweixiao@google.com>
Tue, 25 Jun 2024 00:12:27 +0000 (00:12 +0000)
committerJakub Kicinski <kuba@kernel.org>
Wed, 26 Jun 2024 00:48:33 +0000 (17:48 -0700)
We were depending on the rtnl_lock to make sure there is only one adminq
command running at a time. But some commands may take too long to hold
the rtnl_lock, such as the upcoming flow steering operations. For such
situations, it can temporarily drop the rtnl_lock, and replace it for
these operations with a new adminq lock, which can ensure the adminq
command execution to be thread-safe.

Signed-off-by: Ziwei Xiao <ziweixiao@google.com>
Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com>
Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Link: https://patch.msgid.link/20240625001232.1476315-2-ziweixiao@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/ethernet/google/gve/gve.h
drivers/net/ethernet/google/gve/gve_adminq.c

index ae1e21c9b0a54da18dde42edd2b3cb280e4dccf3..ca7fce17f2c05788e88d8d0d42b1a6220a07c84c 100644 (file)
@@ -724,6 +724,7 @@ struct gve_priv {
        union gve_adminq_command *adminq;
        dma_addr_t adminq_bus_addr;
        struct dma_pool *adminq_pool;
+       struct mutex adminq_lock; /* Protects adminq command execution */
        u32 adminq_mask; /* masks prod_cnt to adminq size */
        u32 adminq_prod_cnt; /* free-running count of AQ cmds executed */
        u32 adminq_cmd_fail; /* free-running count of AQ cmds failed */
index 8ca0def176ef470e74c0f41fd180a7d03184f23e..2e0c1eb87b11c06dad6a822df7cef4441be42f7a 100644 (file)
@@ -284,6 +284,7 @@ int gve_adminq_alloc(struct device *dev, struct gve_priv *priv)
                            &priv->reg_bar0->adminq_base_address_lo);
                iowrite32be(GVE_DRIVER_STATUS_RUN_MASK, &priv->reg_bar0->driver_status);
        }
+       mutex_init(&priv->adminq_lock);
        gve_set_admin_queue_ok(priv);
        return 0;
 }
@@ -511,28 +512,29 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
        return 0;
 }
 
-/* This function is not threadsafe - the caller is responsible for any
- * necessary locks.
- * The caller is also responsible for making sure there are no commands
- * waiting to be executed.
- */
 static int gve_adminq_execute_cmd(struct gve_priv *priv,
                                  union gve_adminq_command *cmd_orig)
 {
        u32 tail, head;
        int err;
 
+       mutex_lock(&priv->adminq_lock);
        tail = ioread32be(&priv->reg_bar0->adminq_event_counter);
        head = priv->adminq_prod_cnt;
-       if (tail != head)
-               // This is not a valid path
-               return -EINVAL;
+       if (tail != head) {
+               err = -EINVAL;
+               goto out;
+       }
 
        err = gve_adminq_issue_cmd(priv, cmd_orig);
        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;
 }
 
 /* The device specifies that the management vector can either be the first irq