[RFC PATCH] NVMe: Runtime suspend

[RFC PATCH] NVMe: Runtime suspend

[Winson Yung (wyung)]

Hi, I am new to the mailing list in case my comment mention here was discussed in the past. Please let me know the past discussion thread if it exists. Here is my comment to the patch:

* How is the current NVMe power states defined by a storage device exposed/used in the kernel? I looked at the driver/block/nvme-scsi.c, and only can see that they were mapped to supporting scsi START STOP UNIT cmd inside nvme_trans_power_state(). So is it accurate to say that no (power management) code in linux kernel take advantage of these NVMe power states?

* I think your patch is a good idea, but you should consider to extend it such that it uses runtime power management to create a dynamic power reduction model based on the idleness of IO activity. For example, if the storage drive supports 3 NVMe power states (0, 1, 2), we can let nvme_runtime_suspend() to determine which NVMe power states to use among 0, 1 or 2 based on the IO activity. This way creates a more scalable solution that gives better balance between power saving and performance (i.e. exit latency) in the middle of D0 and D3.

* As for your observation of the potential issue, I think if a device is in suspended mode, driver should allowed to post a command to IO queue as long as the device can be awaken. Of course, the service of the command will be queued and delay serviced only after runtime resume is completed.

/Winson

[RFC PATCH] NVMe: Runtime suspend

[Keith Busch]

On Sun, 18 May 2014, Winson Yung (wyung) wrote:
> * How is the current NVMe power states defined by a storage device
> exposed/used in the kernel? I looked at the driver/block/nvme-scsi.c, and
> only can see that they were mapped to supporting scsi START STOP UNIT cmd
> inside nvme_trans_power_state(). So is it accurate to say that no (power
> management) code in linux kernel take advantage of these NVMe power states?

Yes, that's true that we're not making use of power states in the core
driver. You can only test this out on your device with IOCTLs, either
passthrough or the SG_IO you mentioned.

> * I think your patch is a good idea, but you should consider to extend it
> such that it uses runtime power management to create a dynamic power
> reduction model based on the idleness of IO activity. For example, if the
> storage drive supports 3 NVMe power states (0, 1, 2), we can let
> nvme_runtime_suspend() to determine which NVMe power states to use among 0, 1
> or 2 based on the IO activity.

That's kind of the idea, but I was only letting the user choose a policy
for idle time until runtime suspending, and which power state to go
to. It would take more driver work to allow a policy with more than 2
power states, but maybe we should support more like how the autonomous
power transitions feature allows.

> This way creates a more scalable solution that gives better balance between
> power saving and performance (i.e. exit latency) in the middle of D0 and D3.

I'm not sure we want to take let the device go to D3 at runtime
suspend. D3 would require we reinitialize the device and add more
overhead in going from idle to resume. Maybe it's acceptable, but I was
assuming not.

> * As for your observation of the potential issue, I think if a device is in
> suspended mode, driver should allowed to post a command to IO queue as long
> as the device can be awaken. Of course, the service of the command will be
> queued and delay serviced only after runtime resume is completed.

I actually did get some clarification on this. The spec defines that a
device in a non-operational power automatically transistions to the last
operational power state that it was in when an IO submission occurs. This
was in the autonomous power state section, but I'm told this applies to
the device even when not using autonomous power states.

[RFC PATCH] NVMe: Runtime suspend

[Winson Yung (wyung)]

See my comment inline below

On 5/18/2014 11:00 AM, Keith Busch wrote:> On Sun, 18 May 2014, Winson Yung (wyung) wrote:
>> * How is the current NVMe power states defined by a storage device
>> exposed/used in the kernel? I looked at the driver/block/nvme-scsi.c, and
>> only can see that they were mapped to supporting scsi START STOP UNIT cmd
>> inside nvme_trans_power_state(). So is it accurate to say that no (power
>> management) code in linux kernel take advantage of these NVMe power 
>> states?
> 
> Yes, that's true that we're not making use of power states in the core
> driver. You can only test this out on your device with IOCTLs, either
> passthrough or the SG_IO you mentioned.
> 
>> * I think your patch is a good idea, but you should consider to extend it
>> such that it uses runtime power management to create a dynamic power
>> reduction model based on the idleness of IO activity. For example, if the
>> storage drive supports 3 NVMe power states (0, 1, 2), we can let
>> nvme_runtime_suspend() to determine which NVMe power states to use 
>> among 0, 1
>> or 2 based on the IO activity.
> 
> That's kind of the idea, but I was only letting the user choose a policy
> for idle time until runtime suspending, and which power state to go
> to. It would take more driver work to allow a policy with more than 2
> power states, but maybe we should support more like how the autonomous
> power transitions feature allows.
> 

