All of lore.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.