All of lore.kernel.org
 help / color / mirror / 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 related	[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 related	[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, other threads:[~2020-07-29  5:46 UTC | newest]

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

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.