Agree, I think beyond 2 power states, the current implementation will be difficult to use, that is why I was proposing to have a dynamic power reduction model based on the idleness of IO activity, and let runtime PM to request different power transition automagically. Each of these NVMe power states definition are very vendor specific, so for sure it will work best with vendor's own driver.

I read from NVMe spec 1.1a, it says D1 and D2 is not recommended to be implemented. D0 is the active states, and device only enter D3 state when system going to S3/4. So it seems making sense to have NVMe PM states for either host OS (to initiate) or device (to enter autonomously) with a shallower sleep state while system is still in S0. 

What will happen if host try to initiate entering one of these NVMe PM states while device firmware is also trying to do the same? Is autonomous power transition defined by NVMe implies device firmware can transition device (if capable) to a different NVMe PM state by itself? Should the host OS use set feature cmd to disable autonomous power transition feature in this case?

>> This way creates a more scalable solution that gives better balance 
>> between
>> power saving and performance (i.e. exit latency) in the middle of D0 
>> and D3.
> 
> I'm not sure we want to take let the device go to D3 at runtime
> suspend. D3 would require we reinitialize the device and add more
> overhead in going from idle to resume. Maybe it's acceptable, but I was
> assuming not.
> 
>> * As for your observation of the potential issue, I think if a device 
>> is in
>> suspended mode, driver should allowed to post a command to IO queue as 
>> long
>> as the device can be awaken. Of course, the service of the command 
>> will be
>> queued and delay serviced only after runtime resume is completed.
> 
> I actually did get some clarification on this. The spec defines that a
> device in a non-operational power automatically transistions to the last
> operational power state that it was in when an IO submission occurs. This
> was in the autonomous power state section, but I'm told this applies to
> the device even when not using autonomous power states.

[RFC PATCH] NVMe: Runtime suspend

[Matthew Wilcox]

On Mon, May 19, 2014@04:32:34AM +0000, Winson Yung (wyung) wrote:
> Agree, I think beyond 2 power states, the current implementation will
> be difficult to use, that is why I was proposing to have a dynamic power
> reduction model based on the idleness of IO activity, and let runtime
> PM to request different power transition automagically. Each of these
> NVMe power states definition are very vendor specific, so for sure it
> will work best with vendor's own driver.

Hi Winson,

I just wanted to note that "vendor's own driver" is not really an option.
Linux distributions are very unwilling to ship different drivers for the
same hardware, and OEMs and ISVs do not want to certify against multiple
drivers.  This was why NVM Express was formed; to give everybody a common
hardware interface and have one driver for many hardware implementations.

We've already seen a couple of hardware vendors try to ship their
own Linux driver and been told "No" by their customers and the ISVs.
If there are changes you would like to the Linux driver to make your
device perform better, let's discuss how we can make that happen.  While I
work for Intel, I don't work for the part of Intel that makes SSDs.

> I read from NVMe spec 1.1a, it says D1 and D2 is not recommended to be
> implemented. D0 is the active states, and device only enter D3 state when
> system going to S3/4. So it seems making sense to have NVMe PM states
> for either host OS (to initiate) or device (to enter autonomously)
> with a shallower sleep state while system is still in S0.

D1/D2 are inactive states; reading & writing device registers don't work,
so the only point in implementing D1/D2 is for reduced latency to return
to D0.

[RFC PATCH] NVMe: Runtime suspend

[Winson Yung (wyung)]

Hi Matthew, so how you see the NVMe linux driver should support system (i.e. ultrabook and tablet) to achieve better battery life and thermal over temperature protection in S0? Correct me if I am wrong, I don't see current driver has such mechanism put in place beyond the conventional suspend/resume support, even with this patch.

/Winson

-----Original Message-----
From: Matthew Wilcox [mailto:willy@linux.intel.com] 
Sent: Monday, May 19, 2014 2:35 AM
To: Winson Yung (wyung)
Cc: Keith Busch; linux-nvme at lists.infradead.org
Subject: Re: [RFC PATCH] NVMe: Runtime suspend

