linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v1 0/2] "Hotremove" persistent memory
@ 2019-04-20 15:31 Pavel Tatashin
  2019-04-20 15:31 ` [v1 1/2] device-dax: fix memory and resource leak if hotplug fails Pavel Tatashin
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 15:31 UTC (permalink / raw)
  To: pasha.tatashin, jmorris, sashal, linux-kernel, linux-mm,
	linux-nvdimm, akpm, mhocko, dave.hansen, dan.j.williams,
	keith.busch, vishal.l.verma, dave.jiang, zwisler,
	thomas.lendacky, ying.huang, fengguang.wu, bp, bhelgaas,
	baiyaowei, tiwai, jglisse

Recently, adding a persistent memory to be used like a regular RAM was
added to Linux. This work extends this functionality to also allow hot
removing persistent memory.

We (Microsoft) have a very important use case for this functionality.

The requirement is for physical machines with small amount of RAM (~8G)
to be able to reboot in a very short period of time (<1s). Yet, there is
a userland state that is expensive to recreate (~2G).

The solution is to boot machines with 2G preserved for persistent
memory.

Copy the state, and hotadd the persistent memory so machine still has all
8G for runtime. Before reboot, hotremove device-dax 2G, copy the memory
that is needed to be preserved to pmem0 device, and reboot.

The series of operations look like this:

	1. After boot restore /dev/pmem0 to boot
	2. Convert raw pmem0 to devdax
	ndctl create-namespace --mode devdax --map mem -e namespace0.0 -f
	3. Hotadd to System RAM 
	echo dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
	echo dax0.0 > /sys/bus/dax/drivers/kmem/new_id
	4. Before reboot hotremove device-dax memory from System RAM
	echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
	5. Create raw pmem0 device
	ndctl create-namespace --mode raw  -e namespace0.0 -f
	6. Copy the state to this device
	7. Do kexec reboot, or reboot through firmware, is firmware does not
	zero memory in pmem region.

Pavel Tatashin (2):
  device-dax: fix memory and resource leak if hotplug fails
  device-dax: "Hotremove" persistent memory that is used like normal RAM

 drivers/dax/dax-private.h |  2 +
 drivers/dax/kmem.c        | 82 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 79 insertions(+), 5 deletions(-)

-- 
2.21.0


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

* [v1 1/2] device-dax: fix memory and resource leak if hotplug fails
  2019-04-20 15:31 [v1 0/2] "Hotremove" persistent memory Pavel Tatashin
@ 2019-04-20 15:31 ` Pavel Tatashin
  2019-04-20 15:31 ` [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM Pavel Tatashin
  2019-04-20 16:34 ` [v1 0/2] "Hotremove" persistent memory Dan Williams
  2 siblings, 0 replies; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 15:31 UTC (permalink / raw)
  To: pasha.tatashin, jmorris, sashal, linux-kernel, linux-mm,
	linux-nvdimm, akpm, mhocko, dave.hansen, dan.j.williams,
	keith.busch, vishal.l.verma, dave.jiang, zwisler,
	thomas.lendacky, ying.huang, fengguang.wu, bp, bhelgaas,
	baiyaowei, tiwai, jglisse

When add_memory() function fails, the resource and the memory should be
freed.

Fixes: c221c0b0308f ("device-dax: "Hotplug" persistent memory for use like normal RAM")

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/dax/kmem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index a02318c6d28a..4c0131857133 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -66,8 +66,11 @@ int dev_dax_kmem_probe(struct device *dev)
 	new_res->name = dev_name(dev);
 
 	rc = add_memory(numa_node, new_res->start, resource_size(new_res));
-	if (rc)
+	if (rc) {
+		release_resource(new_res);
+		kfree(new_res);
 		return rc;
+	}
 
 	return 0;
 }
-- 
2.21.0


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

* [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 15:31 [v1 0/2] "Hotremove" persistent memory Pavel Tatashin
  2019-04-20 15:31 ` [v1 1/2] device-dax: fix memory and resource leak if hotplug fails Pavel Tatashin
@ 2019-04-20 15:31 ` Pavel Tatashin
  2019-04-20 16:18   ` Dan Williams
  2019-04-20 16:34 ` [v1 0/2] "Hotremove" persistent memory Dan Williams
  2 siblings, 1 reply; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 15:31 UTC (permalink / raw)
  To: pasha.tatashin, jmorris, sashal, linux-kernel, linux-mm,
	linux-nvdimm, akpm, mhocko, dave.hansen, dan.j.williams,
	keith.busch, vishal.l.verma, dave.jiang, zwisler,
	thomas.lendacky, ying.huang, fengguang.wu, bp, bhelgaas,
	baiyaowei, tiwai, jglisse

It is now allowed to use persistent memory like a regular RAM, but
currently there is no way to remove this memory until machine is
rebooted.

This work expands the functionality to also allow hot removing
previously hotplugged persistent memory, and recover the device for use
for other purposes.

To hotremove persistent memory, the management software must unbind it
from device-dax/kmem driver:

            echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind

Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
---
 drivers/dax/dax-private.h |  2 +
 drivers/dax/kmem.c        | 77 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
index a45612148ca0..999aaf3a29b3 100644
--- a/drivers/dax/dax-private.h
+++ b/drivers/dax/dax-private.h
@@ -53,6 +53,7 @@ struct dax_region {
  * @pgmap - pgmap for memmap setup / lifetime (driver owned)
  * @ref: pgmap reference count (driver owned)
  * @cmp: @ref final put completion (driver owned)
+ * @dax_mem_res: physical address range of hotadded DAX memory
  */
 struct dev_dax {
 	struct dax_region *region;
@@ -62,6 +63,7 @@ struct dev_dax {
 	struct dev_pagemap pgmap;
 	struct percpu_ref ref;
 	struct completion cmp;
+	struct resource *dax_kmem_res;
 };
 
 static inline struct dev_dax *to_dev_dax(struct device *dev)
diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 4c0131857133..026c34f93df5 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -71,21 +71,90 @@ int dev_dax_kmem_probe(struct device *dev)
 		kfree(new_res);
 		return rc;
 	}
+	dev_dax->dax_kmem_res = new_res;
 
 	return 0;
 }
 
+#ifdef CONFIG_MEMORY_HOTREMOVE
+/*
+ * Offline device-dax's memory_blocks. If a memory_block cannot be offlined
+ * a warning is printed and an error is returned. dax hotremove can succeed
+ * only when every memory_block is offline.
+ */
+static int
+offline_memblock_cb(struct memory_block *mem, void *arg)
+{
+	struct device *dev = (struct device *)arg;
+	int rc = device_offline(&mem->dev);
+
+	if (rc < 0) {
+		unsigned long spfn = section_nr_to_pfn(mem->start_section_nr);
+		unsigned long epfn = section_nr_to_pfn(mem->end_section_nr);
+		phys_addr_t spa = spfn << PAGE_SHIFT;
+		phys_addr_t epa = epfn << PAGE_SHIFT;
+
+		dev_warn(dev, "could not offline memory block [%pa-%pa]\n",
+			 &spa, &epa);
+
+		return rc;
+	}
+
+	return 0;
+}
+
+static int dev_dax_kmem_remove(struct device *dev)
+{
+	struct dev_dax *dev_dax = to_dev_dax(dev);
+	struct resource *res = dev_dax->dax_kmem_res;
+	resource_size_t kmem_start;
+	resource_size_t kmem_size;
+	unsigned long start_pfn;
+	unsigned long end_pfn;
+	int rc;
+
+	/*
+	 * dax kmem resource does not exist, means memory was never hotplugged.
+	 * So, nothing to do here.
+	 */
+	if (!res)
+		return 0;
+
+	kmem_start = res->start;
+	kmem_size = resource_size(res);
+	start_pfn = kmem_start >> PAGE_SHIFT;
+	end_pfn = start_pfn + (kmem_size >> PAGE_SHIFT) - 1;
+
+	/* Walk and offline every singe memory_block of the dax region. */
+	lock_device_hotplug();
+	rc = walk_memory_range(start_pfn, end_pfn, dev, offline_memblock_cb);
+	unlock_device_hotplug();
+	if (rc)
+		return rc;
+
+	/* Hotremove memory, cannot fail because memory is already offlined */
+	remove_memory(dev_dax->target_node, kmem_start, kmem_size);
+
+	/* Release and free dax resources */
+	release_resource(res);
+	kfree(res);
+	dev_dax->dax_kmem_res = NULL;
+
+	return 0;
+}
+#else
 static int dev_dax_kmem_remove(struct device *dev)
 {
 	/*
-	 * Purposely leak the request_mem_region() for the device-dax
-	 * range and return '0' to ->remove() attempts. The removal of
-	 * the device from the driver always succeeds, but the region
-	 * is permanently pinned as reserved by the unreleased
+	 * Without hotremove purposely leak the request_mem_region() for the
+	 * device-dax range and return '0' to ->remove() attempts. The removal
+	 * of the device from the driver always succeeds, but the region is
+	 * permanently pinned as reserved by the unreleased
 	 * request_mem_region().
 	 */
 	return 0;
 }
+#endif /* CONFIG_MEMORY_HOTREMOVE */
 
 static struct dax_device_driver device_dax_kmem_driver = {
 	.drv = {
-- 
2.21.0


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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 15:31 ` [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM Pavel Tatashin
@ 2019-04-20 16:18   ` Dan Williams
  2019-04-20 16:30     ` Pavel Tatashin
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2019-04-20 16:18 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 8:36 AM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> It is now allowed to use persistent memory like a regular RAM, but
> currently there is no way to remove this memory until machine is
> rebooted.
>
> This work expands the functionality to also allow hot removing
> previously hotplugged persistent memory, and recover the device for use
> for other purposes.
>
> To hotremove persistent memory, the management software must unbind it
> from device-dax/kmem driver:
>
>             echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  drivers/dax/dax-private.h |  2 +
>  drivers/dax/kmem.c        | 77 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 75 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
> index a45612148ca0..999aaf3a29b3 100644
> --- a/drivers/dax/dax-private.h
> +++ b/drivers/dax/dax-private.h
> @@ -53,6 +53,7 @@ struct dax_region {
>   * @pgmap - pgmap for memmap setup / lifetime (driver owned)
>   * @ref: pgmap reference count (driver owned)
>   * @cmp: @ref final put completion (driver owned)
> + * @dax_mem_res: physical address range of hotadded DAX memory
>   */
>  struct dev_dax {
>         struct dax_region *region;
> @@ -62,6 +63,7 @@ struct dev_dax {
>         struct dev_pagemap pgmap;
>         struct percpu_ref ref;
>         struct completion cmp;
> +       struct resource *dax_kmem_res;
>  };
>
>  static inline struct dev_dax *to_dev_dax(struct device *dev)
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 4c0131857133..026c34f93df5 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -71,21 +71,90 @@ int dev_dax_kmem_probe(struct device *dev)
>                 kfree(new_res);
>                 return rc;
>         }
> +       dev_dax->dax_kmem_res = new_res;
>
>         return 0;
>  }
>
> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +/*
> + * Offline device-dax's memory_blocks. If a memory_block cannot be offlined
> + * a warning is printed and an error is returned. dax hotremove can succeed
> + * only when every memory_block is offline.
> + */
> +static int
> +offline_memblock_cb(struct memory_block *mem, void *arg)
> +{
> +       struct device *dev = (struct device *)arg;
> +       int rc = device_offline(&mem->dev);
> +
> +       if (rc < 0) {
> +               unsigned long spfn = section_nr_to_pfn(mem->start_section_nr);
> +               unsigned long epfn = section_nr_to_pfn(mem->end_section_nr);
> +               phys_addr_t spa = spfn << PAGE_SHIFT;
> +               phys_addr_t epa = epfn << PAGE_SHIFT;
> +
> +               dev_warn(dev, "could not offline memory block [%pa-%pa]\n",
> +                        &spa, &epa);
> +
> +               return rc;
> +       }
> +
> +       return 0;
> +}
> +
> +static int dev_dax_kmem_remove(struct device *dev)
> +{
> +       struct dev_dax *dev_dax = to_dev_dax(dev);
> +       struct resource *res = dev_dax->dax_kmem_res;
> +       resource_size_t kmem_start;
> +       resource_size_t kmem_size;
> +       unsigned long start_pfn;
> +       unsigned long end_pfn;
> +       int rc;
> +
> +       /*
> +        * dax kmem resource does not exist, means memory was never hotplugged.
> +        * So, nothing to do here.
> +        */
> +       if (!res)
> +               return 0;
> +
> +       kmem_start = res->start;
> +       kmem_size = resource_size(res);
> +       start_pfn = kmem_start >> PAGE_SHIFT;
> +       end_pfn = start_pfn + (kmem_size >> PAGE_SHIFT) - 1;
> +
> +       /* Walk and offline every singe memory_block of the dax region. */
> +       lock_device_hotplug();
> +       rc = walk_memory_range(start_pfn, end_pfn, dev, offline_memblock_cb);
> +       unlock_device_hotplug();
> +       if (rc)
> +               return rc;

This potential early return is the reason why memory hotremove is not
reliable vs the driver-core. If this walk fails to offline the memory
it will still be online, but the driver-core has no consideration for
device-unbind failing. The ubind will proceed while the memory stays
pinned.

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 16:18   ` Dan Williams
@ 2019-04-20 16:30     ` Pavel Tatashin
  2019-04-20 16:36       ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 16:30 UTC (permalink / raw)
  To: Dan Williams
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

> > +
> > +       /* Walk and offline every singe memory_block of the dax region. */
> > +       lock_device_hotplug();
> > +       rc = walk_memory_range(start_pfn, end_pfn, dev, offline_memblock_cb);
> > +       unlock_device_hotplug();
> > +       if (rc)
> > +               return rc;
>
> This potential early return is the reason why memory hotremove is not
> reliable vs the driver-core. If this walk fails to offline the memory
> it will still be online, but the driver-core has no consideration for
> device-unbind failing. The ubind will proceed while the memory stays
> pinned.

Hi Dan,

Thank you for looking at this.  Are you saying, that if drv.remove()
returns a failure it is simply ignored, and unbind proceeds?

Pasha

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

* Re: [v1 0/2] "Hotremove" persistent memory
  2019-04-20 15:31 [v1 0/2] "Hotremove" persistent memory Pavel Tatashin
  2019-04-20 15:31 ` [v1 1/2] device-dax: fix memory and resource leak if hotplug fails Pavel Tatashin
  2019-04-20 15:31 ` [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM Pavel Tatashin
@ 2019-04-20 16:34 ` Dan Williams
  2019-04-20 16:56   ` Pavel Tatashin
  2 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2019-04-20 16:34 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 8:32 AM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Recently, adding a persistent memory to be used like a regular RAM was
> added to Linux. This work extends this functionality to also allow hot
> removing persistent memory.
>
> We (Microsoft) have a very important use case for this functionality.
>
> The requirement is for physical machines with small amount of RAM (~8G)
> to be able to reboot in a very short period of time (<1s). Yet, there is
> a userland state that is expensive to recreate (~2G).
>
> The solution is to boot machines with 2G preserved for persistent
> memory.

Makes sense, but I have some questions about the details.

>
> Copy the state, and hotadd the persistent memory so machine still has all
> 8G for runtime. Before reboot, hotremove device-dax 2G, copy the memory
> that is needed to be preserved to pmem0 device, and reboot.
>
> The series of operations look like this:
>
>         1. After boot restore /dev/pmem0 to boot
>         2. Convert raw pmem0 to devdax
>         ndctl create-namespace --mode devdax --map mem -e namespace0.0 -f
>         3. Hotadd to System RAM
>         echo dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
>         echo dax0.0 > /sys/bus/dax/drivers/kmem/new_id
>         4. Before reboot hotremove device-dax memory from System RAM
>         echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
>         5. Create raw pmem0 device
>         ndctl create-namespace --mode raw  -e namespace0.0 -f
>         6. Copy the state to this device

What is the source of this copy? The state that was in the hot-added
memory? Isn't it "already there" since you effectively renamed dax0.0
to pmem0?

>         7. Do kexec reboot, or reboot through firmware, is firmware does not
>         zero memory in pmem region.

Wouldn't the dax0.0 contents be preserved regardless? How does the
guest recover the pre-initialized state / how does the kernel know to
give out the same pages to the application as the previous boot?

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 16:30     ` Pavel Tatashin
@ 2019-04-20 16:36       ` Dan Williams
  2019-04-20 17:01         ` Pavel Tatashin
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2019-04-20 16:36 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 9:30 AM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> > > +
> > > +       /* Walk and offline every singe memory_block of the dax region. */
> > > +       lock_device_hotplug();
> > > +       rc = walk_memory_range(start_pfn, end_pfn, dev, offline_memblock_cb);
> > > +       unlock_device_hotplug();
> > > +       if (rc)
> > > +               return rc;
> >
> > This potential early return is the reason why memory hotremove is not
> > reliable vs the driver-core. If this walk fails to offline the memory
> > it will still be online, but the driver-core has no consideration for
> > device-unbind failing. The ubind will proceed while the memory stays
> > pinned.
>
> Hi Dan,
>
> Thank you for looking at this.  Are you saying, that if drv.remove()
> returns a failure it is simply ignored, and unbind proceeds?

Yeah, that's the problem. I've looked at making unbind able to fail,
but that can lead to general bad behavior in device-drivers. I.e. why
spend time unwinding allocated resources when the driver can simply
fail unbind? About the best a driver can do is make unbind wait on
some event, but any return results in device-unbind.

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

* Re: [v1 0/2] "Hotremove" persistent memory
  2019-04-20 16:34 ` [v1 0/2] "Hotremove" persistent memory Dan Williams
@ 2019-04-20 16:56   ` Pavel Tatashin
  0 siblings, 0 replies; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 16:56 UTC (permalink / raw)
  To: Dan Williams
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

> Makes sense, but I have some questions about the details.
>
> >
> > Copy the state, and hotadd the persistent memory so machine still has all
> > 8G for runtime. Before reboot, hotremove device-dax 2G, copy the memory
> > that is needed to be preserved to pmem0 device, and reboot.
> >
> > The series of operations look like this:
> >
> >         1. After boot restore /dev/pmem0 to boot

s/boot/to a ramdisk from which is is picked by apps/

> >         2. Convert raw pmem0 to devdax
> >         ndctl create-namespace --mode devdax --map mem -e namespace0.0 -f
> >         3. Hotadd to System RAM
> >         echo dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
> >         echo dax0.0 > /sys/bus/dax/drivers/kmem/new_id
> >         4. Before reboot hotremove device-dax memory from System RAM
> >         echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
> >         5. Create raw pmem0 device
> >         ndctl create-namespace --mode raw  -e namespace0.0 -f
> >         6. Copy the state to this device
>
> What is the source of this copy? The state that was in the hot-added
> memory? Isn't it "already there" since you effectively renamed dax0.0
> to pmem0?

Before hotremove, applications create a file in a ramdisk that is 2G
in size. After that applications, exist. We copy this file from
ramdisk to /dev/pmem0  (RAM to RAM copy) to be able to quickly restore
after reboot. After reboot, applications take that file from ramdisk,
and ramdisk is freed.

>
> >         7. Do kexec reboot, or reboot through firmware, is firmware does not
> >         zero memory in pmem region.
s/is/if/
>
> Wouldn't the dax0.0 contents be preserved regardless? How does the
> guest recover the pre-initialized state / how does the kernel know to
> give out the same pages to the application as the previous boot?

On these machines we do not have real persistent memory, only regular
volatile RAM. So, kernel has to either be booted via memap arguments
that specify persistent range, or via special pmem device node in DTB.

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 16:36       ` Dan Williams
@ 2019-04-20 17:01         ` Pavel Tatashin
  2019-04-20 21:02           ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 17:01 UTC (permalink / raw)
  To: Dan Williams
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

> > Thank you for looking at this.  Are you saying, that if drv.remove()
> > returns a failure it is simply ignored, and unbind proceeds?
>
> Yeah, that's the problem. I've looked at making unbind able to fail,
> but that can lead to general bad behavior in device-drivers. I.e. why
> spend time unwinding allocated resources when the driver can simply
> fail unbind? About the best a driver can do is make unbind wait on
> some event, but any return results in device-unbind.

Hm, just tested, and it is indeed so.

I see the following options:

1. Move hot remove code to some other interface, that can fail. Not
sure what that would be, but outside of unbind/remove_id. Any
suggestion?
2. Option two is don't attept to offline memory in unbind. Do
hot-remove memory in unbind if every section is already offlined.
Basically, do a walk through memblocks, and if every section is
offlined, also do the cleanup.

Pasha

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 17:01         ` Pavel Tatashin
@ 2019-04-20 21:02           ` Dan Williams
  2019-04-20 22:04             ` Pavel Tatashin
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Williams @ 2019-04-20 21:02 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 10:02 AM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> > > Thank you for looking at this.  Are you saying, that if drv.remove()
> > > returns a failure it is simply ignored, and unbind proceeds?
> >
> > Yeah, that's the problem. I've looked at making unbind able to fail,
> > but that can lead to general bad behavior in device-drivers. I.e. why
> > spend time unwinding allocated resources when the driver can simply
> > fail unbind? About the best a driver can do is make unbind wait on
> > some event, but any return results in device-unbind.
>
> Hm, just tested, and it is indeed so.
>
> I see the following options:
>
> 1. Move hot remove code to some other interface, that can fail. Not
> sure what that would be, but outside of unbind/remove_id. Any
> suggestion?
> 2. Option two is don't attept to offline memory in unbind. Do
> hot-remove memory in unbind if every section is already offlined.
> Basically, do a walk through memblocks, and if every section is
> offlined, also do the cleanup.

I think something like option-2 could work just as long as the user is
ok with failure and prepared to handle it. It's already the case that
the request_region() in kmem permanently prevents the memory range
from being reused by any other driver. So if the hot-unplug fails it
could skip the corresponding release_region() and effectively it's the
same as what we have now in terms of reuse protection. In your flow if
the memory remove failed then the conversion attempt from devdax to
raw mode would also fail and presumably you could fall back to doing a
full reboot / rebuild of the application state?

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 21:02           ` Dan Williams
@ 2019-04-20 22:04             ` Pavel Tatashin
  2019-04-20 23:19               ` Dan Williams
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Tatashin @ 2019-04-20 22:04 UTC (permalink / raw)
  To: Dan Williams
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 5:02 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Sat, Apr 20, 2019 at 10:02 AM Pavel Tatashin
> <pasha.tatashin@soleen.com> wrote:
> >
> > > > Thank you for looking at this.  Are you saying, that if drv.remove()
> > > > returns a failure it is simply ignored, and unbind proceeds?
> > >
> > > Yeah, that's the problem. I've looked at making unbind able to fail,
> > > but that can lead to general bad behavior in device-drivers. I.e. why
> > > spend time unwinding allocated resources when the driver can simply
> > > fail unbind? About the best a driver can do is make unbind wait on
> > > some event, but any return results in device-unbind.
> >
> > Hm, just tested, and it is indeed so.
> >
> > I see the following options:
> >
> > 1. Move hot remove code to some other interface, that can fail. Not
> > sure what that would be, but outside of unbind/remove_id. Any
> > suggestion?
> > 2. Option two is don't attept to offline memory in unbind. Do
> > hot-remove memory in unbind if every section is already offlined.
> > Basically, do a walk through memblocks, and if every section is
> > offlined, also do the cleanup.
>
> I think something like option-2 could work just as long as the user is
> ok with failure and prepared to handle it. It's already the case that
> the request_region() in kmem permanently prevents the memory range
> from being reused by any other driver. So if the hot-unplug fails it
> could skip the corresponding release_region() and effectively it's the
> same as what we have now in terms of reuse protection. In your flow if
> the memory remove failed then the conversion attempt from devdax to
> raw mode would also fail and presumably you could fall back to doing a
> full reboot / rebuild of the application state?

With option two, where we will simply check that every memory_block is
offlined, we will have deterministic behavior:

1. If user did not offline every dax memory section beforehand via
echo offline > /sys/devices/system/memory/memoryN/state

echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
Will be the same as now, will simply return, and user won't be able to
use dax afterwords or hotremove it.

2. If user did offline ever dax memory section beforehand
echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
Will be guaranteed to succeed to hotremove the memory, as there is
nothing that can fail.

So, if user wants to hotremove dax memory, he/she must ensure that
every section is offlined before unbinding.

Pasha

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

* Re: [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM
  2019-04-20 22:04             ` Pavel Tatashin
@ 2019-04-20 23:19               ` Dan Williams
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Williams @ 2019-04-20 23:19 UTC (permalink / raw)
  To: Pavel Tatashin
  Cc: James Morris, Sasha Levin, Linux Kernel Mailing List, Linux MM,
	linux-nvdimm, Andrew Morton, Michal Hocko, Dave Hansen,
	Keith Busch, Vishal L Verma, Dave Jiang, Ross Zwisler,
	Tom Lendacky, Huang, Ying, Fengguang Wu, Borislav Petkov,
	Bjorn Helgaas, Yaowei Bai, Takashi Iwai, Jérôme Glisse

On Sat, Apr 20, 2019 at 3:04 PM Pavel Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> On Sat, Apr 20, 2019 at 5:02 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > On Sat, Apr 20, 2019 at 10:02 AM Pavel Tatashin
> > <pasha.tatashin@soleen.com> wrote:
> > >
> > > > > Thank you for looking at this.  Are you saying, that if drv.remove()
> > > > > returns a failure it is simply ignored, and unbind proceeds?
> > > >
> > > > Yeah, that's the problem. I've looked at making unbind able to fail,
> > > > but that can lead to general bad behavior in device-drivers. I.e. why
> > > > spend time unwinding allocated resources when the driver can simply
> > > > fail unbind? About the best a driver can do is make unbind wait on
> > > > some event, but any return results in device-unbind.
> > >
> > > Hm, just tested, and it is indeed so.
> > >
> > > I see the following options:
> > >
> > > 1. Move hot remove code to some other interface, that can fail. Not
> > > sure what that would be, but outside of unbind/remove_id. Any
> > > suggestion?
> > > 2. Option two is don't attept to offline memory in unbind. Do
> > > hot-remove memory in unbind if every section is already offlined.
> > > Basically, do a walk through memblocks, and if every section is
> > > offlined, also do the cleanup.
> >
> > I think something like option-2 could work just as long as the user is
> > ok with failure and prepared to handle it. It's already the case that
> > the request_region() in kmem permanently prevents the memory range
> > from being reused by any other driver. So if the hot-unplug fails it
> > could skip the corresponding release_region() and effectively it's the
> > same as what we have now in terms of reuse protection. In your flow if
> > the memory remove failed then the conversion attempt from devdax to
> > raw mode would also fail and presumably you could fall back to doing a
> > full reboot / rebuild of the application state?
>
> With option two, where we will simply check that every memory_block is
> offlined, we will have deterministic behavior:
>
> 1. If user did not offline every dax memory section beforehand via
> echo offline > /sys/devices/system/memory/memoryN/state
>
> echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
> Will be the same as now, will simply return, and user won't be able to
> use dax afterwords or hotremove it.
>
> 2. If user did offline ever dax memory section beforehand
> echo dax0.0 > /sys/bus/dax/drivers/kmem/unbind
> Will be guaranteed to succeed to hotremove the memory, as there is
> nothing that can fail.
>
> So, if user wants to hotremove dax memory, he/she must ensure that
> every section is offlined before unbinding.

Sounds reasonable to me.

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

end of thread, other threads:[~2019-04-20 23:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-20 15:31 [v1 0/2] "Hotremove" persistent memory Pavel Tatashin
2019-04-20 15:31 ` [v1 1/2] device-dax: fix memory and resource leak if hotplug fails Pavel Tatashin
2019-04-20 15:31 ` [v1 2/2] device-dax: "Hotremove" persistent memory that is used like normal RAM Pavel Tatashin
2019-04-20 16:18   ` Dan Williams
2019-04-20 16:30     ` Pavel Tatashin
2019-04-20 16:36       ` Dan Williams
2019-04-20 17:01         ` Pavel Tatashin
2019-04-20 21:02           ` Dan Williams
2019-04-20 22:04             ` Pavel Tatashin
2019-04-20 23:19               ` Dan Williams
2019-04-20 16:34 ` [v1 0/2] "Hotremove" persistent memory Dan Williams
2019-04-20 16:56   ` Pavel Tatashin

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).