nvme_fc: correct hang in nvme_ns_remove()
Subsystems (2)
PATCH 1/1 nvme_fc: correct hang in nvme_ns_remove()
When connectivity is lost to a device, the association is terminated and the blk-mq queues are quiesced/stopped. When connectivity is re-established, they are resumed.
If connectivity is lost for a sufficient amount of time that the controller is then deleted, the delete path starts tearing down queues, and eventually calling nvme_ns_remove(). It appears that pending commands may cause blk_cleanup_queue() to never complete and the teardown stalls.
Correct by starting the ns queues after transitioning to a DELETING state, allowing pending commands to be flushed with io failures. Thus the delete path is clear when reached.
--- drivers/nvme/host/fc.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/nvme/host/fc.c +3 -0
@@ -2938,6 +2938,9 @@ nvme_fc_delete_ctrl(struct nvme_ctrl *nctrl)* waiting for io to terminate */ nvme_fc_delete_association (ctrl); + +/* resume the io queues so that things will fast fail */ +nvme_start_queues (nctrl); } static void
Signature · 1 line
2.13.1
PATCH 1/1 nvme_fc: correct hang in nvme_ns_remove()
If you compare behavior of FC with rdma, rdma starts the queues at the tail end of losing connectivity to the device - meaning any pending io and any future io issued while connectivity has yet to
be re-established (e.g. in RECONNECTING state) will fail with an io error. This is good, if there is a multipathing config, as it's a near-immediate fast fail scenario. But... if there is no multipath, it means applications and filesystems are now seeing io errors while connectivity is pending and that can be disastrous. FC currently leaves the queues quiesced while connectivity is pending so io errors are not seen. But this means FC won't fastfail the ios to the multipath'er.
For now I want to fix this keeping the existing FC behavior. From there, I'd like the transports to block like FC does so no errors. However, a new timer would be introduced for a "fast failure timeout" - which starts at loss of connectivity and when expires, starts the queues and fails any pending and future io.
Thoughts ?
-- james
Quoted 13 lines · James Smart
When connectivity is lost to a device, the association is terminated and the blk-mq queues are quiesced/stopped. When connectivity is re-established, they are resumed.
If connectivity is lost for a sufficient amount of time that the controller is then deleted, the delete path starts tearing down queues, and eventually calling
nvme_ns_remove(). It appears that pending commands may causeblk_cleanup_queue()to never complete and the teardown stalls.Correct by starting the ns queues after transitioning to a DELETING state, allowing pending commands to be flushed with io failures. Thus the delete path is clear when reached.
PATCH 1/1 nvme_fc: correct hang in nvme_ns_remove()
Quoted 12 lines · James Smart
If you compare behavior of FC with rdma, rdma starts the queues at the tail end of losing connectivity to the device - meaning any pending io and any future io issued while connectivity has yet to
be re-established (e.g. in RECONNECTING state) will fail with an io error. This is good, if there is a multipathing config, as it's a near-immediate fast fail scenario. But... if there is no multipath, it means applications and filesystems are now seeing io errors while connectivity is pending and that can be disastrous. FC currently leaves the queues quiesced while connectivity is pending so io errors are not seen. But this means FC won't fastfail the ios to the multipath'er.
Quoted 7 lines
For now I want to fix this keeping the existing FC behavior. From there, I'd like the transports to block like FC does so no errors. However, a new timer would be introduced for a "fast failure timeout" - which starts at loss of connectivity and when expires, starts the queues and fails any pending and future io.
Thoughts ?
Yes, I think that sounds ok.
Longer term, I think it's a bit tacky that we rely on queue_rq to check for early termination states. Since we can quiece blk-mq, it'd be better if we introduce another tag iterator to end unstarted requests directly when we need to give up on the request, rather than rely on queue_rq. I was going to post a patch that does just that, but I still haven't gotten a chance to test it... :(
Sagi Grimberg
maintainerPATCH 1/1 nvme_fc: correct hang in nvme_ns_remove()
Quoted 21 lines
Quoted 13 lines
If you compare behavior of FC with rdma, rdma starts the queues at the tail end of losing connectivity to the device - meaning any pending io and any future io issued while connectivity has yet to
be re-established (e.g. in RECONNECTING state) will fail with an io error. This is good, if there is a multipathing config, as it's a near-immediate fast fail scenario. But... if there is no multipath, it means applications and filesystems are now seeing io errors while connectivity is pending and that can be disastrous. FC currently leaves the queues quiesced while connectivity is pending so io errors are not seen. But this means FC won't fastfail the ios to the multipath'er.Quoted 7 lines
For now I want to fix this keeping the existing FC behavior. From there, I'd like the transports to block like FC does so no errors. However, a new timer would be introduced for a "fast failure timeout" - which starts at loss of connectivity and when expires, starts the queues and fails any pending and future io.
Thoughts ?
Quoted 8 lines
Yes, I think that sounds ok.
Longer term, I think it's a bit tacky that we rely on
queue_rqto check for early termination states. Since we can quiece blk-mq, it'd be better if we introduce another tag iterator to end unstarted requests directly when we need to give up on the request, rather than rely onqueue_rq. I was going to post a patch that does just that, but I still haven't gotten a chance to test it... :(
I agree with the fast_fail_tmo, we also need it for the multipath code which currently blocks forever until either we reconnect successfully or delete the controller.
James, are you looking into this? Note that this should be done in nvme-core.
Sagi Grimberg
maintainerPATCH 1/1 nvme_fc: correct hang in nvme_ns_remove()
In reply to James Smartdrivers/nvme/host/fc.c nvme_fc_delete_ctrl(struct nvme_ctrl *nctrl) · line 2943
Quoted 12 lines
drivers/nvme/host/fc.c +3
-0@@ -2938,6 +2938,9 @@ nvme_fc_delete_ctrl(struct nvme_ctrl *nctrl)* waiting for io to terminate */ nvme_fc_delete_association (ctrl); + +/* resume the io queues so that things will fast fail */ +nvme_start_queues (nctrl);
Is fc also unquiescing the admin queue here?