[PATCH V2 2/2] nvme: retry commands based on ACRE result

[Chao Leng]

On 2021/1/13 22:35, Minwoo Im wrote:
> Introduce acre flag for Advanced Command Retry Enable to controller
> instance to decide whether to retry or not in error cases.  This flag is
> set if Set Features for Host Behavior Support with ACRE bit set is
> successfully done during reset_work.
> 
> This patch also fixes nvme_decide_disposition() because all the nvme
> requests are initialized with REQ_FAILFAST_DRIVER to req->cmd_flags so
> that blk_noretry_request(req) is always true.  Check ctrl->acre first
> and if it's not host path error, then it will be retried.
> 
> Cc: Chao Leng <lengchao at huawei.com>
> Signed-off-by: Minwoo Im <minwoo.im.dev at gmail.com>
> ---
>   drivers/nvme/host/core.c | 16 ++++++++++++++--
>   drivers/nvme/host/nvme.h |  1 +
>   2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index a8cee380b3c0..391bb2d08d0f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -303,8 +303,7 @@ static inline enum nvme_disposition nvme_decide_disposition(struct request *req)
>   	if (likely(nvme_req(req)->status == 0))
>   		return COMPLETE;
>   
> -	if (blk_noretry_request(req) ||
> -	    (nvme_req(req)->status & NVME_SC_DNR) ||
> +	if ((nvme_req(req)->status & NVME_SC_DNR) ||
>   	    nvme_req(req)->retries >= nvme_max_retries)
>   		return COMPLETE;
>   
> @@ -317,6 +316,13 @@ static inline enum nvme_disposition nvme_decide_disposition(struct request *req)
>   			return COMPLETE;
>   	}
>   
> +	if (nvme_req(req)->ctrl->acre) {
> +		if (!nvme_is_path_error(nvme_req(req)->status) &&
> +				!blk_queue_dying(req->q))
> +			return RETRY;
> +	} else if (blk_noretry_request(req))
> +		return COMPLETE;
If acre is true and nvme_req(req)->status is nvme path error and
blk_noretry_request(req) is true, we need return complete for REQ_FAILFAST_XX.
Otherwise, connection process may takes long time in abnormal scenarios.
We can do like this:

+    if (nvme_req(req)->ctrl->acre &&
+        !nvme_is_path_error(nvme_req(req)->status) &&
+        !blk_queue_dying(req->q))
+        return RETRY;
+
+    if (blk_noretry_request(req))
+        return COMPLETE;
> +
>   	return RETRY;
>   }
>   
> @@ -2498,6 +2504,8 @@ static int nvme_configure_acre(struct nvme_ctrl *ctrl)
>   	struct nvme_feat_host_behavior *host;
>   	int ret;
>   
> +	ctrl->acre = false;
> +
>   	/* Don't bother enabling the feature if retry delay is not reported */
>   	if (!ctrl->crdt[0] && !ctrl->crdt[1] && !ctrl->crdt[2])
>   		return 0;
> @@ -2509,6 +2517,10 @@ static int nvme_configure_acre(struct nvme_ctrl *ctrl)
>   	host->acre = NVME_ENABLE_ACRE;
>   	ret = nvme_set_features(ctrl, NVME_FEAT_HOST_BEHAVIOR, 0,
>   				host, sizeof(*host), NULL);
> +
> +	if (!ret)
> +		ctrl->acre = true;
> +
>   	kfree(host);
>   	return ret;
>   }
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 88a6b97247f5..db8b45e8ffde 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -270,6 +270,7 @@ struct nvme_ctrl {
>   #ifdef CONFIG_BLK_DEV_ZONED
>   	u32 max_zone_append;
>   #endif
> +	bool acre;
>   	u16 crdt[3];
>   	u16 oncs;
>   	u16 oacs;
>