linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully
@ 2021-07-20 12:43 Daniel Wagner
  2021-07-20 12:43 ` [PATCH v3 1/6] nvme-fc: Update hardware queues before using them Daniel Wagner
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Daniel Wagner

Hi,

I've replaced my 'nvme_start_freeze' patch with the two patches from
James and gave it another test run on top of Ming's 'v2 fix
blk_mq_alloc_request_hctx' series. All looks good.

Thanks,
Daniel


v1:
 - https://lore.kernel.org/linux-nvme/20210625101649.49296-1-dwagner@suse.de/
v2:
 - https://lore.kernel.org/linux-nvme/20210708092755.15660-1-dwagner@suse.de/
 - reviewed tags collected
 - added 'update hardware queues' for all transport
 - added fix for fc hanger in nvme_wait_freeze_timeout
v3:
 - dropped 'nvme-fc: Freeze queues before destroying them'
 - added James' two patches


Initial cover letter:

this is a followup on the crash I reported in

  https://lore.kernel.org/linux-block/20210608183339.70609-1-dwagner@suse.de/

By moving the hardware check up the crash was gone. Unfortuntatly, I
don't understand why this fixes the crash. The per-cpu access is
crashing but I can't see why the blk_mq_update_nr_hw_queues() is
fixing this problem.

Even though I can't explain why it fixes it, I think it makes sense to
update the hardware queue mapping bevore we recreate the IO
queues. Thus I avoided in the commit message to say it fixes
something.

Also during testing I observed the we hang indivinetly in
blk_mq_freeze_queue_wait(). Again I can't explain why we get stuck
there but given a common pattern for the nvme_wait_freeze() is to use
it with a timeout I think the timeout should be used too :)

Anyway, someone with more undertanding of the stack can explain the
problems.

Daniel Wagner (3):
  nvme-fc: Update hardware queues before using them
  nvme-rdma: Update number of hardware queues before using them
  nvme-fc: Wait with a timeout for queue to freeze

Hannes Reinecke (1):
  nvme-tcp: Update number of hardware queues before using them

James Smart (2):
  nvme-fc: avoid race between time out and tear down
  nvme-fc: fix controller reset hang during traffic

 drivers/nvme/host/fc.c   | 28 +++++++++++++++++++---------
 drivers/nvme/host/rdma.c | 13 ++++++-------
 drivers/nvme/host/tcp.c  | 14 ++++++--------
 3 files changed, 31 insertions(+), 24 deletions(-)

-- 
2.29.2


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

* [PATCH v3 1/6] nvme-fc: Update hardware queues before using them
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 17:54   ` Hannes Reinecke
  2021-07-20 12:43 ` [PATCH v3 2/6] nvme-tcp: Update number of " Daniel Wagner
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Daniel Wagner, James Smart

In case the number of hardware queues changes, do the update the
tagset and ctx to hctx first before using the mapping to recreate and
connnect the IO queues.

Reviewed-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/fc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 053b0f94859f..8d742a6c82c1 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2952,14 +2952,6 @@ nvme_fc_recreate_io_queues(struct nvme_fc_ctrl *ctrl)
 	if (ctrl->ctrl.queue_count == 1)
 		return 0;
 
-	ret = nvme_fc_create_hw_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
-	if (ret)
-		goto out_free_io_queues;
-
-	ret = nvme_fc_connect_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
-	if (ret)
-		goto out_delete_hw_queues;
-
 	if (prior_ioq_cnt != nr_io_queues) {
 		dev_info(ctrl->ctrl.device,
 			"reconnect: revising io queue count from %d to %d\n",
@@ -2969,6 +2961,14 @@ nvme_fc_recreate_io_queues(struct nvme_fc_ctrl *ctrl)
 		nvme_unfreeze(&ctrl->ctrl);
 	}
 
+	ret = nvme_fc_create_hw_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
+	if (ret)
+		goto out_free_io_queues;
+
+	ret = nvme_fc_connect_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
+	if (ret)
+		goto out_delete_hw_queues;
+
 	return 0;
 
 out_delete_hw_queues:
-- 
2.29.2


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

* [PATCH v3 2/6] nvme-tcp: Update number of hardware queues before using them
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
  2021-07-20 12:43 ` [PATCH v3 1/6] nvme-fc: Update hardware queues before using them Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 12:43 ` [PATCH v3 3/6] nvme-rdma: " Daniel Wagner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Hannes Reinecke, Daniel Wagner

From: Hannes Reinecke <hare@suse.de>

When the number of hardware queues changes during resetting we should
update the tagset first before using it.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/tcp.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index cf3440fdada8..286e318403cb 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1791,6 +1791,7 @@ static void nvme_tcp_destroy_io_queues(struct nvme_ctrl *ctrl, bool remove)
 static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new)
 {
 	int ret;
+	u32 prior_q_cnt = ctrl->queue_count;
 
 	ret = nvme_tcp_alloc_io_queues(ctrl);
 	if (ret)
@@ -1808,14 +1809,7 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new)
 			ret = PTR_ERR(ctrl->connect_q);
 			goto out_free_tag_set;
 		}
-	}
-
-	ret = nvme_tcp_start_io_queues(ctrl);
-	if (ret)
-		goto out_cleanup_connect_q;
-
-	if (!new) {
-		nvme_start_queues(ctrl);
+	} else if (prior_q_cnt != ctrl->queue_count) {
 		if (!nvme_wait_freeze_timeout(ctrl, NVME_IO_TIMEOUT)) {
 			/*
 			 * If we timed out waiting for freeze we are likely to
@@ -1830,6 +1824,10 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new)
 		nvme_unfreeze(ctrl);
 	}
 
+	ret = nvme_tcp_start_io_queues(ctrl);
+	if (ret)
+		goto out_cleanup_connect_q;
+
 	return 0;
 
 out_wait_freeze_timed_out:
-- 
2.29.2


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

* [PATCH v3 3/6] nvme-rdma: Update number of hardware queues before using them
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
  2021-07-20 12:43 ` [PATCH v3 1/6] nvme-fc: Update hardware queues before using them Daniel Wagner
  2021-07-20 12:43 ` [PATCH v3 2/6] nvme-tcp: Update number of " Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 17:54   ` Hannes Reinecke
  2021-07-20 12:43 ` [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze Daniel Wagner
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Daniel Wagner

When the number of hardware queues changes during resetting we should
update the tagset first before using it.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/rdma.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 00fcbd985102..57d20100be56 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -966,6 +966,7 @@ static void nvme_rdma_destroy_io_queues(struct nvme_rdma_ctrl *ctrl,
 static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 {
 	int ret;
+	u32 prior_q_cnt = ctrl->ctrl.queue_count;
 
 	ret = nvme_rdma_alloc_io_queues(ctrl);
 	if (ret)
@@ -983,13 +984,7 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 			ret = PTR_ERR(ctrl->ctrl.connect_q);
 			goto out_free_tag_set;
 		}
-	}
-
-	ret = nvme_rdma_start_io_queues(ctrl);
-	if (ret)
-		goto out_cleanup_connect_q;
-
-	if (!new) {
+	} else if (prior_q_cnt != ctrl->ctrl.queue_count) {
 		nvme_start_queues(&ctrl->ctrl);
 		if (!nvme_wait_freeze_timeout(&ctrl->ctrl, NVME_IO_TIMEOUT)) {
 			/*
@@ -1005,6 +1000,10 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 		nvme_unfreeze(&ctrl->ctrl);
 	}
 
+	ret = nvme_rdma_start_io_queues(ctrl);
+	if (ret)
+		goto out_cleanup_connect_q;
+
 	return 0;
 
 out_wait_freeze_timed_out:
-- 
2.29.2


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

* [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
                   ` (2 preceding siblings ...)
  2021-07-20 12:43 ` [PATCH v3 3/6] nvme-rdma: " Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 17:55   ` Hannes Reinecke
  2021-07-20 12:43 ` [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down Daniel Wagner
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Daniel Wagner, James Smart

Do not wait indifinitly for all queues to freeze. Instead use a
timeout and abort the operation if we get stuck.

Reviewed-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/fc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 8d742a6c82c1..a64be4fb07af 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2956,7 +2956,14 @@ nvme_fc_recreate_io_queues(struct nvme_fc_ctrl *ctrl)
 		dev_info(ctrl->ctrl.device,
 			"reconnect: revising io queue count from %d to %d\n",
 			prior_ioq_cnt, nr_io_queues);
-		nvme_wait_freeze(&ctrl->ctrl);
+		if (!nvme_wait_freeze_timeout(&ctrl->ctrl, NVME_IO_TIMEOUT)) {
+			/*
+			 * If we timed out waiting for freeze we are likely to
+			 * be stuck.  Fail the controller initialization just
+			 * to be safe.
+			 */
+			return -ENODEV;
+		}
 		blk_mq_update_nr_hw_queues(&ctrl->tag_set, nr_io_queues);
 		nvme_unfreeze(&ctrl->ctrl);
 	}
-- 
2.29.2


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

* [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
                   ` (3 preceding siblings ...)
  2021-07-20 12:43 ` [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 17:56   ` Hannes Reinecke
  2021-07-20 12:43 ` [PATCH v3 6/6] nvme-fc: fix controller reset hang during traffic Daniel Wagner
  2021-07-20 12:48 ` [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
  6 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	James Smart, Chao Leng, Daniel Wagner

From: James Smart <jsmart2021@gmail.com>

To avoid race between time out and tear down, in tear down process,
first we quiesce the queue, and then delete the timer and cancel
the time out work for the queue.

This patch merges the admin and io sync ops into the queue teardown logic
as shown in the RDMA patch 3017013dcc "nvme-rdma: avoid race between time
out and tear down". There is no teardown_lock in nvme-fc.

Signed-off-by: James Smart <jsmart2021@gmail.com>
CC: Chao Leng <lengchao@huawei.com>
Tested-by: Daniel Wagner <dwagner@suse.de>
[dwagner: updated commit id referenced in commit message]
Reviewed-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/fc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index a64be4fb07af..112e62cd8a2a 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2487,6 +2487,7 @@ __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
 	 */
 	if (ctrl->ctrl.queue_count > 1) {
 		nvme_stop_queues(&ctrl->ctrl);
+		nvme_sync_io_queues(&ctrl->ctrl);
 		blk_mq_tagset_busy_iter(&ctrl->tag_set,
 				nvme_fc_terminate_exchange, &ctrl->ctrl);
 		blk_mq_tagset_wait_completed_request(&ctrl->tag_set);
@@ -2510,6 +2511,7 @@ __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
 	 * clean up the admin queue. Same thing as above.
 	 */
 	blk_mq_quiesce_queue(ctrl->ctrl.admin_q);
+	blk_sync_queue(ctrl->ctrl.admin_q);
 	blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
 				nvme_fc_terminate_exchange, &ctrl->ctrl);
 	blk_mq_tagset_wait_completed_request(&ctrl->admin_tag_set);
-- 
2.29.2


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

* [PATCH v3 6/6] nvme-fc: fix controller reset hang during traffic
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
                   ` (4 preceding siblings ...)
  2021-07-20 12:43 ` [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down Daniel Wagner
@ 2021-07-20 12:43 ` Daniel Wagner
  2021-07-20 12:48 ` [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
  6 siblings, 0 replies; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:43 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	James Smart, Daniel Wagner

From: James Smart <jsmart2021@gmail.com>

commit fe35ec58f0d3 ("block: update hctx map when use multiple maps")
exposed an issue where we may hang trying to wait for queue freeze
during I/O. We call blk_mq_update_nr_hw_queues which may attempt to freeze
the queue. However we never started queue freeze when starting the
reset, which means that we have inflight pending requests that entered the
queue that we will not complete once the queue is quiesced.

So start a freeze before we quiesce the queue, and unfreeze the queue
after we successfully connected the I/O queues (the unfreeze is already
present in the code). blk_mq_update_nr_hw_queues will be called only
after we are sure that the queue was already frozen.

This follows to how the pci driver handles resets.

This patch added logic introduced in commit 9f98772ba307 "nvme-rdma: fix
controller reset hang during traffic".

Signed-off-by: James Smart <jsmart2021@gmail.com>
CC: Sagi Grimberg <sagi@grimberg.me>
Tested-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
---
 drivers/nvme/host/fc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 112e62cd8a2a..ad3344f6048d 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2486,6 +2486,7 @@ __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
 	 * (but with error status).
 	 */
 	if (ctrl->ctrl.queue_count > 1) {
+		nvme_start_freeze(&ctrl->ctrl);
 		nvme_stop_queues(&ctrl->ctrl);
 		nvme_sync_io_queues(&ctrl->ctrl);
 		blk_mq_tagset_busy_iter(&ctrl->tag_set,
-- 
2.29.2


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

* Re: [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully
  2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
                   ` (5 preceding siblings ...)
  2021-07-20 12:43 ` [PATCH v3 6/6] nvme-fc: fix controller reset hang during traffic Daniel Wagner
@ 2021-07-20 12:48 ` Daniel Wagner
  2021-07-26 17:27   ` Daniel Wagner
  6 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-20 12:48 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Hannes Reinecke

On Tue, Jul 20, 2021 at 02:43:47PM +0200, Daniel Wagner wrote:
> v1:
>  - https://lore.kernel.org/linux-nvme/20210625101649.49296-1-dwagner@suse.de/
> v2:
>  - https://lore.kernel.org/linux-nvme/20210708092755.15660-1-dwagner@suse.de/
>  - reviewed tags collected
>  - added 'update hardware queues' for all transport
>  - added fix for fc hanger in nvme_wait_freeze_timeout
> v3:
>  - dropped 'nvme-fc: Freeze queues before destroying them'
>  - added James' two patches

Forgot to add Hannes' reviewed tag. Sorry!

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

* Re: [PATCH v3 1/6] nvme-fc: Update hardware queues before using them
  2021-07-20 12:43 ` [PATCH v3 1/6] nvme-fc: Update hardware queues before using them Daniel Wagner
@ 2021-07-20 17:54   ` Hannes Reinecke
  0 siblings, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2021-07-20 17:54 UTC (permalink / raw)
  To: Daniel Wagner, linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	James Smart

On 7/20/21 2:43 PM, Daniel Wagner wrote:
> In case the number of hardware queues changes, do the update the
> tagset and ctx to hctx first before using the mapping to recreate and
> connnect the IO queues.
> 
> Reviewed-by: James Smart <jsmart2021@gmail.com>
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>   drivers/nvme/host/fc.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v3 3/6] nvme-rdma: Update number of hardware queues before using them
  2021-07-20 12:43 ` [PATCH v3 3/6] nvme-rdma: " Daniel Wagner
@ 2021-07-20 17:54   ` Hannes Reinecke
  0 siblings, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2021-07-20 17:54 UTC (permalink / raw)
  To: Daniel Wagner, linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg

On 7/20/21 2:43 PM, Daniel Wagner wrote:
> When the number of hardware queues changes during resetting we should
> update the tagset first before using it.
> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>   drivers/nvme/host/rdma.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze
  2021-07-20 12:43 ` [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze Daniel Wagner
@ 2021-07-20 17:55   ` Hannes Reinecke
  0 siblings, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2021-07-20 17:55 UTC (permalink / raw)
  To: Daniel Wagner, linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	James Smart

On 7/20/21 2:43 PM, Daniel Wagner wrote:
> Do not wait indifinitly for all queues to freeze. Instead use a
> timeout and abort the operation if we get stuck.
> 
> Reviewed-by: James Smart <jsmart2021@gmail.com>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>   drivers/nvme/host/fc.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down
  2021-07-20 12:43 ` [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down Daniel Wagner
@ 2021-07-20 17:56   ` Hannes Reinecke
  0 siblings, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2021-07-20 17:56 UTC (permalink / raw)
  To: Daniel Wagner, linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	James Smart, Chao Leng

On 7/20/21 2:43 PM, Daniel Wagner wrote:
> From: James Smart <jsmart2021@gmail.com>
> 
> To avoid race between time out and tear down, in tear down process,
> first we quiesce the queue, and then delete the timer and cancel
> the time out work for the queue.
> 
> This patch merges the admin and io sync ops into the queue teardown logic
> as shown in the RDMA patch 3017013dcc "nvme-rdma: avoid race between time
> out and tear down". There is no teardown_lock in nvme-fc.
> 
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> CC: Chao Leng <lengchao@huawei.com>
> Tested-by: Daniel Wagner <dwagner@suse.de>
> [dwagner: updated commit id referenced in commit message]
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> ---
>   drivers/nvme/host/fc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

* Re: [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully
  2021-07-20 12:48 ` [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
@ 2021-07-26 17:27   ` Daniel Wagner
  2021-07-30  9:49     ` Daniel Wagner
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-26 17:27 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Hannes Reinecke

On Tue, Jul 20, 2021 at 02:48:00PM +0200, Daniel Wagner wrote:
> On Tue, Jul 20, 2021 at 02:43:47PM +0200, Daniel Wagner wrote:
> > v1:
> >  - https://lore.kernel.org/linux-nvme/20210625101649.49296-1-dwagner@suse.de/
> > v2:
> >  - https://lore.kernel.org/linux-nvme/20210708092755.15660-1-dwagner@suse.de/
> >  - reviewed tags collected
> >  - added 'update hardware queues' for all transport
> >  - added fix for fc hanger in nvme_wait_freeze_timeout
> > v3:
> >  - dropped 'nvme-fc: Freeze queues before destroying them'
> >  - added James' two patches
> 
> Forgot to add Hannes' reviewed tag. Sorry!

FTR, I've tested the 'prior_ioq_cnt != nr_io_queues' case. In this
scenario the series works. Though in the case of 'prior_ioq_cnt ==
nr_io_queues' I see hanging I/Os.

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

* Re: [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully
  2021-07-26 17:27   ` Daniel Wagner
@ 2021-07-30  9:49     ` Daniel Wagner
  2021-07-30 11:34       ` Daniel Wagner
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Wagner @ 2021-07-30  9:49 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Hannes Reinecke, Wen Xiong

On Mon, Jul 26, 2021 at 07:27:04PM +0200, Daniel Wagner wrote:
> FTR, I've tested the 'prior_ioq_cnt != nr_io_queues' case. In this
> scenario the series works. Though in the case of 'prior_ioq_cnt ==
> nr_io_queues' I see hanging I/Os.

Back on starring on this issue. So the hanging I/Os happen in this path
after a remote port has been disabled:

 nvme nvme1: NVME-FC{1}: new ctrl: NQN "nqn.1992-08.com.netapp:sn.d646dc63336511e995cb00a0988fb732:subsystem.nvme-svm-dolin-ana_subsystem"
 nvme nvme1: NVME-FC{1}: controller connectivity lost. Awaiting Reconnect
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: io failed due to lldd error 6
 nvme nvme1: NVME-FC{1}: connectivity re-established. Attempting reconnect
 nvme nvme1: NVME-FC{1}: create association : host wwpn 0x100000109b579ef6  rport wwpn 0x201900a09890f5bf: NQN "nqn.1992-08.com.netapp:sn.d646dc63336511e995cb00a0988fb732:subsystem.nvme-svm-dolin-ana_subsystem"
 nvme nvme1: NVME-FC{1}: controller connect complete

and all hanging tasks have the same call trace:

 task:fio             state:D stack:    0 pid:13545 ppid: 13463 flags:0x00000000
 Call Trace:
  __schedule+0x2d7/0x8f0
  schedule+0x3c/0xa0
  blk_queue_enter+0x106/0x1f0
  ? wait_woken+0x80/0x80
  submit_bio_noacct+0x116/0x4b0
  ? submit_bio+0x4b/0x1a0
  submit_bio+0x4b/0x1a0
  __blkdev_direct_IO_simple+0x20c/0x350
  ? update_load_avg+0x1ac/0x5e0
  ? blkdev_iopoll+0x30/0x30
  ? blkdev_direct_IO+0x4a2/0x520
  blkdev_direct_IO+0x4a2/0x520
  ? update_load_avg+0x1ac/0x5e0
  ? update_load_avg+0x1ac/0x5e0
  ? generic_file_read_iter+0x84/0x140
  ? __blkdev_direct_IO_simple+0x350/0x350
  generic_file_read_iter+0x84/0x140
  blkdev_read_iter+0x41/0x50
  new_sync_read+0x118/0x1a0
  vfs_read+0x15a/0x180
  ksys_pread64+0x71/0x90
  do_syscall_64+0x3c/0x80
  entry_SYSCALL_64_after_hwframe+0x44/0xae

(gdb) l *blk_queue_enter+0x106
0xffffffff81473736 is in blk_queue_enter (block/blk-core.c:469).
464                      * queue dying flag, otherwise the following wait may
465                      * never return if the two reads are reordered.
466                      */
467                     smp_rmb();
468
469                     wait_event(q->mq_freeze_wq,
470                                (!q->mq_freeze_depth &&
471                                 blk_pm_resume_queue(pm, q)) ||
472                                blk_queue_dying(q));
473                     if (blk_queue_dying(q))


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

* Re: [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully
  2021-07-30  9:49     ` Daniel Wagner
@ 2021-07-30 11:34       ` Daniel Wagner
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Wagner @ 2021-07-30 11:34 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-kernel, James Smart, Keith Busch, Ming Lei, Sagi Grimberg,
	Hannes Reinecke, Wen Xiong

On Fri, Jul 30, 2021 at 11:49:07AM +0200, Daniel Wagner wrote:
> On Mon, Jul 26, 2021 at 07:27:04PM +0200, Daniel Wagner wrote:
> > FTR, I've tested the 'prior_ioq_cnt != nr_io_queues' case. In this
> > scenario the series works. Though in the case of 'prior_ioq_cnt ==
> > nr_io_queues' I see hanging I/Os.
> 
> Back on starring on this issue. So the hanging I/Os happen in this path
> after a remote port has been disabled:

It turns out, the same happens with TCP as transport. I've got two
connection configured and block traffic on the target side with
iptables rules. This is what I see

 nvme nvme16: creating 80 I/O queues.
 nvme nvme16: mapped 80/0/0 default/read/poll queues.
 nvme nvme16: new ctrl: NQN "nqn.2014-08.org.nvmexpress:NVMf:uuid:de63429f-50a4-4e03-ade6-0be27b75be77", addr 10.161.8.24:4420
 nvme nvme17: creating 80 I/O queues.
 nvme nvme17: mapped 80/0/0 default/read/poll queues.
 nvme nvme17: new ctrl: NQN "nqn.2014-08.org.nvmexpress:NVMf:uuid:de63429f-50a4-4e03-ade6-0be27b75be77", addr 10.161.8.24:4421
 nvme nvme17: starting error recovery
 nvme nvme17: failed nvme_keep_alive_end_io error=10
 nvme nvme17: Reconnecting in 10 seconds...
 nvme nvme17: failed to connect socket: -110
 nvme nvme17: Failed reconnect attempt 1
 nvme nvme17: Reconnecting in 10 seconds...
 nvme nvme17: failed to connect socket: -110
 nvme nvme17: Failed reconnect attempt 2
 nvme nvme17: Reconnecting in 10 seconds...
 nvme nvme17: creating 80 I/O queues.
 nvme nvme17: Successfully reconnected (3 attempt)

 Call Trace:
  __schedule+0x2d7/0x8f0
  schedule+0x3c/0xa0
  blk_queue_enter+0x106/0x1f0
  ? wait_woken+0x80/0x80
  submit_bio_noacct+0x116/0x4b0
  ? submit_bio+0x4b/0x1a0
  submit_bio+0x4b/0x1a0
  __blkdev_direct_IO_simple+0x20c/0x350
  ? blkdev_iopoll+0x30/0x30
  ? blkdev_direct_IO+0x4a2/0x520
  blkdev_direct_IO+0x4a2/0x520
  ? asm_sysvec_apic_timer_interrupt+0x12/0x20
  ? generic_file_read_iter+0x84/0x140
  ? __blkdev_direct_IO_simple+0x350/0x350
  generic_file_read_iter+0x84/0x140
  blkdev_read_iter+0x41/0x50
  new_sync_read+0x118/0x1a0
  vfs_read+0x15a/0x180
  ksys_pread64+0x71/0x90
  do_syscall_64+0x3c/0x80
  entry_SYSCALL_64_after_hwframe+0x44/0xae

I think all transport handle the unfreezing incorrectly in the
recovering path. At least for TCP and FC I could test this. I don't have
and RDMA setup but this code looks suspiciously the same.. I think the
nvme_unfreeze() needs to be called always not just in the case where the
number of queues change.

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

end of thread, other threads:[~2021-07-30 11:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-20 12:43 [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
2021-07-20 12:43 ` [PATCH v3 1/6] nvme-fc: Update hardware queues before using them Daniel Wagner
2021-07-20 17:54   ` Hannes Reinecke
2021-07-20 12:43 ` [PATCH v3 2/6] nvme-tcp: Update number of " Daniel Wagner
2021-07-20 12:43 ` [PATCH v3 3/6] nvme-rdma: " Daniel Wagner
2021-07-20 17:54   ` Hannes Reinecke
2021-07-20 12:43 ` [PATCH v3 4/6] nvme-fc: Wait with a timeout for queue to freeze Daniel Wagner
2021-07-20 17:55   ` Hannes Reinecke
2021-07-20 12:43 ` [PATCH v3 5/6] nvme-fc: avoid race between time out and tear down Daniel Wagner
2021-07-20 17:56   ` Hannes Reinecke
2021-07-20 12:43 ` [PATCH v3 6/6] nvme-fc: fix controller reset hang during traffic Daniel Wagner
2021-07-20 12:48 ` [PATCH v3 0/6] Handle update hardware queues and queue freeze more carefully Daniel Wagner
2021-07-26 17:27   ` Daniel Wagner
2021-07-30  9:49     ` Daniel Wagner
2021-07-30 11:34       ` Daniel Wagner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).