On Mon, May 19, 2014@04:32:34AM +0000, Winson Yung (wyung) wrote:
> Agree, I think beyond 2 power states, the current implementation will 
> be difficult to use, that is why I was proposing to have a dynamic 
> power reduction model based on the idleness of IO activity, and let 
> runtime PM to request different power transition automagically. Each 
> of these NVMe power states definition are very vendor specific, so for 
> sure it will work best with vendor's own driver.

Hi Winson,

I just wanted to note that "vendor's own driver" is not really an option.
Linux distributions are very unwilling to ship different drivers for the same hardware, and OEMs and ISVs do not want to certify against multiple drivers.  This was why NVM Express was formed; to give everybody a common hardware interface and have one driver for many hardware implementations.

We've already seen a couple of hardware vendors try to ship their own Linux driver and been told "No" by their customers and the ISVs.
If there are changes you would like to the Linux driver to make your device perform better, let's discuss how we can make that happen.  While I work for Intel, I don't work for the part of Intel that makes SSDs.

> I read from NVMe spec 1.1a, it says D1 and D2 is not recommended to be 
> implemented. D0 is the active states, and device only enter D3 state 
> when system going to S3/4. So it seems making sense to have NVMe PM 
> states for either host OS (to initiate) or device (to enter 
> autonomously) with a shallower sleep state while system is still in S0.

D1/D2 are inactive states; reading & writing device registers don't work, so the only point in implementing D1/D2 is for reduced latency to return to D0.

[RFC PATCH] NVMe: Runtime suspend

[Keith Busch]

On Tue, 20 May 2014, Winson Yung (wyung) wrote:
> Hi Matthew,
> so how you see the NVMe linux driver should support system (i.e.
> ultrabook and tablet) to achieve better battery life and thermal over
> temperature protection in S0? Correct me if I am wrong, I don't see current
> driver has such mechanism put in place beyond the conventional suspend/resume
> support, even with this patch.

This is what the RFC patch in question was proposing to address. The
only thing the patch added was supporting NVM-e low power states with a
user-defined policy, so I'm not sure what you're missing if you don't
see that mechanism in the patch. If all you are asking is to support
more than 2 power states, then we can talk about that.

Having the policy be user-defined should make it work exactly how you'd
want for a vendor's unique power states without needing a vendor specific
driver.

I don't think we want these going into lower D-states as the latency to
get out of there is probably too high for a runtime setting: we have to do
a device shutdown and full reinitilazation for D-state transitions.

I highly suspect that devices targeted to tablets and ultrabooks will
support the autonomous power state transitions feature, but lacking that,
I wanted to at least get some feedback if a driver initiated power state
transition is even desirable.

> /Winson
>
> -----Original Message-----
> From: Matthew Wilcox [mailto:willy at linux.intel.com]
> Sent: Monday, May 19, 2014 2:35 AM
> To: Winson Yung (wyung)
> Cc: Keith Busch; linux-nvme at lists.infradead.org
> Subject: Re: [RFC PATCH] NVMe: Runtime suspend
>
> On Mon, May 19, 2014@04:32:34AM +0000, Winson Yung (wyung) wrote:
>> Agree, I think beyond 2 power states, the current implementation will
>> be difficult to use, that is why I was proposing to have a dynamic
>> power reduction model based on the idleness of IO activity, and let
>> runtime PM to request different power transition automagically. Each
>> of these NVMe power states definition are very vendor specific, so for
>> sure it will work best with vendor's own driver.
>
> Hi Winson,
>
> I just wanted to note that "vendor's own driver" is not really an option.
> Linux distributions are very unwilling to ship different drivers for the same
> hardware, and OEMs and ISVs do not want to certify against multiple drivers.
> This was why NVM Express was formed; to give everybody a common hardware
> interface and have one driver for many hardware implementations.
>
> We've already seen a couple of hardware vendors try to ship their own Linux
> driver and been told "No" by their customers and the ISVs.  If there are
> changes you would like to the Linux driver to make your device perform
> better, let's discuss how we can make that happen.  While I work for Intel, I
> don't work for the part of Intel that makes SSDs.
>
>> I read from NVMe spec 1.1a, it says D1 and D2 is not recommended to be
>> implemented. D0 is the active states, and device only enter D3 state
>> when system going to S3/4. So it seems making sense to have NVMe PM
>> states for either host OS (to initiate) or device (to enter
>> autonomously) with a shallower sleep state while system is still in S0.
>
> D1/D2 are inactive states; reading & writing device registers don't work, so
> the only point in implementing D1/D2 is for reduced latency to return to D0.

