All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maurizio Lombardi <mlombard@redhat.com>
To: kbusch@kernel.org
Cc: mheyne@amazon.de, emilne@redhat.com, jmeneghi@redhat.com,
	linux-nvme@lists.infradead.org, dwagner@suse.de,
	mlombard@arkamax.eu, mkhalfella@purestorage.com,
	chaitanyak@nvidia.com, hare@kernel.org, hch@lst.de
Subject: [PATCH V4 8/9] nvmet-loop: do not alloc admin tag set during reset
Date: Fri,  8 May 2026 15:33:34 +0200	[thread overview]
Message-ID: <20260508133335.98612-9-mlombard@redhat.com> (raw)
In-Reply-To: <20260508133335.98612-1-mlombard@redhat.com>

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().

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/nvme/target/loop.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index d98d0cdc5d6f..070d16068e6b 100644
--- 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 @@ static void nvme_loop_reset_ctrl_work(struct work_struct *work)
 	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 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 	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:
-- 
2.54.0



  parent reply	other threads:[~2026-05-08 13:34 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-08 13:33 [PATCH V4 0/9] nvme: Refactor and expose per-controller timeout configuration Maurizio Lombardi
2026-05-08 13:33 ` [PATCH V4 1/9] nvme: Let the blocklayer set timeouts for requests Maurizio Lombardi
2026-05-11  9:37   ` Hannes Reinecke
2026-05-08 13:33 ` [PATCH V4 2/9] nvme: add sysfs attribute to change admin timeout per nvme controller Maurizio Lombardi
2026-05-08 16:57   ` Daniel Wagner
2026-05-10 22:10   ` Sagi Grimberg
2026-05-11  8:07   ` Christoph Hellwig
2026-05-11 11:29     ` Maurizio Lombardi
2026-05-11 12:31       ` Christoph Hellwig
2026-05-11  9:46   ` Hannes Reinecke
2026-05-11 10:05     ` Maurizio Lombardi
2026-05-08 13:33 ` [PATCH V4 3/9] nvme: fix race condition between connected uevent and STARTED_ONCE flag Maurizio Lombardi
2026-05-08 16:57   ` Daniel Wagner
2026-05-10 22:10   ` Sagi Grimberg
2026-05-11  8:07     ` Christoph Hellwig
2026-05-11 12:54       ` Sagi Grimberg
2026-05-11 15:09         ` Keith Busch
2026-05-11 15:45           ` Maurizio Lombardi
2026-05-11 17:10             ` Keith Busch
2026-05-11  8:08   ` Christoph Hellwig
2026-05-11  9:47   ` Hannes Reinecke
2026-05-08 13:33 ` [PATCH V4 4/9] nvme: pci: use admin queue timeout over NVME_ADMIN_TIMEOUT Maurizio Lombardi
2026-05-10 22:10   ` Sagi Grimberg
2026-05-11  8:08   ` Christoph Hellwig
2026-05-11  9:48   ` Hannes Reinecke
2026-05-08 13:33 ` [PATCH V4 5/9] nvme: add sysfs attribute to change IO timeout per controller Maurizio Lombardi
2026-05-08 17:08   ` Daniel Wagner
2026-05-13 13:00     ` Maurizio Lombardi
2026-05-10 22:12   ` Sagi Grimberg
2026-05-11  8:52     ` Maurizio Lombardi
2026-05-08 13:33 ` [PATCH V4 6/9] nvme: use per controller timeout waits over depending on global default Maurizio Lombardi
2026-05-11  8:10   ` Christoph Hellwig
2026-05-11 11:42     ` Maurizio Lombardi
2026-05-11 12:32       ` Christoph Hellwig
2026-05-11  9:50   ` Hannes Reinecke
2026-05-08 13:33 ` [PATCH V4 7/9] nvme-core: align fabrics_q teardown with admin_q in nvme_free_ctrl Maurizio Lombardi
2026-05-10 22:15   ` Sagi Grimberg
2026-05-11  8:11   ` Christoph Hellwig
2026-05-11  9:53   ` Hannes Reinecke
2026-05-11  9:57     ` Maurizio Lombardi
2026-05-08 13:33 ` Maurizio Lombardi [this message]
2026-05-08 17:09   ` [PATCH V4 8/9] nvmet-loop: do not alloc admin tag set during reset Daniel Wagner
2026-05-10 22:16   ` Sagi Grimberg
2026-05-11  8:12   ` Christoph Hellwig
2026-05-11  9:55   ` Hannes Reinecke
2026-05-08 13:33 ` [PATCH V4 9/9] nvme-core: warn on allocating admin tag set with existing queue Maurizio Lombardi
2026-05-10 22:16   ` Sagi Grimberg

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=20260508133335.98612-9-mlombard@redhat.com \
    --to=mlombard@redhat.com \
    --cc=chaitanyak@nvidia.com \
    --cc=dwagner@suse.de \
    --cc=emilne@redhat.com \
    --cc=hare@kernel.org \
    --cc=hch@lst.de \
    --cc=jmeneghi@redhat.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mheyne@amazon.de \
    --cc=mkhalfella@purestorage.com \
    --cc=mlombard@arkamax.eu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.