linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps)
@ 2017-04-21 23:19 Andy Lutomirski
  2017-04-21 23:19 ` [PATCH 1/3] nvme: Fix APST comment Andy Lutomirski
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Andy Lutomirski @ 2017-04-21 23:19 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: linux-kernel, Kai-Heng Feng, linux-nvme, Andy Lutomirski

Hi all-

These are APST improvements for 4.12 or so.  The first one fixes a
buggy comment.  The second makes debugging easier.  The third makes
it possible to force APST on despite quirks.

Andy Lutomirski (3):
  nvme: Fix APST comment
  nvme: Display raw APST configuration via DYNAMIC_DEBUG
  nvme: Add nvme_core.force_apst to ignore the NO_APST quirk

 drivers/nvme/host/core.c | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

-- 
2.9.3

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

* [PATCH 1/3] nvme: Fix APST comment
  2017-04-21 23:19 [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Andy Lutomirski
@ 2017-04-21 23:19 ` Andy Lutomirski
  2017-04-23  8:12   ` Christoph Hellwig
  2017-04-21 23:19 ` [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG Andy Lutomirski
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Andy Lutomirski @ 2017-04-21 23:19 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: linux-kernel, Kai-Heng Feng, linux-nvme, Andy Lutomirski

There was a typo in the description of the timeout heuristic.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index eeb409c287b8..b0c692a14e9b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1267,7 +1267,7 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 	 * heuristic: we are willing to spend at most 2% of the time
 	 * transitioning between power states.  Therefore, when running
 	 * in any given state, we will enter the next lower-power
-	 * non-operational state after waiting 100 * (enlat + exlat)
+	 * non-operational state after waiting 50 * (enlat + exlat)
 	 * microseconds, as long as that state's total latency is under
 	 * the requested maximum latency.
 	 *
-- 
2.9.3

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

* [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG
  2017-04-21 23:19 [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Andy Lutomirski
  2017-04-21 23:19 ` [PATCH 1/3] nvme: Fix APST comment Andy Lutomirski
@ 2017-04-21 23:19 ` Andy Lutomirski
  2017-04-23  8:12   ` Christoph Hellwig
  2017-04-21 23:19 ` [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk Andy Lutomirski
  2017-04-25  4:04 ` [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Jens Axboe
  3 siblings, 1 reply; 9+ messages in thread
From: Andy Lutomirski @ 2017-04-21 23:19 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: linux-kernel, Kai-Heng Feng, linux-nvme, Andy Lutomirski

Debugging APST is currently a bit of a pain.  This gives optional
simple log messages that describe the APST state.

The easiest way to use this is probably with the nvme_core.dyndbg=+p
module parameter.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/nvme/host/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b0c692a14e9b..05ba4f8bb73b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1278,6 +1278,8 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 
 	unsigned apste;
 	struct nvme_feat_auto_pst *table;
+	u64 max_lat_us = 0;
+	int max_ps = -1;
 	int ret;
 
 	/*
@@ -1299,6 +1301,7 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 	if (ctrl->ps_max_latency_us == 0) {
 		/* Turn off APST. */
 		apste = 0;
+		dev_dbg(ctrl->device, "APST disabled\n");
 	} else {
 		__le64 target = cpu_to_le64(0);
 		int state;
@@ -1348,9 +1351,22 @@ static void nvme_configure_apst(struct nvme_ctrl *ctrl)
 
 			target = cpu_to_le64((state << 3) |
 					     (transition_ms << 8));
+
+			if (max_ps == -1)
+				max_ps = state;
+
+			if (total_latency_us > max_lat_us)
+				max_lat_us = total_latency_us;
 		}
 
 		apste = 1;
+
+		if (max_ps == -1) {
+			dev_dbg(ctrl->device, "APST enabled but no non-operational states are available\n");
+		} else {
+			dev_dbg(ctrl->device, "APST enabled: max PS = %d, max round-trip latency = %lluus, table = %*phN\n",
+				max_ps, max_lat_us, (int)sizeof(*table), table);
+		}
 	}
 
 	ret = nvme_set_features(ctrl, NVME_FEAT_AUTO_PST, apste,
-- 
2.9.3

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

* [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
  2017-04-21 23:19 [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Andy Lutomirski
  2017-04-21 23:19 ` [PATCH 1/3] nvme: Fix APST comment Andy Lutomirski
  2017-04-21 23:19 ` [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG Andy Lutomirski
@ 2017-04-21 23:19 ` Andy Lutomirski
  2017-04-23  8:12   ` Christoph Hellwig
  2017-04-25  4:04 ` [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Jens Axboe
  3 siblings, 1 reply; 9+ messages in thread
From: Andy Lutomirski @ 2017-04-21 23:19 UTC (permalink / raw)
  To: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch
  Cc: linux-kernel, Kai-Heng Feng, linux-nvme, Andy Lutomirski

We're probably going to be stuck quirking APST off on an over-broad
range of devices for 4.11.  Let's make it easy to override the quirk
for testing.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 drivers/nvme/host/core.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 05ba4f8bb73b..5249027a76ca 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -62,6 +62,10 @@ module_param(default_ps_max_latency_us, ulong, 0644);
 MODULE_PARM_DESC(default_ps_max_latency_us,
 		 "max power saving latency for new devices; use PM QOS to change per device");
 
+static bool force_apst;
+module_param(force_apst, bool, 0644);
+MODULE_PARM_DESC(force_apst, "allow APST for newly enumerated devices even if quirked off");
+
 static LIST_HEAD(nvme_ctrl_list);
 static DEFINE_SPINLOCK(dev_list_lock);
 
@@ -1504,6 +1508,11 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 		}
 	}
 
+	if (force_apst && (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS)) {
+		dev_warn(ctrl->dev, "forcibly allowing all power states due to nvme_core.force_apst -- use at your own risk\n");
+		ctrl->quirks &= ~NVME_QUIRK_NO_DEEPEST_PS;
+	}
+
 	ctrl->oacs = le16_to_cpu(id->oacs);
 	ctrl->vid = le16_to_cpu(id->vid);
 	ctrl->oncs = le16_to_cpup(&id->oncs);
@@ -1526,7 +1535,16 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 
 	ctrl->npss = id->npss;
 	prev_apsta = ctrl->apsta;
-	ctrl->apsta = (ctrl->quirks & NVME_QUIRK_NO_APST) ? 0 : id->apsta;
+	if (ctrl->quirks & NVME_QUIRK_NO_APST) {
+		if (force_apst && id->apsta) {
+			dev_warn(ctrl->dev, "forcibly allowing APST due to nvme_core.force_apst -- use at your own risk\n");
+			ctrl->apsta = 1;
+		} else {
+			ctrl->apsta = 0;
+		}
+	} else {
+		ctrl->apsta = id->apsta;
+	}
 	memcpy(ctrl->psd, id->psd, sizeof(ctrl->psd));
 
 	if (ctrl->ops->is_fabrics) {
-- 
2.9.3

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

* Re: [PATCH 1/3] nvme: Fix APST comment
  2017-04-21 23:19 ` [PATCH 1/3] nvme: Fix APST comment Andy Lutomirski
@ 2017-04-23  8:12   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-04-23  8:12 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch,
	linux-kernel, Kai-Heng Feng, linux-nvme

On Fri, Apr 21, 2017 at 04:19:22PM -0700, Andy Lutomirski wrote:
> There was a typo in the description of the timeout heuristic.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG
  2017-04-21 23:19 ` [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG Andy Lutomirski
@ 2017-04-23  8:12   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-04-23  8:12 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch,
	linux-kernel, Kai-Heng Feng, linux-nvme

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
  2017-04-21 23:19 ` [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk Andy Lutomirski
@ 2017-04-23  8:12   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-04-23  8:12 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, Keith Busch,
	linux-kernel, Kai-Heng Feng, linux-nvme

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps)
  2017-04-21 23:19 [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Andy Lutomirski
                   ` (2 preceding siblings ...)
  2017-04-21 23:19 ` [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk Andy Lutomirski
@ 2017-04-25  4:04 ` Jens Axboe
  2017-04-25  7:19   ` Christoph Hellwig
  3 siblings, 1 reply; 9+ messages in thread
From: Jens Axboe @ 2017-04-25  4:04 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Christoph Hellwig, Sagi Grimberg, Keith Busch, linux-kernel,
	Kai-Heng Feng, linux-nvme

On Fri, Apr 21 2017, Andy Lutomirski wrote:
> Hi all-
> 
> These are APST improvements for 4.12 or so.  The first one fixes a
> buggy comment.  The second makes debugging easier.  The third makes
> it possible to force APST on despite quirks.
> 
> Andy Lutomirski (3):
>   nvme: Fix APST comment
>   nvme: Display raw APST configuration via DYNAMIC_DEBUG
>   nvme: Add nvme_core.force_apst to ignore the NO_APST quirk
> 
>  drivers/nvme/host/core.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)

I have added these for 4.12, thanks Andy.

-- 
Jens Axboe

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

* Re: [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps)
  2017-04-25  4:04 ` [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Jens Axboe
@ 2017-04-25  7:19   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2017-04-25  7:19 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Andy Lutomirski, Christoph Hellwig, Sagi Grimberg, Keith Busch,
	linux-kernel, Kai-Heng Feng, linux-nvme

On Mon, Apr 24, 2017 at 10:04:02PM -0600, Jens Axboe wrote:
> I have added these for 4.12, thanks Andy.

This shouldn't conflict with what we've queued up in the nvme tree so
we should be ok..

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

end of thread, other threads:[~2017-04-25  7:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-21 23:19 [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Andy Lutomirski
2017-04-21 23:19 ` [PATCH 1/3] nvme: Fix APST comment Andy Lutomirski
2017-04-23  8:12   ` Christoph Hellwig
2017-04-21 23:19 ` [PATCH 2/3] nvme: Display raw APST configuration via DYNAMIC_DEBUG Andy Lutomirski
2017-04-23  8:12   ` Christoph Hellwig
2017-04-21 23:19 ` [PATCH 3/3] nvme: Add nvme_core.force_apst to ignore the NO_APST quirk Andy Lutomirski
2017-04-23  8:12   ` Christoph Hellwig
2017-04-25  4:04 ` [PATCH 0/3] nvme: APST improvements (for 4.12, perhaps) Jens Axboe
2017-04-25  7:19   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).