* [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
[parent not found: <1288e338-9e92-eeeb-6f7b-86590c6e1a4c@broadcom.com>]
* 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).