* [PATCH] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
@ 2020-07-27 8:09 Chao Leng
2020-07-27 18:44 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Chao Leng @ 2020-07-27 8:09 UTC (permalink / raw)
To: linux-nvme; +Cc: kbusch, axboe, hch, lengchao, sagi
A deadlock happens when test link blink for nvme over roce. If time out
in reconneting process, nvme_rdma_timeout->nvme_rdma_teardown_io_queues
will quiesce the io queues, and then the ctrl will be deleted after
reconnect times exceed max_reconnects. If run fdisk from the time
when the queue is quiesced to the time when the ctrl is deleted,
delete ctrl will deadlock, the process: nvme_do_delete_ctrl->
nvme_remove_namespaces->nvme_ns_remove->blk_cleanup_queue->
blk_freeze_queue->blk_mq_freeze_queue_wait, blk_mq_freeze_queue_wait
will wait until q_usage_counter of queue become 0, but the queue is
quiesced, can not clean any request.
Solution: nvme_rdma_timeout should call nvme_start_queues after
call nvme_rdma_teardown_io_queues. further more, we need start queues
regardless of whether the remove flag is set, after cancel requests
in nvme_rdma_teardown_io_queues.
Signed-off-by: Chao Leng <lengchao@huawei.com>
---
drivers/nvme/host/rdma.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index f8f856dc0c67..b381e2cde50a 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -989,8 +989,7 @@ static void nvme_rdma_teardown_io_queues(struct nvme_rdma_ctrl *ctrl,
nvme_cancel_request, &ctrl->ctrl);
blk_mq_tagset_wait_completed_request(ctrl->ctrl.tagset);
}
- if (remove)
- nvme_start_queues(&ctrl->ctrl);
+ nvme_start_queues(&ctrl->ctrl);
nvme_rdma_destroy_io_queues(ctrl, remove);
}
}
@@ -1128,7 +1127,6 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work)
nvme_stop_keep_alive(&ctrl->ctrl);
nvme_rdma_teardown_io_queues(ctrl, false);
- nvme_start_queues(&ctrl->ctrl);
nvme_rdma_teardown_admin_queue(ctrl, false);
blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
--
2.16.4
_______________________________________________
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] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
2020-07-27 8:09 [PATCH] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail Chao Leng
@ 2020-07-27 18:44 ` Sagi Grimberg
2020-07-27 23:31 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-27 18:44 UTC (permalink / raw)
To: Chao Leng, linux-nvme; +Cc: kbusch, axboe, hch
> A deadlock happens when test link blink for nvme over roce. If time out
> in reconneting process, nvme_rdma_timeout->nvme_rdma_teardown_io_queues
> will quiesce the io queues, and then the ctrl will be deleted after
> reconnect times exceed max_reconnects. If run fdisk from the time
> when the queue is quiesced to the time when the ctrl is deleted,
> delete ctrl will deadlock, the process: nvme_do_delete_ctrl->
> nvme_remove_namespaces->nvme_ns_remove->blk_cleanup_queue->
> blk_freeze_queue->blk_mq_freeze_queue_wait, blk_mq_freeze_queue_wait
> will wait until q_usage_counter of queue become 0, but the queue is
> quiesced, can not clean any request.
>
> Solution: nvme_rdma_timeout should call nvme_start_queues after
> call nvme_rdma_teardown_io_queues. further more, we need start queues
> regardless of whether the remove flag is set, after cancel requests
> in nvme_rdma_teardown_io_queues.
>
> Signed-off-by: Chao Leng <lengchao@huawei.com>
> ---
> drivers/nvme/host/rdma.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index f8f856dc0c67..b381e2cde50a 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -989,8 +989,7 @@ static void nvme_rdma_teardown_io_queues(struct nvme_rdma_ctrl *ctrl,
> nvme_cancel_request, &ctrl->ctrl);
> blk_mq_tagset_wait_completed_request(ctrl->ctrl.tagset);
> }
> - if (remove)
> - nvme_start_queues(&ctrl->ctrl);
> + nvme_start_queues(&ctrl->ctrl);
This will fail I/O during controller reset, so nak on this.
> nvme_rdma_destroy_io_queues(ctrl, remove);
> }
> }
> @@ -1128,7 +1127,6 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work)
>
> nvme_stop_keep_alive(&ctrl->ctrl);
> nvme_rdma_teardown_io_queues(ctrl, false);
> - nvme_start_queues(&ctrl->ctrl);
> nvme_rdma_teardown_admin_queue(ctrl, false);
> blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
>
>
_______________________________________________
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] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
2020-07-27 18:44 ` Sagi Grimberg
@ 2020-07-27 23:31 ` Sagi Grimberg
2020-07-28 3:06 ` Chao Leng
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-27 23:31 UTC (permalink / raw)
To: Chao Leng, linux-nvme; +Cc: kbusch, axboe, hch
>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>> index f8f856dc0c67..b381e2cde50a 100644
>> --- a/drivers/nvme/host/rdma.c
>> +++ b/drivers/nvme/host/rdma.c
>> @@ -989,8 +989,7 @@ static void nvme_rdma_teardown_io_queues(struct
>> nvme_rdma_ctrl *ctrl,
>> nvme_cancel_request, &ctrl->ctrl);
>> blk_mq_tagset_wait_completed_request(ctrl->ctrl.tagset);
>> }
>> - if (remove)
>> - nvme_start_queues(&ctrl->ctrl);
>> + nvme_start_queues(&ctrl->ctrl);
>
> This will fail I/O during controller reset, so nak on this.
Can you try this:
--
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index d58231636d11..96c0d664fe9b 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1149,6 +1149,11 @@ static void nvme_rdma_reconnect_ctrl_work(struct
work_struct *work)
return;
requeue:
+ /*
+ * make sure queues are not quiesced due to a reconnect
+ * sequence that failed after creating some I/O queues
+ */
+ nvme_start_queues(ctrl);
dev_info(ctrl->ctrl.device, "Failed reconnect attempt %d\n",
ctrl->ctrl.nr_reconnects);
nvme_rdma_reconnect_or_remove(ctrl);
--
_______________________________________________
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] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
2020-07-27 23:31 ` Sagi Grimberg
@ 2020-07-28 3:06 ` Chao Leng
2020-07-28 3:32 ` Sagi Grimberg
0 siblings, 1 reply; 6+ messages in thread
From: Chao Leng @ 2020-07-28 3:06 UTC (permalink / raw)
To: Sagi Grimberg, linux-nvme; +Cc: kbusch, axboe, hch
On 2020/7/28 7:31, Sagi Grimberg wrote:
>
>>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>>> index f8f856dc0c67..b381e2cde50a 100644
>>> --- a/drivers/nvme/host/rdma.c
>>> +++ b/drivers/nvme/host/rdma.c
>>> @@ -989,8 +989,7 @@ static void nvme_rdma_teardown_io_queues(struct nvme_rdma_ctrl *ctrl,
>>> nvme_cancel_request, &ctrl->ctrl);
>>> blk_mq_tagset_wait_completed_request(ctrl->ctrl.tagset);
>>> }
>>> - if (remove)
>>> - nvme_start_queues(&ctrl->ctrl);
>>> + nvme_start_queues(&ctrl->ctrl);
>>
>> This will fail I/O during controller reset, so nak on this.
The io will do not fail. If work with native multipath or dm-multipath,
nvme_rdma_queue_rq will return io error, and then multipath will
fail over to other path and retry io, this is we expected. If work
without multipath, nvme_rdma_queue_rq will return BLK_STS_RESOURCE,
and then the upper layer will requeue and retry. Surely there is a
weakness:the io will retry repeated every BLK_MQ_RESOURCE_DELAY(3ms)
while reconnecting. Because controller reset may need long time,
and nvme over roce is mainly used with multipath software, so when
controller reset we expect fail over to other path and retry io,, just
like error recovery. If work without multipath, we tolerate repeated
I/O retries during error recovery or controller reset.
>
> Can you try this:
> --
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index d58231636d11..96c0d664fe9b 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1149,6 +1149,11 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)
> return;
>
> requeue:
> + /*
> + * make sure queues are not quiesced due to a reconnect
> + * sequence that failed after creating some I/O queues
> + */
> + nvme_start_queues(ctrl);
> dev_info(ctrl->ctrl.device, "Failed reconnect attempt %d\n",
> ctrl->ctrl.nr_reconnects);
> nvme_rdma_reconnect_or_remove(ctrl);
> --
> .Surely this can solve the problem, but when controller reset in
multipath environment we expect fail over to other path and retry io,
just like error recovery. Both for controller reset and error recovery,
we need unquiesce the queue. So nvme_rdma_teardown_io_queues should
directly unquiesce queues after cancel request, do not need care
the parameter:remove.
For this solution, if nvme_rdma_setup_ctrl failed in nvme_rdma_reset_ctrl_work,
will start nvme_rdma_reconnect_ctrl_work, thus no difference.
If we do not tolerate repeated I/O retries during controller reset,
we can do like this:
---
drivers/nvme/host/rdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index b381e2cde50a..ac79c4d294c2 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1936,6 +1936,7 @@ nvme_rdma_timeout(struct request *rq, bool reserved)
*/
flush_work(&ctrl->err_work);
nvme_rdma_teardown_io_queues(ctrl, false);
+ nvme_start_queues(&ctrl->ctrl);
nvme_rdma_teardown_admin_queue(ctrl, false);
return BLK_EH_DONE;
}
--
2.16.4
_______________________________________________
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] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
2020-07-28 3:06 ` Chao Leng
@ 2020-07-28 3:32 ` Sagi Grimberg
[not found] ` <1288e338-9e92-eeeb-6f7b-86590c6e1a4c@broadcom.com>
0 siblings, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-28 3:32 UTC (permalink / raw)
To: Chao Leng, linux-nvme; +Cc: kbusch, axboe, hch
>>>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>>>> index f8f856dc0c67..b381e2cde50a 100644
>>>> --- a/drivers/nvme/host/rdma.c
>>>> +++ b/drivers/nvme/host/rdma.c
>>>> @@ -989,8 +989,7 @@ static void nvme_rdma_teardown_io_queues(struct
>>>> nvme_rdma_ctrl *ctrl,
>>>> nvme_cancel_request, &ctrl->ctrl);
>>>> blk_mq_tagset_wait_completed_request(ctrl->ctrl.tagset);
>>>> }
>>>> - if (remove)
>>>> - nvme_start_queues(&ctrl->ctrl);
>>>> + nvme_start_queues(&ctrl->ctrl);
>>>
>>> This will fail I/O during controller reset, so nak on this.
> The io will do not fail. If work with native multipath or dm-multipath,
> nvme_rdma_queue_rq will return io error, and then multipath will
> fail over to other path and retry io, this is we expected. If work
> without multipath, nvme_rdma_queue_rq will return BLK_STS_RESOURCE,
> and then the upper layer will requeue and retry. Surely there is a
> weakness:the io will retry repeated every BLK_MQ_RESOURCE_DELAY(3ms)
> while reconnecting. Because controller reset may need long time,
> and nvme over roce is mainly used with multipath software, so when
> controller reset we expect fail over to other path and retry io,, just
> like error recovery. If work without multipath, we tolerate repeated
> I/O retries during error recovery or controller reset.
I/O should not fail during reset, mpath or not, period.
>> Can you try this:
>> --
>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>> index d58231636d11..96c0d664fe9b 100644
>> --- a/drivers/nvme/host/rdma.c
>> +++ b/drivers/nvme/host/rdma.c
>> @@ -1149,6 +1149,11 @@ static void
>> nvme_rdma_reconnect_ctrl_work(struct work_struct *work)
>> return;
>>
>> requeue:
>> + /*
>> + * make sure queues are not quiesced due to a reconnect
>> + * sequence that failed after creating some I/O queues
>> + */
>> + nvme_start_queues(ctrl);
>> dev_info(ctrl->ctrl.device, "Failed reconnect attempt %d\n",
>> ctrl->ctrl.nr_reconnects);
>> nvme_rdma_reconnect_or_remove(ctrl);
>> --
>> .Surely this can solve the problem, but when controller reset in
> multipath environment we expect fail over to other path and retry io,
> just like error recovery. Both for controller reset and error recovery,
> we need unquiesce the queue. So nvme_rdma_teardown_io_queues should
> directly unquiesce queues after cancel request, do not need care
> the parameter:remove.
> For this solution, if nvme_rdma_setup_ctrl failed in
> nvme_rdma_reset_ctrl_work,
> will start nvme_rdma_reconnect_ctrl_work, thus no difference.
> If we do not tolerate repeated I/O retries during controller reset,
> we can do like this:
> ---
> drivers/nvme/host/rdma.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index b381e2cde50a..ac79c4d294c2 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1936,6 +1936,7 @@ nvme_rdma_timeout(struct request *rq, bool reserved)
> */
> flush_work(&ctrl->err_work);
> nvme_rdma_teardown_io_queues(ctrl, false);
> + nvme_start_queues(&ctrl->ctrl);
> nvme_rdma_teardown_admin_queue(ctrl, false);
> return BLK_EH_DONE;
> }
That is fine too, just do not unquiesce the queues in a normal reset.
_______________________________________________
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] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail
[not found] ` <1288e338-9e92-eeeb-6f7b-86590c6e1a4c@broadcom.com>
@ 2020-07-28 16:27 ` Sagi Grimberg
0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2020-07-28 16:27 UTC (permalink / raw)
To: James Smart, Chao Leng, linux-nvme; +Cc: kbusch, axboe, hch
>>> The io will do not fail. If work with native multipath or dm-multipath,
>>> nvme_rdma_queue_rq will return io error, and then multipath will
>>> fail over to other path and retry io, this is we expected. If work
>>> without multipath, nvme_rdma_queue_rq will return BLK_STS_RESOURCE,
>>> and then the upper layer will requeue and retry. Surely there is a
>>> weakness:the io will retry repeated every BLK_MQ_RESOURCE_DELAY(3ms)
>>> while reconnecting. Because controller reset may need long time,
>>> and nvme over roce is mainly used with multipath software, so when
>>> controller reset we expect fail over to other path and retry io,, just
>>> like error recovery. If work without multipath, we tolerate repeated
>>> I/O retries during error recovery or controller reset.
>>
>> I/O should not fail during reset, mpath or not, period.
>
> except when marked as an internal io (one used for reconnect, or maybe
> an ioctl) or marked for mpath.
I meant normal fs I/O, from the user perspective.
_______________________________________________
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-28 16:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 8:09 [PATCH] nvme-rdma: fix deadlock when delete ctrl due to reconnect fail Chao Leng
2020-07-27 18:44 ` Sagi Grimberg
2020-07-27 23:31 ` Sagi Grimberg
2020-07-28 3:06 ` Chao Leng
2020-07-28 3:32 ` Sagi Grimberg
[not found] ` <1288e338-9e92-eeeb-6f7b-86590c6e1a4c@broadcom.com>
2020-07-28 16:27 ` Sagi Grimberg
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).