Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] nvme-loop: a fix and cleanup
@ 2020-07-29  2:36 Chaitanya Kulkarni
  2020-07-29  2:36 ` [PATCH 1/2] nvme-loop: set ctrl state connecting after init Chaitanya Kulkarni
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2020-07-29  2:36 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

Hi,

This is a small patch-series which fixes an issue with the loop
transport and removes the extra variable where a function can be called
directly.

Keeping the fixes tag on patch #1 if needed, patch #2 is just my
personal preference to minimize the number of variables when I touch the
code.

-ck

Chaitanya Kulkarni (2):
  nvme-loop: set ctrl state connecting after init
  nvme-loop: remove extra variable in create ctrl

 drivers/nvme/target/loop.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.26.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] nvme-loop: set ctrl state connecting after init
  2020-07-29  2:36 [PATCH 0/2] nvme-loop: a fix and cleanup Chaitanya Kulkarni
@ 2020-07-29  2:36 ` Chaitanya Kulkarni
  2020-07-29  4:35   ` Sagi Grimberg
  2020-07-29  2:36 ` [PATCH 2/2] nvme-loop: remove extra variable in create ctrl Chaitanya Kulkarni
  2020-07-29  5:46 ` [PATCH 0/2] nvme-loop: a fix and cleanup Christoph Hellwig
  2 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2020-07-29  2:36 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

When creating a loop controller (ctrl) in nvme_loop_create_ctrl() ->
nvme_init_ctrl() we set the ctrl state to NVME_CTRL_NEW.

Prior to [1] NVME_CTRL_NEW state was allowed in nvmf_check_ready() for
fabrics command type connect. Now, this fails in the following code path
for fabrics connect command when creating admin queue :-

nvme_loop_create_ctrl()
 nvme_loo_configure_admin_queue()
  nvmf_connect_admin_queue()
   __nvme_submit_sync_cmd()
    blk_execute_rq()
      nvme_loop_queue_rq()
	nvmf_check_ready()

# echo  "transport=loop,nqn=fs" > /dev/nvme-fabrics
[ 6047.741327] nvmet: adding nsid 1 to subsystem fs
[ 6048.756430] nvme nvme1: Connect command failed, error wo/DNR bit: 880

We need to set the ctrl state to NVME_CTRL_CONNECTING after :-
nvme_loop_create_ctrl()
 nvme_init_ctrl()
so that the above mentioned check for nvmf_check_ready() will return
true.

This patch sets the ctrl state to connecting after we init the ctrl in
nvme_loop_create_ctrl()
 nvme_init_ctrl() .

[1] commit aa63fa6776a7 ("nvme-fabrics: allow to queue requests for live queues")

Fixes: aa63fa6776a7 ("nvme-fabrics: allow to queue requests for live queues")
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index f2c80a51985f..14d820c9a276 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -583,6 +583,9 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 	if (ret)
 		goto out_put_ctrl;
 
+	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING);
+	WARN_ON_ONCE(!changed);
+
 	ret = -ENOMEM;
 
 	ctrl->ctrl.sqsize = opts->queue_size - 1;
-- 
2.26.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] nvme-loop: remove extra variable in create ctrl
  2020-07-29  2:36 [PATCH 0/2] nvme-loop: a fix and cleanup Chaitanya Kulkarni
  2020-07-29  2:36 ` [PATCH 1/2] nvme-loop: set ctrl state connecting after init Chaitanya Kulkarni
