linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] nvme: allow user toggling hmb usage
@ 2021-07-27 16:40 Keith Busch
  2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Keith Busch @ 2021-07-27 16:40 UTC (permalink / raw)
  To: linux-nvme, hch; +Cc: sagi, kai.heng.feng, Keith Busch

The NVMe host memory buffer may consume a non-negligable amount of
memory. Controllers are required to function without the host memory
buffer enabled, but with possibly degraded performance. Export a sysfs
property to toggle this feature on a per-device granularity so users may
choose to reclaim memory at the expense of storage performance.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 45 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index ac72386b1f2c..e8f3b32131d2 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -137,6 +137,7 @@ struct nvme_dev {
 	u32 cmbloc;
 	struct nvme_ctrl ctrl;
 	u32 last_ps;
+	bool hmb;
 
 	mempool_t *iod_mempool;
 
@@ -1896,7 +1897,9 @@ static int nvme_set_host_mem(struct nvme_dev *dev, u32 bits)
 		dev_warn(dev->ctrl.device,
 			 "failed to set host mem (err %d, flags %#x).\n",
 			 ret, bits);
-	}
+	} else
+		dev->hmb = bits & NVME_HOST_MEM_ENABLE;
+
 	return ret;
 }
 
@@ -2081,6 +2084,42 @@ static ssize_t cmbsz_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(cmbsz);
 
+static ssize_t hmb_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
+
+	return sysfs_emit(buf, "%d\n", ndev->hmb);
+}
+
+static ssize_t hmb_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
+	bool new;
+	int ret;
+
+	if (strtobool(buf, &new) < 0)
+		return -EINVAL;
+
+	if (new == ndev->hmb)
+		return count;
+
+	if (new) {
+		ret = nvme_setup_host_mem(ndev);
+	} else {
+		ret = nvme_set_host_mem(ndev, 0);
+		if (!ret)
+			nvme_free_host_mem(ndev);
+	}
+
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+static DEVICE_ATTR_RW(hmb);
+
 static umode_t nvme_pci_attrs_are_visible(struct kobject *kobj,
 		struct attribute *a, int n)
 {
@@ -2094,6 +2133,9 @@ static umode_t nvme_pci_attrs_are_visible(struct kobject *kobj,
 	    	if (!dev->cmbsz)
 			return 0;
 	}
+	if (a == &dev_attr_hmb.attr && !ctrl->hmpre)
+		return 0;
+
 	return a->mode;
 }
 
@@ -2101,6 +2143,7 @@ static struct attribute *nvme_pci_attrs[] = {
 	&dev_attr_cmb.attr,
 	&dev_attr_cmbloc.attr,
 	&dev_attr_cmbsz.attr,
+	&dev_attr_hmb.attr,
 	NULL,
 };
 
-- 
2.25.4


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

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

* [PATCH 2/2] nvme-pci: disable hmb on idle suspend
  2021-07-27 16:40 [PATCH 1/2] nvme: allow user toggling hmb usage Keith Busch
@ 2021-07-27 16:40 ` Keith Busch
  2021-08-05 14:12   ` Kai-Heng Feng
  2021-08-06 19:37   ` Sagi Grimberg
  2021-08-06 19:37 ` [PATCH 1/2] nvme: allow user toggling hmb usage Sagi Grimberg
  2021-08-10 15:58 ` Christoph Hellwig
  2 siblings, 2 replies; 6+ messages in thread
From: Keith Busch @ 2021-07-27 16:40 UTC (permalink / raw)
  To: linux-nvme, hch; +Cc: sagi, kai.heng.feng, Keith Busch

An idle suspend may or may not disable host memory access from devices
placed in low power mode. Either way, it should always be safe to
disable the host memory buffer prior to entering the low power mode, and
this should also always be faster than a full device shutdown.

Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/pci.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e8f3b32131d2..42d4cbe31a28 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3130,8 +3130,13 @@ static int nvme_resume(struct device *dev)
 
 	if (ndev->last_ps == U32_MAX ||
 	    nvme_set_power_state(ctrl, ndev->last_ps) != 0)
-		return nvme_try_sched_reset(&ndev->ctrl);
+		goto reset;
+	if (ctrl->hmpre && nvme_setup_host_mem(ndev))
+		goto reset;
+
 	return 0;
+reset:
+	return nvme_try_sched_reset(ctrl);
 }
 
 static int nvme_suspend(struct device *dev)
@@ -3155,15 +3160,9 @@ static int nvme_suspend(struct device *dev)
 	 * the PCI bus layer to put it into D3 in order to take the PCIe link
 	 * down, so as to allow the platform to achieve its minimum low-power
 	 * state (which may not be possible if the link is up).
-	 *
-	 * If a host memory buffer is enabled, shut down the device as the NVMe
-	 * specification allows the device to access the host memory buffer in
-	 * host DRAM from all power states, but hosts will fail access to DRAM
-	 * during S3.
 	 */
 	if (pm_suspend_via_firmware() || !ctrl->npss ||
 	    !pcie_aspm_enabled(pdev) ||
-	    ndev->nr_host_mem_descs ||
 	    (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND))
 		return nvme_disable_prepare_reset(ndev, true);
 
@@ -3174,6 +3173,17 @@ static int nvme_suspend(struct device *dev)
 	if (ctrl->state != NVME_CTRL_LIVE)
 		goto unfreeze;
 
+	/*
+	 * Host memory access may not be successful in a system suspend state,
+	 * but the specification allows the controller to access memory in a
+	 * non-operational power state.
+	 */
+	if (ndev->hmb) {
+		ret = nvme_set_host_mem(ndev, 0);
+		if (ret < 0)
+			goto unfreeze;
+	}
+
 	ret = nvme_get_power_state(ctrl, &ndev->last_ps);
 	if (ret < 0)
 		goto unfreeze;
-- 
2.25.4


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

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

* Re: [PATCH 2/2] nvme-pci: disable hmb on idle suspend
  2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
@ 2021-08-05 14:12   ` Kai-Heng Feng
  2021-08-06 19:37   ` Sagi Grimberg
  1 sibling, 0 replies; 6+ messages in thread
From: Kai-Heng Feng @ 2021-08-05 14:12 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-nvme, Christoph Hellwig, Sagi Grimberg

On Wed, Jul 28, 2021 at 12:40 AM Keith Busch <kbusch@kernel.org> wrote:
>
> An idle suspend may or may not disable host memory access from devices
> placed in low power mode. Either way, it should always be safe to
> disable the host memory buffer prior to entering the low power mode, and
> this should also always be faster than a full device shutdown.
>
> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>

Positive test result from user:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912057/comments/74

Kai-Heng


> ---
>  drivers/nvme/host/pci.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e8f3b32131d2..42d4cbe31a28 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3130,8 +3130,13 @@ static int nvme_resume(struct device *dev)
>
>         if (ndev->last_ps == U32_MAX ||
>             nvme_set_power_state(ctrl, ndev->last_ps) != 0)
> -               return nvme_try_sched_reset(&ndev->ctrl);
> +               goto reset;
> +       if (ctrl->hmpre && nvme_setup_host_mem(ndev))
> +               goto reset;
> +
>         return 0;
> +reset:
> +       return nvme_try_sched_reset(ctrl);
>  }
>
>  static int nvme_suspend(struct device *dev)
> @@ -3155,15 +3160,9 @@ static int nvme_suspend(struct device *dev)
>          * the PCI bus layer to put it into D3 in order to take the PCIe link
>          * down, so as to allow the platform to achieve its minimum low-power
>          * state (which may not be possible if the link is up).
> -        *
> -        * If a host memory buffer is enabled, shut down the device as the NVMe
> -        * specification allows the device to access the host memory buffer in
> -        * host DRAM from all power states, but hosts will fail access to DRAM
> -        * during S3.
>          */
>         if (pm_suspend_via_firmware() || !ctrl->npss ||
>             !pcie_aspm_enabled(pdev) ||
> -           ndev->nr_host_mem_descs ||
>             (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND))
>                 return nvme_disable_prepare_reset(ndev, true);
>
> @@ -3174,6 +3173,17 @@ static int nvme_suspend(struct device *dev)
>         if (ctrl->state != NVME_CTRL_LIVE)
>                 goto unfreeze;
>
> +       /*
> +        * Host memory access may not be successful in a system suspend state,
> +        * but the specification allows the controller to access memory in a
> +        * non-operational power state.
> +        */
> +       if (ndev->hmb) {
> +               ret = nvme_set_host_mem(ndev, 0);
> +               if (ret < 0)
> +                       goto unfreeze;
> +       }
> +
>         ret = nvme_get_power_state(ctrl, &ndev->last_ps);
>         if (ret < 0)
>                 goto unfreeze;
> --
> 2.25.4
>

_______________________________________________
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 1/2] nvme: allow user toggling hmb usage
  2021-07-27 16:40 [PATCH 1/2] nvme: allow user toggling hmb usage Keith Busch
  2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
@ 2021-08-06 19:37 ` Sagi Grimberg
  2021-08-10 15:58 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2021-08-06 19:37 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: kai.heng.feng

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

* Re: [PATCH 2/2] nvme-pci: disable hmb on idle suspend
  2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
  2021-08-05 14:12   ` Kai-Heng Feng
@ 2021-08-06 19:37   ` Sagi Grimberg
  1 sibling, 0 replies; 6+ messages in thread
From: Sagi Grimberg @ 2021-08-06 19:37 UTC (permalink / raw)
  To: Keith Busch, linux-nvme, hch; +Cc: kai.heng.feng

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

* Re: [PATCH 1/2] nvme: allow user toggling hmb usage
  2021-07-27 16:40 [PATCH 1/2] nvme: allow user toggling hmb usage Keith Busch
  2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
  2021-08-06 19:37 ` [PATCH 1/2] nvme: allow user toggling hmb usage Sagi Grimberg
@ 2021-08-10 15:58 ` Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2021-08-10 15:58 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-nvme, hch, sagi, kai.heng.feng

Thanks, I've applied both patches to nvme-5.15.

_______________________________________________
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, other threads:[~2021-08-10 15:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 16:40 [PATCH 1/2] nvme: allow user toggling hmb usage Keith Busch
2021-07-27 16:40 ` [PATCH 2/2] nvme-pci: disable hmb on idle suspend Keith Busch
2021-08-05 14:12   ` Kai-Heng Feng
2021-08-06 19:37   ` Sagi Grimberg
2021-08-06 19:37 ` [PATCH 1/2] nvme: allow user toggling hmb usage Sagi Grimberg
2021-08-10 15:58 ` 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).