Re: [PATCH 09/10] nvme/nvme-fabrics: move reset ctrl flow to common code

[James Smart <jsmart2021@gmail.com>]

On 10/20/2021 3:38 AM, Max Gurtovoy wrote:
> Reset work is duplicated in RDMA and TCP transports. Move this logic
> to common code. For that, introduce a new ctrl op to teardown a ctrl.
> 
> Also update the RDMA/TCP transport drivers to use this API and
> remove the duplicated code.
> 
> Make nvmf_reconnect_or_remove function static since it's only used
> inside the fabrics driver now.
> 
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
>   drivers/nvme/host/fabrics.c | 29 +++++++++++++++++++++++--
>   drivers/nvme/host/fabrics.h |  1 -
>   drivers/nvme/host/nvme.h    |  1 +
>   drivers/nvme/host/rdma.c    | 43 ++++++++-----------------------------
>   drivers/nvme/host/tcp.c     | 27 +----------------------
>   5 files changed, 38 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index e50f6b32a286..e13891619de0 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -472,7 +472,7 @@ bool nvmf_should_reconnect(struct nvme_ctrl *ctrl)
>   }
>   EXPORT_SYMBOL_GPL(nvmf_should_reconnect);
>   
> -void nvmf_reconnect_or_remove(struct nvme_ctrl *ctrl)
> +static void nvmf_reconnect_or_remove(struct nvme_ctrl *ctrl)
>   {
>   	/* If we are resetting/deleting then do nothing */
>   	if (ctrl->state != NVME_CTRL_CONNECTING) {
> @@ -491,7 +491,6 @@ void nvmf_reconnect_or_remove(struct nvme_ctrl *ctrl)
>   		nvme_delete_ctrl(ctrl);
>   	}
>   }
> -EXPORT_SYMBOL_GPL(nvmf_reconnect_or_remove);
>   
>   static void nvmf_error_recovery_work(struct work_struct *work)
>   {
> @@ -548,10 +547,36 @@ static void nvmf_reconnect_ctrl_work(struct work_struct *work)
>   	nvmf_reconnect_or_remove(ctrl);
>   }
>   
> +void nvmf_reset_ctrl_work(struct work_struct *work)
> +{
> +	struct nvme_ctrl *ctrl =
> +		container_of(work, struct nvme_ctrl, reset_work);
> +
> +	nvme_stop_ctrl(ctrl);
> +	ctrl->ops->teardown_ctrl(ctrl, false);
> +
> +	if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) {
> +		/* state change failure is ok if we started ctrl delete */
> +		WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING &&
> +			     ctrl->state != NVME_CTRL_DELETING_NOIO);
> +		return;
> +	}
> +
> +	if (ctrl->ops->setup_ctrl(ctrl, false))
> +		goto out_fail;
> +
> +	return;
> +
> +out_fail:
> +	++ctrl->nr_reconnects;
> +	nvmf_reconnect_or_remove(ctrl);
> +}
> +

This is close to what FC could use - just need a callout to check for 
connectivity and schedule a connect rather than attempt to connect to 
something not there.

Rest is fine for rdma/tcp. We would need something different to 
commonize this path for all 3 transports. Something to do later.

-- james