[RFC PATCH] NVMe: Runtime suspend

[Keith Busch]

This is might be an odd idea to try for a block device, but I'll throw
it out there anyway.

NVMe defines support for low power states outside the PCI power states. By
default, this patch has this ability disabled, but maybe some use cases
may wish to take advantage of this when power reduction is paramount
above IO latency.

I'm making a new "device" here because I don't want to use the
pci_dev->dev for runtime pm. That will cause the device to go to D3,
and I don't think we want that.

This patch makes it possible to set up runtime power management to
transition to the lower power states the device supports, and allowing
the user to set the desired lower power level and auto suspend delay.
Some 1.1 compliant devices may support power states autonomously which can
be configured with the admin passthrough IOCTL, but other devices don't
have this feature, or may wish to have the driver control it instead.

One potential issue I already spotted, and the spec doesn't really make
this clear: I have runtime resume occur asynchronously when a new command
is allocated while suspended. The spec defines a "non-operational" power
state. Is the driver allowed to post a command to an IO queue while in
a non-op state and the command will be completed after the driver sets
the power state to operational, or does it need to be operational prior
to posting an IO command?
---
Usage Example:

 # Check the current runtime status

 root at keith:/sys/bus/pci/drivers/nvme/<D:B:D.f># cat nvme0/power/runtime_status
 active

 # Set the device to use power state "1" when idle

 root at keith:/sys/bus/pci/drivers/nvme/<D:B:D.f># echo 1 > nvme0/runtime_power_state

 # Set a 5 second autosuspend delay

 root at keith:/sys/bus/pci/drivers/nvme/<D:B:D.f># echo 5000 > nvme0/power/autosuspend_delay_ms

 # Check the current status after not using the device for 5 seconds

 root at keith:/sys/bus/pci/drivers/nvme/<D:B:D.f># cat nvme0/power/runtime_status
 suspended

 # Run an IO and verify device goes back to active

 root at keith:/sys/bus/pci/drivers/nvme/<D:B:D.f># dd if=/dev/nvme0n1 of=/dev/null bs=4k count=1 iflag=direct && \
 > cat nvme0/power/runtime_status
 1+0 records in
 1+0 records out
 4096 bytes (4.1 kB) copied, 0.000136335 s, 30.0 MB/s
 active



 drivers/block/nvme-core.c |   88 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/nvme.h      |    4 +++
 2 files changed, 92 insertions(+)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index cd8a8bc7..0163809 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -34,6 +34,7 @@
 #include <linux/moduleparam.h>
 #include <linux/pci.h>
 #include <linux/percpu.h>
+#include <linux/pm_runtime.h>
 #include <linux/poison.h>
 #include <linux/ptrace.h>
 #include <linux/sched.h>
@@ -180,6 +181,10 @@ static int alloc_cmdid(struct nvme_queue *nvmeq, void *ctx,
 	info[cmdid].ctx = ctx;
 	info[cmdid].timeout = jiffies + timeout;
 	info[cmdid].aborted = 0;
+	pm_runtime_mark_last_busy(&nvmeq->dev->dev);
+	atomic_inc(&nvmeq->dev->nr_pending);
+	if (pm_runtime_suspended(&nvmeq->dev->dev))
+		pm_request_resume(&nvmeq->dev->dev);
 	return cmdid;
 }
 
@@ -254,6 +259,8 @@ static void *free_cmdid(struct nvme_queue *nvmeq, int cmdid,
 	info[cmdid].ctx = CMD_CTX_COMPLETED;
 	clear_bit(cmdid, nvmeq->cmdid_data);
 	wake_up(&nvmeq->sq_full);
+	pm_runtime_mark_last_busy(&nvmeq->dev->dev);
+	atomic_dec(&nvmeq->dev->nr_pending);
 	return ctx;
 }
 
@@ -2208,6 +2215,7 @@ static int nvme_dev_add(struct nvme_dev *dev)
 	dev->oncs = le16_to_cpup(&ctrl->oncs);
 	dev->abort_limit = ctrl->acl + 1;
 	dev->vwc = ctrl->vwc;
+	dev->npss = ctrl->npss;
 	memcpy(dev->serial, ctrl->sn, sizeof(ctrl->sn));
 	memcpy(dev->model, ctrl->mn, sizeof(ctrl->mn));
 	memcpy(dev->firmware_rev, ctrl->fr, sizeof(ctrl->fr));
