[PATCH] nvme-tcp: always fail a request when sending it failed

[PATCH] nvme-tcp: always fail a request when sending it failed

[Sagi Grimberg]

queue stoppage and inflight requests cancellation is fully fenced from
io_work and thus failing a request from this context. Hence we don't
need to try to guess from the socket retcode if this failure is because
the queue is about to be torn down or not.

We are perfectly safe to just fail it, the request will not be cancelled
later on.

This solves possible very long shutdown delays when the users issues a
'nvme disconnect-all'

Reported-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/tcp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index bb67538d241b..009c2cf3f106 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1180,8 +1180,7 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
 	} else if (ret < 0) {
 		dev_err(queue->ctrl->ctrl.device,
 			"failed to send request %d\n", ret);
-		if (ret != -EPIPE && ret != -ECONNRESET)
-			nvme_tcp_fail_request(queue->request);
+		nvme_tcp_fail_request(queue->request);
 		nvme_tcp_done_send_req(queue);
 	}
 	return ret;
-- 
2.34.1

Re: [PATCH] nvme-tcp: always fail a request when sending it failed

[Daniel Wagner]

On Sun, Jun 26, 2022 at 12:24:51PM +0300, Sagi Grimberg wrote:
> queue stoppage and inflight requests cancellation is fully fenced from
> io_work and thus failing a request from this context. Hence we don't
> need to try to guess from the socket retcode if this failure is because
> the queue is about to be torn down or not.
> 
> We are perfectly safe to just fail it, the request will not be cancelled
> later on.
> 
> This solves possible very long shutdown delays when the users issues a
> 'nvme disconnect-all'
> 
> Reported-by: Daniel Wagner <dwagner@suse.de>
> Signed-off-by: Sagi Grimberg <sagi@grimberg.me>

I've tested the case where we disconnect all controllers in a multipath
setup. In this scenario we reliable ended up waiting for a register
read/write in the shutdown path. With this path, the I/O fails
immediately and we don't wait the extra time out time for the I/O to fail
eventually.

Tested-by: Daniel Wagner <dwagner@suse.de>

> ---
>  drivers/nvme/host/tcp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index bb67538d241b..009c2cf3f106 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1180,8 +1180,7 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
>  	} else if (ret < 0) {
>  		dev_err(queue->ctrl->ctrl.device,
>  			"failed to send request %d\n", ret);
> -		if (ret != -EPIPE && ret != -ECONNRESET)
> -			nvme_tcp_fail_request(queue->request);
> +		nvme_tcp_fail_request(queue->request);
>  		nvme_tcp_done_send_req(queue);
>  	}
>  	return ret;
> -- 
> 2.34.1
> 

Re: [PATCH] nvme-tcp: always fail a request when sending it failed

[Christoph Hellwig]

Thanks,

added to nvme-5.19.