Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv4 0/2] nvme: sanitize KATO setting
@ 2021-04-16 11:46 Hannes Reinecke
  2021-04-16 11:46 ` [PATCH 1/2] " Hannes Reinecke
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Hannes Reinecke @ 2021-04-16 11:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Hi all,

here is a patchset to sanitize and expose the KATO value to sysfs.
We should follow the rules outlined in the NVMe base spec for
calculating the KATO value and interval.
And by exposing it to sysfs we allow userspace to identify persistent
discovery controllers (which will have a non-zero KATO value); a patch
for that has been sent to the nvme-cli github repository.

As usual, comments and reviews are welcome.

Changes to v2:
- Include reviews from Christoph
- Rebase on top of nvme-5.13

Hannes Reinecke (2):
  nvme: sanitize KATO setting
  nvme: add 'kato' sysfs attribute

 drivers/nvme/host/core.c    | 20 +++++++++++++++++---
 drivers/nvme/host/fabrics.c |  4 +---
 drivers/nvme/host/nvme.h    |  1 -
 3 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.29.2


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] nvme: sanitize KATO setting
  2021-04-16 11:46 [PATCHv4 0/2] nvme: sanitize KATO setting Hannes Reinecke
@ 2021-04-16 11:46 ` Hannes Reinecke
  2021-04-16 11:46 ` [PATCH 2/2] nvme: add 'kato' sysfs attribute Hannes Reinecke
  2021-04-20  8:07 ` [PATCHv4 0/2] nvme: sanitize KATO setting Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Hannes Reinecke @ 2021-04-16 11:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

According to the NVMe base spec the KATO commands should be sent
at half of the KATO interval, to properly account for round-trip
times.
As we now will only ever send one KATO command per connection we
can easily use the recommended values.
This also fixes a potential issue where the request timeout for
the KATO command does not match the value in the connect command,
which might be causing spurious connection drops from the target.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 40f08e6325ef..f58e196d5b4b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1109,6 +1109,18 @@ void nvme_execute_passthru_rq(struct request *rq)
 }
 EXPORT_SYMBOL_NS_GPL(nvme_execute_passthru_rq, NVME_TARGET_PASSTHRU);
 
+/*
+ * Recommended frequency for KATO commands per NVMe 1.4 section 7.12.1:
+ *
+ * The host should send Keep Alive commands at half of the
+ * Keep Alive Timeout accounting for transport roundtrip times [..].
+ */
+static inline void nvme_queue_keep_alive_work(struct nvme_ctrl *ctrl)
+{
+	queue_delayed_work(nvme_wq, &ctrl->ka_work,
+			   ctrl->kato * HZ / 2);
+}
+
 static void nvme_keep_alive_end_io(struct request *rq, blk_status_t status)
 {
 	struct nvme_ctrl *ctrl = rq->end_io_data;
@@ -1131,7 +1143,7 @@ static void nvme_keep_alive_end_io(struct request *rq, blk_status_t status)
 		startka = true;
 	spin_unlock_irqrestore(&ctrl->lock, flags);
 	if (startka)
-		queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
+		nvme_queue_keep_alive_work(ctrl);
 }
 
 static int nvme_keep_alive(struct nvme_ctrl *ctrl)
@@ -1161,7 +1173,7 @@ static void nvme_keep_alive_work(struct work_struct *work)
 		dev_dbg(ctrl->device,
 			"reschedule traffic based keep-alive timer\n");
 		ctrl->comp_seen = false;
-		queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
+		nvme_queue_keep_alive_work(ctrl);
 		return;
 	}
 
@@ -1178,7 +1190,7 @@ static void nvme_start_keep_alive(struct nvme_ctrl *ctrl)
 	if (unlikely(ctrl->kato == 0))
 		return;
 
-	queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
+	nvme_queue_keep_alive_work(ctrl);
 }
 
 void nvme_stop_keep_alive(struct nvme_ctrl *ctrl)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 604ab0e5a2ad..13c2747e3d00 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -379,10 +379,8 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 
 	/*
 	 * Set keep-alive timeout in seconds granularity (ms * 1000)
-	 * and add a grace period for controller kato enforcement
 	 */
-	cmd.connect.kato = ctrl->kato ?
-		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000) : 0;
+	cmd.connect.kato = cpu_to_le32(ctrl->kato * 1000);
 
 	if (ctrl->opts->disable_sqflow)
 		cmd.connect.cattr |= NVME_CONNECT_DISABLE_SQFLOW;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index c6102ce83bb4..49276186d5bd 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -27,7 +27,6 @@ extern unsigned int admin_timeout;
 #define NVME_ADMIN_TIMEOUT	(admin_timeout * HZ)
 
 #define NVME_DEFAULT_KATO	5