@ 2020-07-29  2:36 ` Chaitanya Kulkarni
  2020-07-29  4:35   ` Sagi Grimberg
  2020-07-29  5:46 ` [PATCH 0/2] nvme-loop: a fix and cleanup Christoph Hellwig
  2 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2020-07-29  2:36 UTC (permalink / raw)
  To: linux-nvme; +Cc: hch, Chaitanya Kulkarni, sagi

We can call the nvme_change_ctrl_state() directly and have
WARN_ON_ONCE(1) call instead of having to use an extra variable which
matches the name of the function.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
---
 drivers/nvme/target/loop.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 14d820c9a276..4884ef1e46a2 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -444,7 +444,6 @@ static void nvme_loop_reset_ctrl_work(struct work_struct *work)
 {
 	struct nvme_loop_ctrl *ctrl =
 		container_of(work, struct nvme_loop_ctrl, ctrl.reset_work);
-	bool changed;
 	int ret;
 
 	nvme_stop_ctrl(&ctrl->ctrl);
@@ -471,8 +470,8 @@ static void nvme_loop_reset_ctrl_work(struct work_struct *work)
 	blk_mq_update_nr_hw_queues(&ctrl->tag_set,
 			ctrl->ctrl.queue_count - 1);
 
-	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
-	WARN_ON_ONCE(!changed);
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE))
+		WARN_ON_ONCE(1);
 
 	nvme_start_ctrl(&ctrl->ctrl);
 
@@ -567,7 +566,6 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 		struct nvmf_ctrl_options *opts)
 {
 	struct nvme_loop_ctrl *ctrl;
-	bool changed;
 	int ret;
 
 	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
@@ -583,8 +581,8 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 	if (ret)
 		goto out_put_ctrl;
 
-	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING);
-	WARN_ON_ONCE(!changed);
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING))
+		WARN_ON_ONCE(1);
 
 	ret = -ENOMEM;
 
@@ -620,8 +618,8 @@ static struct nvme_ctrl *nvme_loop_create_ctrl(struct device *dev,
 	dev_info(ctrl->ctrl.device,
 		 "new ctrl: \"%s\"\n", ctrl->ctrl.opts->subsysnqn);
 
-	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
-	WARN_ON_ONCE(!changed);
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE))
+		WARN_ON_ONCE(1);
 
 	mutex_lock(&nvme_loop_ctrl_mutex);
 	list_add_tail(&ctrl->list, &nvme_loop_ctrl_list);
-- 
2.26.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] nvme-loop: set ctrl state connecting after init
  2020-07-29  2:36 ` [PATCH 1/2] nvme-loop: set ctrl state connecting after init Chaitanya Kulkarni
@ 2020-07-29  4:35   ` Sagi Grimberg
  0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-29  4:35 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-nvme; +Cc: hch



On 7/28/20 7:36 PM, Chaitanya Kulkarni wrote:
> When creating a loop controller (ctrl) in nvme_loop_create_ctrl() ->
> nvme_init_ctrl() we set the ctrl state to NVME_CTRL_NEW.
> 
> Prior to [1] NVME_CTRL_NEW state was allowed in nvmf_check_ready() for
> fabrics command type connect. Now, this fails in the following code path
> for fabrics connect command when creating admin queue :-
> 
> nvme_loop_create_ctrl()
>   nvme_loo_configure_admin_queue()
>    nvmf_connect_admin_queue()
>     __nvme_submit_sync_cmd()
>      blk_execute_rq()
>        nvme_loop_queue_rq()
> 	nvmf_check_ready()
> 
> # echo  "transport=loop,nqn=fs" > /dev/nvme-fabrics
> [ 6047.741327] nvmet: adding nsid 1 to subsystem fs
> [ 6048.756430] nvme nvme1: Connect command failed, error wo/DNR bit: 880

I have exactly the same patch to send tonight :)

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Tested-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] nvme-loop: remove extra variable in create ctrl
  2020-07-29  2:36 ` [PATCH 2/2] nvme-loop: remove extra variable in create ctrl Chaitanya Kulkarni
@ 2020-07-29  4:35   ` Sagi Grimberg
  0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-29  4:35 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-nvme; +Cc: hch

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] nvme-loop: a fix and cleanup
  2020-07-29  2:36 [PATCH 0/2] nvme-loop: a fix and cleanup Chaitanya Kulkarni
  2020-07-29  2:36 ` [PATCH 1/2] nvme-loop: set ctrl state connecting after init Chaitanya Kulkarni
  2020-07-29  2:36 ` [PATCH 2/2] nvme-loop: remove extra variable in create ctrl Chaitanya Kulkarni
@ 2020-07-29  5:46 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2020-07-29  5:46 UTC (permalink / raw)
  To: Chaitanya Kulkarni; +Cc: hch, linux-nvme, sagi

Applied to nvme-5.9.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29  2:36 [PATCH 0/2] nvme-loop: a fix and cleanup Chaitanya Kulkarni
2020-07-29  2:36 ` [PATCH 1/2] nvme-loop: set ctrl state connecting after init Chaitanya Kulkarni
2020-07-29  4:35   ` Sagi Grimberg
2020-07-29  2:36 ` [PATCH 2/2] nvme-loop: remove extra variable in create ctrl Chaitanya Kulkarni
2020-07-29  4:35   ` Sagi Grimberg
2020-07-29  5:46 ` [PATCH 0/2] nvme-loop: a fix and cleanup Christoph Hellwig

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git