[PATCH] nvme: move nvme_stop_keep_alive() back to original position

[Sagi Grimberg]

> Stopping keep-alive not only stops the keep-alive workqueue,
> but also needs to be synchronized with I/O termination as we
> must not send a keep-alive command after all I/O had been
> terminated.
> So to avoid any regressions move the call to stop_keep_alive()
> back to its original position and ensure that keep-alive is
> correctly stopped failing to setup the admin queue.
> 
> Fixes: 4733b65d82bd ("nvme: start keep-alive after admin queue setup")

It should be folded in I guess. I suggest to either suggest an
alternative phrasing of the patch or resend an alternative patch
set.

> Suggested-by: Sagi Grimberg <sagi at grimberg.me>
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> ---
>   drivers/nvme/host/core.c |  2 +-
>   drivers/nvme/host/fc.c   | 19 ++++++++-----------
>   drivers/nvme/host/rdma.c |  1 +
>   drivers/nvme/host/tcp.c  |  1 +
>   4 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index fd28e6b6574c..46a4c9c5ea96 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -482,7 +482,6 @@ EXPORT_SYMBOL_GPL(nvme_cancel_tagset);
>   
>   void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl)
>   {
> -	nvme_stop_keep_alive(ctrl);
>   	if (ctrl->admin_tagset) {
>   		blk_mq_tagset_busy_iter(ctrl->admin_tagset,
>   				nvme_cancel_request, ctrl);
> @@ -4355,6 +4354,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
>   {
>   	nvme_mpath_stop(ctrl);
>   	nvme_auth_stop(ctrl);
> +	nvme_stop_keep_alive(ctrl);
>   	nvme_stop_failfast_work(ctrl);
>   	flush_work(&ctrl->async_event_work);
>   	cancel_work_sync(&ctrl->fw_act_work);
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 49c3e46eaa1e..9f9a3b35dc64 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -2530,12 +2530,6 @@ __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
>   	 * clean up the admin queue. Same thing as above.
>   	 */
>   	nvme_quiesce_admin_queue(&ctrl->ctrl);
> -
> -	/*
> -	 * Open-coding nvme_cancel_admin_tagset() as fc
> -	 * is not using nvme_cancel_request().
> -	 */
> -	nvme_stop_keep_alive(&ctrl->ctrl);
>   	blk_sync_queue(ctrl->ctrl.admin_q);
>   	blk_mq_tagset_busy_iter(&ctrl->admin_tag_set,
>   				nvme_fc_terminate_exchange, &ctrl->ctrl);
> @@ -3138,11 +3132,12 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   	nvme_unquiesce_admin_queue(&ctrl->ctrl);
>   
>   	ret = nvme_init_ctrl_finish(&ctrl->ctrl, false);
> -	if (!ret && test_bit(ASSOC_FAILED, &ctrl->flags))
> -		ret = -EIO;
>   	if (ret)
>   		goto out_disconnect_admin_queue;
> -
> +	if (test_bit(ASSOC_FAILED, &ctrl->flags)) {
> +		ret = -EIO;
> +		goto out_stop_keep_alive;
> +	}
>   	/* sanity checks */
>   
>   	/* FC-NVME does not have other data in the capsule */
> @@ -3150,7 +3145,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   		dev_err(ctrl->ctrl.device, "icdoff %d is not supported!\n",
>   				ctrl->ctrl.icdoff);
>   		ret = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> -		goto out_disconnect_admin_queue;
> +		goto out_stop_keep_alive;
>   	}
>   
>   	/* FC-NVME supports normal SGL Data Block Descriptors */
> @@ -3158,7 +3153,7 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   		dev_err(ctrl->ctrl.device,
>   			"Mandatory sgls are not supported!\n");
>   		ret = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
> -		goto out_disconnect_admin_queue;
> +		goto out_stop_keep_alive;
>   	}
>   
>   	if (opts->queue_size > ctrl->ctrl.maxcmd) {
> @@ -3205,6 +3200,8 @@ nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
>   
>   out_term_aen_ops:
>   	nvme_fc_term_aen_ops(ctrl);
> +out_stop_keep_alive:
> +	nvme_stop_keep_alive(&ctrl->ctrl);
>   out_disconnect_admin_queue:
>   	dev_warn(ctrl->ctrl.device,
>   		"NVME-FC{%d}: create_assoc failed, assoc_id %llx ret %d\n",
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index a7fea4cbacd7..6d178d555920 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1080,6 +1080,7 @@ static int nvme_rdma_setup_ctrl(struct nvme_rdma_ctrl *ctrl, bool new)
>   		nvme_rdma_free_io_queues(ctrl);
>   	}
>   destroy_admin:
> +	nvme_stop_keep_alive(&ctrl->ctrl);
>   	nvme_quiesce_admin_queue(&ctrl->ctrl);
>   	blk_sync_queue(ctrl->ctrl.admin_q);
>   	nvme_rdma_stop_queue(&ctrl->queues[0]);
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 6ed794815517..ddcd23fb8b75 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2237,6 +2237,7 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
>   		nvme_tcp_destroy_io_queues(ctrl, new);
>   	}
>   destroy_admin:
> +	nvme_stop_keep_alive(ctrl);
>   	nvme_tcp_teardown_admin_queue(ctrl, false);
>   	return ret;
>   }