@@ -2715,6 +2723,67 @@ static void nvme_reset_workfn(struct work_struct *work)
 	dev->reset_workfn(work);
 }
 
+#ifdef CONFIG_PM_RUNTIME
+static int nvme_runtime_suspend(struct device *dev)
+{
+	struct nvme_dev *ndev = container_of(dev, struct nvme_dev, dev);
+	if (atomic_read(&ndev->nr_pending))
+		return -EBUSY;
+	if (ndev->low_power)
+		nvme_set_features(ndev, NVME_FEAT_POWER_MGMT, ndev->low_power,
+								0, NULL);
+	return 0;
+}
+
+static int nvme_runtime_resume(struct device *dev)
+{
+	struct nvme_dev *ndev = container_of(dev, struct nvme_dev, dev);
+	if (ndev->low_power)
+		nvme_set_features(ndev, NVME_FEAT_POWER_MGMT, 0, 0, NULL);
+	return 0;
+}
+
+static ssize_t set_runtime_power_state(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct nvme_dev *ndev = container_of(dev, struct nvme_dev, dev);
+	u8 low_power;
+
+	if (kstrtou8(buf, 10, &low_power))
+		return count;
+	if (low_power <= ndev->npss) {
+		if (ndev->low_power && !low_power) {
+			pm_runtime_dont_use_autosuspend(dev);
+			pm_runtime_disable(dev);
+		} else if (!ndev->low_power && low_power) {
+			pm_runtime_use_autosuspend(dev);
+			pm_runtime_enable(dev);
+		}
+		ndev->low_power = low_power;
+	} else
+		return -EINVAL;
+	return count;
+}
+
+static ssize_t show_runtime_power_state(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct nvme_dev *ndev = container_of(dev, struct nvme_dev, dev);
+	return scnprintf(buf, PAGE_SIZE, "%d\n", ndev->low_power);
+}
+static DEVICE_ATTR(runtime_power_state, S_IRUGO | S_IWUSR,
+		show_runtime_power_state, set_runtime_power_state);
+#endif
+
+const struct dev_pm_ops nvme_dev_rpm_ops = {
+	SET_PM_RUNTIME_PM_OPS(nvme_runtime_suspend, nvme_runtime_resume, NULL)
+};
+
+static struct device_driver nvme_dev_driver = {
+	.name = "nvme",
+	.pm = &nvme_dev_rpm_ops,
+};
+
 static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	int result = -ENOMEM;
@@ -2743,6 +2812,23 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	result = nvme_set_instance(dev);
 	if (result)
 		goto free;
+	
+	device_initialize(&dev->dev);
+	dev->dev.parent = &pdev->dev;
+	dev->dev.driver = &nvme_dev_driver;
+	dev_set_name(&dev->dev, "nvme%d", dev->instance);
+	if (device_add(&dev->dev))
+		goto free;
+	get_device(&dev->dev);
+
+	pm_runtime_set_autosuspend_delay(&dev->dev, -1);
+	pm_runtime_allow(&dev->dev);
+	pm_runtime_set_active(&dev->dev);
+
+	device_enable_async_suspend(&dev->dev);
+#ifdef CONFIG_PM_RUNTIME
+	device_create_file(&dev->dev, &dev_attr_runtime_power_state);
+#endif
 
 	result = nvme_setup_prp_pools(dev);
 	if (result)
@@ -2814,6 +2900,8 @@ static void nvme_remove(struct pci_dev *pdev)
 	rcu_barrier();
 	nvme_release_instance(dev);
 	nvme_release_prp_pools(dev);
+	put_device(&dev->dev);
+	device_del(&dev->dev);
 	kref_put(&dev->kref, nvme_free_dev);
 }
 
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index 1813cfd..a219016 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -69,6 +69,7 @@ extern unsigned char io_timeout;
  * Represents an NVM Express device.  Each nvme_dev is a PCI function.
  */
 struct nvme_dev {
+	struct device dev;
 	struct list_head node;
 	struct nvme_queue __rcu **queues;
 	unsigned short __percpu *io_queue;
@@ -95,12 +96,15 @@ struct nvme_dev {
 	char serial[20];
 	char model[40];
 	char firmware_rev[8];
+	atomic_t nr_pending;
 	u32 max_hw_sectors;
 	u32 stripe_size;
 	u16 oncs;
 	u16 abort_limit;
 	u8 vwc;
 	u8 initialized;
+	u8 low_power;
+	u8 npss;
 };
 
 /*
-- 
1.7.10.4