-#define NVME_KATO_GRACE		10
 
 #ifdef CONFIG_ARCH_NO_SG_CHAIN
 #define  NVME_INLINE_SG_CNT  0
-- 
2.29.2


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] nvme: add 'kato' sysfs attribute
  2021-04-16 11:46 [PATCHv4 0/2] nvme: sanitize KATO setting Hannes Reinecke
  2021-04-16 11:46 ` [PATCH 1/2] " Hannes Reinecke
@ 2021-04-16 11:46 ` Hannes Reinecke
  2021-04-20  8:07 ` [PATCHv4 0/2] nvme: sanitize KATO setting Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Hannes Reinecke @ 2021-04-16 11:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Add a 'kato' controller sysfs attribute to display the current
keep-alive timeout value (if any). This allows userspace to identify
persistent discovery controllers, as these will have a non-zero
KATO value.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f58e196d5b4b..67f0607055e8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3173,6 +3173,7 @@ nvme_show_int_function(cntlid);
 nvme_show_int_function(numa_node);
 nvme_show_int_function(queue_count);
 nvme_show_int_function(sqsize);
+nvme_show_int_function(kato);
 
 static ssize_t nvme_sysfs_delete(struct device *dev,
 				struct device_attribute *attr, const char *buf,
@@ -3370,6 +3371,7 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_ctrl_loss_tmo.attr,
 	&dev_attr_reconnect_delay.attr,
 	&dev_attr_fast_io_fail_tmo.attr,
+	&dev_attr_kato.attr,
 	NULL
 };
 
-- 
2.29.2


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCHv4 0/2] nvme: sanitize KATO setting
  2021-04-16 11:46 [PATCHv4 0/2] nvme: sanitize KATO setting Hannes Reinecke
  2021-04-16 11:46 ` [PATCH 1/2] " Hannes Reinecke
  2021-04-16 11:46 ` [PATCH 2/2] nvme: add 'kato' sysfs attribute Hannes Reinecke
@ 2021-04-20  8:07 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-04-20  8:07 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-nvme

Thanks,

applied to nvme-5.13.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] nvme: add 'kato' sysfs attribute
  2021-03-11 10:37 ` [PATCH 2/2] nvme: add 'kato' sysfs attribute Hannes Reinecke
@ 2021-03-15 17:22   ` Sagi Grimberg
  0 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2021-03-15 17:22 UTC (permalink / raw)
  To: Hannes Reinecke, Christoph Hellwig; +Cc: Keith Busch, linux-nvme

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] nvme: add 'kato' sysfs attribute
  2021-03-11 10:37 [PATCHv3 " Hannes Reinecke
@ 2021-03-11 10:37 ` Hannes Reinecke
  2021-03-15 17:22   ` Sagi Grimberg
  0 siblings, 1 reply; 6+ messages in thread
From: Hannes Reinecke @ 2021-03-11 10:37 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Sagi Grimberg, Keith Busch, linux-nvme, Hannes Reinecke

Add a 'kato' controller sysfs attribute to display the current
keep-alive timeout value (if any). This allows userspace to identify
persistent discovery controllers, as these will have a non-zero
KATO value.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/host/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b56903982b27..206e6f883ca8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3540,6 +3540,7 @@ nvme_show_int_function(cntlid);
 nvme_show_int_function(numa_node);
 nvme_show_int_function(queue_count);
 nvme_show_int_function(sqsize);
+nvme_show_int_function(kato);
 
 static ssize_t nvme_sysfs_delete(struct device *dev,
 				struct device_attribute *attr, const char *buf,
@@ -3706,6 +3707,7 @@ static struct attribute *nvme_dev_attrs[] = {
 	&dev_attr_hostid.attr,
 	&dev_attr_ctrl_loss_tmo.attr,
 	&dev_attr_reconnect_delay.attr,
+	&dev_attr_kato.attr,
 	NULL
 };
 
-- 
2.29.2


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 11:46 [PATCHv4 0/2] nvme: sanitize KATO setting Hannes Reinecke
2021-04-16 11:46 ` [PATCH 1/2] " Hannes Reinecke
2021-04-16 11:46 ` [PATCH 2/2] nvme: add 'kato' sysfs attribute Hannes Reinecke
2021-04-20  8:07 ` [PATCHv4 0/2] nvme: sanitize KATO setting Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2021-03-11 10:37 [PATCHv3 " Hannes Reinecke
2021-03-11 10:37 ` [PATCH 2/2] nvme: add 'kato' sysfs attribute Hannes Reinecke
2021-03-15 17:22   ` Sagi Grimberg

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git