Few improvements for nvme-rdma host driver
Sagi Grimberg
maintainercover Few improvements for nvme-rdma host driver
Patch 1 fixes a possible controller deletion hang during I/O, the other patches are centralizing code such that common paths can be taken in multiple flows.
Sagi Grimberg (3):
nvme-rdma: unquiesce queues when deleting the controller
nvme-rdma: centralize controller setup sequence
nvme-rdma: centralize admin/io queue teardown sequence
drivers/nvme/host/rdma.c | 194 ++++++++++++++++++++--------------------------- 1 file changed, 82 insertions(+), 112 deletions(-)
Signature · 1 line
2.14.1
Sagi Grimberg
maintainerPATCH 1/3 nvme-rdma: unquiesce queues when deleting the controller
If the controller is going away, we need to unquiesce the IO queues so that all pending request can fail gracefully before moving forward with controller deletion. Do that before we destroy the IO queues so blk_cleanup_queue won't block in freeze.
Signed-off-by Sagi Grimberg <sagi at grimberg.me>
Signed-off-by Christoph Hellwig <hch at lst.de>
--- drivers/nvme/host/rdma.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/nvme/host/rdma.c +2 -0
@@ -1759,6 +1759,8 @@ static void nvme_rdma_shutdown_ctrl(struct nvme_rdma_ctrl *ctrl, bool shutdown)nvme_rdma_stop_io_queues (ctrl);blk_mq_tagset_busy_iter (&ctrl->tag_set, nvme_cancel_request, &ctrl->ctrl); +if (shutdown) +nvme_start_queues (&ctrl->ctrl);nvme_rdma_destroy_io_queues (ctrl, shutdown); }
Signature · 1 line
2.14.1
Sagi Grimberg
maintainerPATCH 2/3 nvme-rdma: centralize controller setup sequence
In reply to Sagi GrimbergCentralize controller sequence to a single routine that correctly cleans up after failures instead of having multiple apperances in several flows (create, reset, reconnect).
One thing that we also gain here are the sanity/boundary checks also when connecting back to a dynamic controller.
Signed-off-by Sagi Grimberg <sagi at grimberg.me>
Signed-off-by Christoph Hellwig <hch at lst.de>
--- drivers/nvme/host/rdma.c | 130 +++++++++++++++++++---------------------------- 1 file changed, 53 insertions(+), 77 deletions(-)
drivers/nvme/host/rdma.c +53 -77
@@ -917,24 +917,44 @@ static void nvme_rdma_reconnect_or_remove(struct nvme_rdma_ctrl *ctrl) } } -static void nvme_rdma_reconnect_ctrl_work (struct work_struct *work) +static int nvme_rdma_setup_ctrl (struct nvme_rdma_ctrl *ctrl,bool new) { -struct nvme_rdma_ctrl *ctrl =container_of (to_delayed_work (work), - struct nvme_rdma_ctrl, reconnect_work); +int ret = -EINVAL ;bool changed; -int ret; - - ++ctrl->ctrl.nr_reconnects; - ret =nvme_rdma_configure_admin_queue (ctrl, false); + ret =nvme_rdma_configure_admin_queue (ctrl, new);if (ret) - goto requeue; +return ret; + +if (ctrl->ctrl.icdoff) { +dev_err (ctrl->ctrl.device,"icdoff is not supported!\n" ); + goto destroy_admin; + } + +if (!(ctrl->ctrl.sgls & (1 <<2 ))) { +dev_err (ctrl->ctrl.device, +"Mandatory keyed sgls are not supported!\n" ); + goto destroy_admin; + } + +if (ctrl->ctrl.opts->queue_size > ctrl->ctrl.sqsize +1 ) { +dev_warn (ctrl->ctrl.device, +"queue_size %zu > ctrl sqsize %u, clamping down\n" , + ctrl->ctrl.opts->queue_size, ctrl->ctrl.sqsize +1 ); + } + +if (ctrl->ctrl.sqsize +1 > ctrl->ctrl.maxcmd) { +dev_warn (ctrl->ctrl.device, +"sqsize %u > ctrl maxcmd %u, clamping down\n" , + ctrl->ctrl.sqsize +1 , ctrl->ctrl.maxcmd); + ctrl->ctrl.sqsize = ctrl->ctrl.maxcmd -1 ; + }if (ctrl->ctrl.sgls & (1 <<20 )) ctrl->use_inline_data = true;if (ctrl->ctrl.queue_count >1 ) { - ret =nvme_rdma_configure_io_queues (ctrl, false); + ret =nvme_rdma_configure_io_queues (ctrl, new);if (ret) goto destroy_admin; }
@@ -943,10 +963,31 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)if (!changed) {/* state change failure is ok if we're in DELETING state */ WARN_ON_ONCE (ctrl->ctrl.state !=NVME_CTRL_DELETING ); -return ; + ret = -EINVAL ; + goto destroy_io; }nvme_start_ctrl (&ctrl->ctrl); +return 0 ; + +destroy_io: +if (ctrl->ctrl.queue_count >1 ) +nvme_rdma_destroy_io_queues (ctrl, new); +destroy_admin: +nvme_rdma_stop_queue (&ctrl->queues[0 ]); +nvme_rdma_destroy_admin_queue (ctrl, new); +return ret; +} + +static void nvme_rdma_reconnect_ctrl_work (struct work_struct *work) +{ +struct nvme_rdma_ctrl *ctrl =container_of (to_delayed_work (work), + struct nvme_rdma_ctrl, reconnect_work); + + ++ctrl->ctrl.nr_reconnects; + +if (nvme_rdma_setup_ctrl (ctrl, false)) + goto requeue;dev_info (ctrl->ctrl.device,"Successfully reconnected (%d attempts)\n" , ctrl->ctrl.nr_reconnects);
@@ -955,9 +996,6 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)return ; -destroy_admin: -nvme_rdma_stop_queue (&ctrl->queues[0 ]); -nvme_rdma_destroy_admin_queue (ctrl, false); requeue:dev_info (ctrl->ctrl.device,"Failed reconnect attempt %d\n" , ctrl->ctrl.nr_reconnects);
@@ -1786,8 +1824,6 @@ static void nvme_rdma_reset_ctrl_work(struct work_struct *work) {struct nvme_rdma_ctrl *ctrl = container_of(work, struct nvme_rdma_ctrl, ctrl.reset_work); -int ret; -bool changed;nvme_stop_ctrl (&ctrl->ctrl);nvme_rdma_shutdown_ctrl (ctrl, false);
@@ -1798,25 +1834,9 @@ static void nvme_rdma_reset_ctrl_work(struct work_struct *work)return ; } - ret =nvme_rdma_configure_admin_queue (ctrl, false); -if (ret) +if (nvme_rdma_setup_ctrl (ctrl, false)) goto out_fail; -if (ctrl->ctrl.queue_count >1 ) { - ret =nvme_rdma_configure_io_queues (ctrl, false); -if (ret) - goto out_fail; - } - - changed =nvme_change_ctrl_state (&ctrl->ctrl,NVME_CTRL_LIVE ); -if (!changed) { -/* state change failure is ok if we're in DELETING state */ -WARN_ON_ONCE (ctrl->ctrl.state !=NVME_CTRL_DELETING ); -return ; - } - -nvme_start_ctrl (&ctrl->ctrl); -return ; out_fail:
@@ -1979,49 +1999,10 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev, changed =nvme_change_ctrl_state (&ctrl->ctrl,NVME_CTRL_CONNECTING );WARN_ON_ONCE (!changed); - ret =nvme_rdma_configure_admin_queue (ctrl, true); + ret =nvme_rdma_setup_ctrl (ctrl, true);if (ret) goto out_uninit_ctrl; -/* sanity check icdoff */ -if (ctrl->ctrl.icdoff) { -dev_err (ctrl->ctrl.device,"icdoff is not supported!\n" ); - ret = -EINVAL ; - goto out_remove_admin_queue; - } - -/* sanity check keyed sgls */ -if (!(ctrl->ctrl.sgls & (1 <<2 ))) { -dev_err (ctrl->ctrl.device, -"Mandatory keyed sgls are not supported!\n" ); - ret = -EINVAL ; - goto out_remove_admin_queue; - } - -/* only warn if argument is too large here, will clamp later */ -if (opts->queue_size > ctrl->ctrl.sqsize +1 ) { -dev_warn (ctrl->ctrl.device, -"queue_size %zu > ctrl sqsize %u, clamping down\n" , - opts->queue_size, ctrl->ctrl.sqsize +1 ); - } - -/* warn if maxcmd is lower than sqsize+1 */ -if (ctrl->ctrl.sqsize +1 > ctrl->ctrl.maxcmd) { -dev_warn (ctrl->ctrl.device, -"sqsize %u > ctrl maxcmd %u, clamping down\n" , - ctrl->ctrl.sqsize +1 , ctrl->ctrl.maxcmd); - ctrl->ctrl.sqsize = ctrl->ctrl.maxcmd -1 ; - } - -if (opts->nr_io_queues) { - ret =nvme_rdma_configure_io_queues (ctrl, true); -if (ret) - goto out_remove_admin_queue; - } - - changed =nvme_change_ctrl_state (&ctrl->ctrl,NVME_CTRL_LIVE ); -WARN_ON_ONCE (!changed); -dev_info (ctrl->ctrl.device,"new ctrl: NQN \"%s\", addr %pISpcs\n" , ctrl->ctrl.opts->subsysnqn, &ctrl->addr);
@@ -2031,13 +2012,8 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,list_add_tail (&ctrl->list, &nvme_rdma_ctrl_list);mutex_unlock (&nvme_rdma_ctrl_mutex); -nvme_start_ctrl (&ctrl->ctrl); -return &ctrl->ctrl; -out_remove_admin_queue: -nvme_rdma_stop_queue (&ctrl->queues[0 ]); -nvme_rdma_destroy_admin_queue (ctrl, true); out_uninit_ctrl:nvme_uninit_ctrl (&ctrl->ctrl);nvme_put_ctrl (&ctrl->ctrl);
Signature · 1 line
2.14.1
Sagi Grimberg
maintainerPATCH 3/3 nvme-rdma: centralize admin/io queue teardown sequence
In reply to Sagi GrimbergWe follow the same queue teardown sequence in delete, reset and error recovery. Centralize the logic. This patch does not change any functionality.
Signed-off-by Sagi Grimberg <sagi at grimberg.me>
Signed-off-by Christoph Hellwig <hch at lst.de>
--- drivers/nvme/host/rdma.c | 66 +++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 37 deletions(-)
drivers/nvme/host/rdma.c +29 -37
@@ -873,6 +873,31 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)return ret; } +static void nvme_rdma_teardown_admin_queue (struct nvme_rdma_ctrl *ctrl, +bool remove) +{ +blk_mq_quiesce_queue (ctrl->ctrl.admin_q); +nvme_rdma_stop_queue (&ctrl->queues[0 ]); +blk_mq_tagset_busy_iter (&ctrl->admin_tag_set, nvme_cancel_request, + &ctrl->ctrl); +blk_mq_unquiesce_queue (ctrl->ctrl.admin_q); +nvme_rdma_destroy_admin_queue (ctrl, remove); +} + +static void nvme_rdma_teardown_io_queues (struct nvme_rdma_ctrl *ctrl, +bool remove) +{ +if (ctrl->ctrl.queue_count >1 ) { +nvme_stop_queues (&ctrl->ctrl); +nvme_rdma_stop_io_queues (ctrl); +blk_mq_tagset_busy_iter (&ctrl->tag_set, nvme_cancel_request, + &ctrl->ctrl); +if (remove) +nvme_start_queues (&ctrl->ctrl); +nvme_rdma_destroy_io_queues (ctrl, remove); + } +} +static void nvme_rdma_stop_ctrl (struct nvme_ctrl *nctrl) {struct nvme_rdma_ctrl *ctrl =to_rdma_ctrl (nctrl);
@@ -1008,27 +1033,9 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work) struct nvme_rdma_ctrl, err_work);nvme_stop_keep_alive (&ctrl->ctrl); - -if (ctrl->ctrl.queue_count >1 ) { -nvme_stop_queues (&ctrl->ctrl); -nvme_rdma_stop_io_queues (ctrl); -blk_mq_tagset_busy_iter (&ctrl->tag_set, - nvme_cancel_request, &ctrl->ctrl); -nvme_rdma_destroy_io_queues (ctrl, false); - } - -blk_mq_quiesce_queue (ctrl->ctrl.admin_q); -nvme_rdma_stop_queue (&ctrl->queues[0 ]); -blk_mq_tagset_busy_iter (&ctrl->admin_tag_set, - nvme_cancel_request, &ctrl->ctrl); -nvme_rdma_destroy_admin_queue (ctrl, false); - -/* -* queues are not a live anymore, so restart the queues to fail fast -* new IO -*/ -blk_mq_unquiesce_queue (ctrl->ctrl.admin_q); +nvme_rdma_teardown_io_queues (ctrl, false);nvme_start_queues (&ctrl->ctrl); +nvme_rdma_teardown_admin_queue (ctrl, false);if (!nvme_change_ctrl_state (&ctrl->ctrl,NVME_CTRL_CONNECTING )) {/* state change failure is ok if we're in DELETING state */
@@ -1792,27 +1799,12 @@ static const struct blk_mq_ops nvme_rdma_admin_mq_ops = {static void nvme_rdma_shutdown_ctrl (struct nvme_rdma_ctrl *ctrl,bool shutdown) { -if (ctrl->ctrl.queue_count >1 ) { -nvme_stop_queues (&ctrl->ctrl); -nvme_rdma_stop_io_queues (ctrl); -blk_mq_tagset_busy_iter (&ctrl->tag_set, - nvme_cancel_request, &ctrl->ctrl); -if (shutdown) -nvme_start_queues (&ctrl->ctrl); -nvme_rdma_destroy_io_queues (ctrl, shutdown); - } - +nvme_rdma_teardown_io_queues (ctrl, shutdown);if (shutdown)nvme_shutdown_ctrl (&ctrl->ctrl);else nvme_disable_ctrl (&ctrl->ctrl, ctrl->ctrl.cap); - -blk_mq_quiesce_queue (ctrl->ctrl.admin_q); -nvme_rdma_stop_queue (&ctrl->queues[0 ]); -blk_mq_tagset_busy_iter (&ctrl->admin_tag_set, - nvme_cancel_request, &ctrl->ctrl); -blk_mq_unquiesce_queue (ctrl->ctrl.admin_q); -nvme_rdma_destroy_admin_queue (ctrl, shutdown); +nvme_rdma_teardown_admin_queue (ctrl, shutdown); }static void nvme_rdma_delete_ctrl (struct nvme_ctrl *ctrl)
Signature · 1 line
2.14.1
cover Few improvements for nvme-rdma host driver
In reply to Sagi GrimbergThanks,
applied.