public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
From: Hannes Reinecke <hare@suse.de>
To: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>, Keith Busch <kbusch@kernel.org>,
	linux-nvme@lists.infradead.org, Hannes Reinecke <hare@suse.de>
Subject: [PATCH 3/3] nvme-tcp: delay error recovery until the next KATO interval
Date: Fri,  8 Sep 2023 12:00:49 +0200	[thread overview]
Message-ID: <20230908100049.80809-4-hare@suse.de> (raw)
In-Reply-To: <20230908100049.80809-1-hare@suse.de>

Section 3.9 of the NVMe base spec states that:

 If a Keep Alive Timer expires:
 a) the controller shall ...
 and
 b) the host assumes all outstanding commands are not completed
   and re-issues commands as appropriate.

which means that we should _not_ retry any commands until KATO
expired (or the equivalent of the default KATO timeout if KATO
is not active).
And there are some target which reported a data corruption if
we retry commands immediately after reporting a command timeout
or link loss.
So always delay error recovery until the next KATO interval
to give controllers enough time to detect a KATO failure.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/core.c |  3 ++-
 drivers/nvme/host/nvme.h |  1 +
 drivers/nvme/host/tcp.c  | 12 ++++++++++--
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f3a01b79148c..2d546612b20a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1175,7 +1175,7 @@ EXPORT_SYMBOL_NS_GPL(nvme_passthru_end, NVME_TARGET_PASSTHRU);
  *   The host should send Keep Alive commands at half of the Keep Alive Timeout
  *   accounting for transport roundtrip times [..].
  */
-static unsigned long nvme_keep_alive_work_period(struct nvme_ctrl *ctrl)
+unsigned long nvme_keep_alive_work_period(struct nvme_ctrl *ctrl)
 {
 	unsigned long delay = ctrl->kato * HZ / 2;
 
@@ -1189,6 +1189,7 @@ static unsigned long nvme_keep_alive_work_period(struct nvme_ctrl *ctrl)
 		delay /= 2;
 	return delay;
 }
+EXPORT_SYMBOL_GPL(nvme_keep_alive_work_period);
 
 static void nvme_queue_keep_alive_work(struct nvme_ctrl *ctrl)
 {
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f35647c470af..38845e5fa5a3 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -777,6 +777,7 @@ void nvme_unfreeze(struct nvme_ctrl *ctrl);
 void nvme_wait_freeze(struct nvme_ctrl *ctrl);
 int nvme_wait_freeze_timeout(struct nvme_ctrl *ctrl, long timeout);
 void nvme_start_freeze(struct nvme_ctrl *ctrl);
+unsigned long nvme_keep_alive_work_period(struct nvme_ctrl *ctrl);
 
 static inline enum req_op nvme_req_op(struct nvme_command *cmd)
 {
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 990bd863ae34..7bfdb05b2f17 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -533,13 +533,21 @@ static void nvme_tcp_init_recv_ctx(struct nvme_tcp_queue *queue)
 	queue->ddgst_remaining = 0;
 }
 
+/*
+ * Error recovery needs to be started after KATO expired,
+ * always delay until the next KATO interval before
+ * starting error recovery.
+ */
 static void nvme_tcp_error_recovery(struct nvme_ctrl *ctrl)
 {
+	unsigned long delay = nvme_keep_alive_work_period(ctrl);
+
 	if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING))
 		return;
 
-	dev_warn(ctrl->device, "starting error recovery\n");
-	queue_delayed_work(nvme_reset_wq, &to_tcp_ctrl(ctrl)->err_work, 0);
+	dev_warn(ctrl->device, "starting error recovery in %lu seconds\n",
+		 delay / HZ);
+	queue_delayed_work(nvme_reset_wq, &to_tcp_ctrl(ctrl)->err_work, delay);
 }
 
 static int nvme_tcp_process_nvme_cqe(struct nvme_tcp_queue *queue,
-- 
2.35.3



  parent reply	other threads:[~2023-09-08 10:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-08 10:00 [PATCH 0/3] nvme-tcp: start error recovery after KATO Hannes Reinecke
2023-09-08 10:00 ` [PATCH 1/3] nvme-tcp: Do not terminate commands when in RESETTING Hannes Reinecke
2023-09-12 11:54   ` Sagi Grimberg
2023-09-12 12:06     ` Hannes Reinecke
2023-09-12 12:15       ` Sagi Grimberg
2023-09-12 12:59         ` Hannes Reinecke
2023-09-12 13:02           ` Sagi Grimberg
2023-09-08 10:00 ` [PATCH 2/3] nvme-tcp: make 'err_work' a delayed work Hannes Reinecke
2023-09-08 10:00 ` Hannes Reinecke [this message]
2023-09-12 12:17   ` [PATCH 3/3] nvme-tcp: delay error recovery until the next KATO interval Sagi Grimberg
2023-09-12 11:51 ` [PATCH 0/3] nvme-tcp: start error recovery after KATO Sagi Grimberg
2023-09-12 11:56   ` Hannes Reinecke
2023-09-12 12:12     ` Sagi Grimberg
2023-09-12 13:25       ` Hannes Reinecke
2023-09-12 13:43         ` Sagi Grimberg
2023-12-04 10:30           ` Sagi Grimberg
2023-12-04 11:37             ` Hannes Reinecke
2023-12-04 13:27               ` Sagi Grimberg
2023-12-04 14:15                 ` Hannes Reinecke
2023-12-04 16:11                   ` Sagi Grimberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230908100049.80809-4-hare@suse.de \
    --to=hare@suse.de \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox