nvmet-loop: do not alloc admin tag set during reset

Currently, resetting a loopback controller unconditionally invokes
nvme_alloc_admin_tag_set() inside nvme_loop_configure_admin_queue().
Doing so drops the old queue and allocates a new one. Consequently,
this reverts the admin queue's timeout (q->rq_timeout) back to the
module default (NVME_ADMIN_TIMEOUT), completely wiping out any custom
timeout values the user may have configured via sysfs and potentially
racing against the sysfs nvme_admin_timeout_store() function
that may dereference the admin_q pointer during the RESETTING state.

Decouple the admin tag set lifecycle from the admin queue
configuration and destruction paths, which are executed during resets;
Specifically:

* Move nvme_alloc_admin_tag_set() into nvme_loop_create_ctrl() so it
  is only allocated once during the initial controller creation.

* Defer the destruction of the admin tag set to
  nvme_loop_delete_ctrl_host() and the terminal error-handling
  paths of nvme_loop_reset_ctrl_work() and
  nvme_loop_create_ctrl().

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index d98d0cdc5d6fad10a3f8a7ad901ea41afd1e91df..070d16068e6b28b05150a98b952556b2c2ce0ceb 100644 (file)
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -274,7 +274,6 @@ static void nvme_loop_destroy_admin_queue(struct nvme_loop_ctrl *ctrl)
 
        nvmet_sq_destroy(&ctrl->queues[0].nvme_sq);
        nvmet_cq_put(&ctrl->queues[0].nvme_cq);
-       nvme_remove_admin_tag_set(&ctrl->ctrl);
 }
 
 static void nvme_loop_free_ctrl(struct nvme_ctrl *nctrl)
@@ -375,25 +374,18 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
        }
        ctrl->ctrl.queue_count = 1;
 
-       error = nvme_alloc_admin_tag_set(&ctrl->ctrl, &ctrl->admin_tag_set,
-                       &nvme_loop_admin_mq_ops,
-                       sizeof(struct nvme_loop_iod) +
-                       NVME_INLINE_SG_CNT * sizeof(struct scatterlist));
-       if (error)
-               goto out_free_sq;
-
        /* reset stopped state for the fresh admin queue */
        clear_bit(NVME_CTRL_ADMIN_Q_STOPPED, &ctrl->ctrl.flags);
 
        error = nvmf_connect_admin_queue(&ctrl->ctrl);
        if (error)
-               goto out_cleanup_tagset;
+               goto out_free_sq;
 
        set_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
 
        error = nvme_enable_ctrl(&ctrl->ctrl);
        if (error)
-               goto out_cleanup_tagset;
+               goto out_free_sq;
 
        ctrl->ctrl.max_hw_sectors =
                (NVME_LOOP_MAX_SEGMENTS - 1) << PAGE_SECTORS_SHIFT;
@@ -402,14 +394,12 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
 
        error = nvme_init_ctrl_finish(&ctrl->ctrl, false);
        if (error)
-               goto out_cleanup_tagset;
+               goto out_free_sq;
 
        return 0;
 
-out_cleanup_tagset:
-       clear_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
-       nvme_remove_admin_tag_set(&ctrl->ctrl);
 out_free_sq:
+       clear_bit(NVME_LOOP_Q_LIVE, &ctrl->queues[0].flags);
        nvmet_sq_destroy(&ctrl->queues[0].nvme_sq);
        nvmet_cq_put(&ctrl->queues[0].nvme_cq);
        return error;
@@ -432,6 +422,7 @@ static void nvme_loop_shutdown_ctrl(struct nvme_loop_ctrl *ctrl)
 static void nvme_loop_delete_ctrl_host(struct nvme_ctrl *ctrl)
 {
        nvme_loop_shutdown_ctrl(to_loop_ctrl(ctrl));
+       nvme_remove_admin_tag_set(ctrl);
 }
 
 static void nvme_loop_delete_ctrl(struct nvmet_ctrl *nctrl)
@@ -494,6 +485,7 @@ out_destroy_admin:
        nvme_cancel_admin_tagset(&ctrl->ctrl);
        nvme_loop_destroy_admin_queue(ctrl);
 out_disable:
+       nvme_remove_admin_tag_set(&ctrl->ctrl);
        dev_warn(ctrl->ctrl.device, "Removing after reset failure\n");
        nvme_uninit_ctrl(&ctrl->ctrl);
 }
@@ -594,10 +586,17 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
        if (!ctrl->queues)
                goto out_uninit_ctrl;
 
-       ret = nvme_loop_configure_admin_queue(ctrl);
+       ret = nvme_alloc_admin_tag_set(&ctrl->ctrl, &ctrl->admin_tag_set,
+                       &nvme_loop_admin_mq_ops,
+                       sizeof(struct nvme_loop_iod) +
+                       NVME_INLINE_SG_CNT * sizeof(struct scatterlist));
        if (ret)
                goto out_free_queues;
 
+       ret = nvme_loop_configure_admin_queue(ctrl);
+       if (ret)
+               goto out_remove_admin_tagset;
+
        if (opts->queue_size > ctrl->ctrl.maxcmd) {
                /* warn if maxcmd is lower than queue_size */
                dev_warn(ctrl->ctrl.device,
@@ -633,6 +632,8 @@ out_remove_admin_queue:
        nvme_quiesce_admin_queue(&ctrl->ctrl);
        nvme_cancel_admin_tagset(&ctrl->ctrl);
        nvme_loop_destroy_admin_queue(ctrl);
+out_remove_admin_tagset:
+       nvme_remove_admin_tag_set(&ctrl->ctrl);
 out_free_queues:
        kfree(ctrl->queues);
 out_uninit_ctrl: