Allow persistent memory to be used like normal RAM

From: Dave Hansen <dave.hansen-VuQAYsv1563Yd54FQh9/[email protected]>

walk_system_ram_range() can return an error code either becuase *it* failed, or because the 'func' that it calls returned an error. The memory hotplug does the following:

        ret = walk_system_ram_range(..., func);
        if (ret)
                return ret;

and 'ret' makes it out to userspace, eventually. The problem is, walk_system_ram_range() failues that result from *it* failing (as opposed to 'func') return -1. That leads to a very odd -EPERM (-1) return code out to userspace.

Make walk_system_ram_range() return -EINVAL for internal failures to keep userspace less confused.

This return code is compatible with all the callers that I audited.

Signed-off-by Dave Hansen <dave.hansen-VuQAYsv1563Yd54FQh9/[email protected]>

Cc Dan Williams <[email protected]>

Cc Dave Jiang <[email protected]>

Cc Ross Zwisler <[email protected]>

Cc Vishal Verma <[email protected]>

Cc Tom Lendacky <[email protected]>

Cc Andrew Morton <akpm-de/[email protected]>

Cc Michal Hocko <mhocko-IBi9RG/[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Huang Ying <[email protected]>

Cc Fengguang Wu <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Yaowei Bai <baiyaowei-0p4V/sDNsUmm0O/7XYngnFaTQe2KTcn/@public.gmane.org>

Cc Takashi Iwai <[email protected]>

Cc Jerome Glisse <[email protected]>

---

 b/kernel/resource.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -puN kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 kernel/resource.c

kernel/resource.c 2019-01-24 15:13:13.954199540 -0800 renamed from kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 2019-01-24 15:13:13.950199540 -0800 +1 -1

@@ -375,7 +375,7 @@ static int __walk_iomem_res_desc(resourc
                                 int (*func)(struct resource *, void *))
 {
        struct resource res;
-       int ret = -1;
+       int ret = -EINVAL;
 
        while (start < end &&
               !find_next_iomem_res(start, end, flags, desc, first_lvl, &res)) {

kernel/resource.c 2019-01-24 15:13:13.954199540 -0800 renamed from kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 2019-01-24 15:13:13.950199540 -0800 +0 -0

        start = (u64) start_pfn << PAGE_SHIFT;
        end = ((u64)(start_pfn + nr_pages) << PAGE_SHIFT) - 1;

_

kernel/resource.c int walk_system_ram_range(unsigned long · line 456

Quoted 6 lines

kernel/resource.c renamed from kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 +1 -1

@@ -453,7 +453,7 @@ int walk_system_ram_range(unsigned long
        unsigned long flags;
        struct resource res;
        unsigned long pfn, end_pfn;
-       int ret = -1;
+       int ret = -EINVAL;

Can you either make a similar change to the powerpc version of walk_system_ram_range() in arch/powerpc/mm/mem.c or explain why it's not needed? It *seems* like we'd want both versions of walk_system_ram_range() to behave similarly in this respect.

Quoted 10 lines · Bjorn Helgaas

kernel/resource.c int walk_system_ram_range(unsigned long · line 456

Quoted 6 lines

kernel/resource.c renamed from kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 +1 -1

@@ -453,7 +453,7 @@ int walk_system_ram_range(unsigned long
        unsigned long flags;
        struct resource res;
        unsigned long pfn, end_pfn;
-       int ret = -1;
+       int ret = -EINVAL;

Can you either make a similar change to the powerpc version of walk_system_ram_range() in arch/powerpc/mm/mem.c or explain why it's not needed? It *seems* like we'd want both versions of walk_system_ram_range() to behave similarly in this respect.

Sure. A quick grep shows powerpc being the only other implementation. I'll just add this hunk:

arch/powerpc/mm/mem.c 2019-01-25 12:58:13.215004263 -0800 walk_system_ram_range(unsigned long star · line 191

Quoted 9 lines

diff -puN arch/powerpc/mm/mem.c~memory-hotplug-walk_system_ram_range-returns-neg-1 arch/powerpc/mm/mem.c

arch/powerpc/mm/mem.c 2019-01-25 12:58:13.215004263 -0800 renamed from arch/powerpc/mm/mem.c~memory-hotplug-walk_system_ram_range-returns-neg-1 2019-01-25 12:57:00.000004446 -0800 +1 -1

@@ -188,7 +188,7 @@ walk_system_ram_range(unsigned long star
        struct memblock_region *reg;
        unsigned long end_pfn = start_pfn + nr_pages;
        unsigned long tstart, tend;
-       int ret = -1;
+       int ret = -EINVAL;

I'll also dust off the ol' cross-compiler and make sure I didn't fat-finger anything.

Quoted 10 lines · Bjorn Helgaas

kernel/resource.c int walk_system_ram_range(unsigned long · line 456

Quoted 6 lines

kernel/resource.c renamed from kernel/resource.c~memory-hotplug-walk_system_ram_range-returns-neg-1 +1 -1

@@ -453,7 +453,7 @@ int walk_system_ram_range(unsigned long
        unsigned long flags;
        struct resource res;
        unsigned long pfn, end_pfn;
-       int ret = -1;
+       int ret = -EINVAL;

Can you either make a similar change to the powerpc version of walk_system_ram_range() in arch/powerpc/mm/mem.c or explain why it's not needed? It *seems* like we'd want both versions of walk_system_ram_range() to behave similarly in this respect.

Sure. A quick grep shows powerpc being the only other implementation.

I'll just add this hunk:

arch/powerpc/mm/mem.c 2019-01-25 12:58:13.215004263 -0800 walk_system_ram_range(unsigned long star · line 191

Quoted 9 lines

diff -puN arch/powerpc/mm/mem.c~memory-hotplug-walk_system_ram_range-returns-neg-1 arch/powerpc/mm/mem.c

arch/powerpc/mm/mem.c 2019-01-25 12:58:13.215004263 -0800 renamed from arch/powerpc/mm/mem.c~memory-hotplug-walk_system_ram_range-returns-neg-1 2019-01-25 12:57:00.000004446 -0800 +1 -1

@@ -188,7 +188,7 @@ walk_system_ram_range(unsigned long star 
        struct memblock_region *reg; 
        unsigned long end_pfn = start_pfn + nr_pages; 
        unsigned long tstart, tend; 
-       int ret = -1; 
+       int ret = -EINVAL; 

I'll also dust off the ol' cross-compiler and make sure I didn't fat-finger anything.

From: Dave Hansen <[email protected]>

HMM consumes physical address space for its own use, even though nothing is mapped or accessible there. It uses a special resource description (IORES_DESC_DEVICE_PRIVATE_MEMORY) to uniquely identify these areas.

When HMM consumes address space, it makes a best guess about what to consume. However, it is possible that a future memory or device hotplug can collide with the reserved area. In the case of these conflicts, there is an error message in register_memory_resource().

Later patches in this series move register_memory_resource() from using request_resource_conflict() to __request_region(). Unfortunately, __request_region() does not return the conflict like the previous function did, which makes it impossible to check for IORES_DESC_DEVICE_PRIVATE_MEMORY in a conflicting resource.

Instead of warning in register_memory_resource(), move the check into the core resource code itself (__request_region()) where the conflicting resource _is_ available. This has the added bonus of producing a warning in case of HMM conflicts with devices *or* RAM address space, as opposed to the RAM- only warnings that were there previously.

Signed-off-by Dave Hansen <[email protected]>

Cc Dan Williams <[email protected]>

Cc Dave Jiang <[email protected]>

Cc Ross Zwisler <[email protected]>

Cc Vishal Verma <[email protected]>

Cc Tom Lendacky <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Michal Hocko <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Huang Ying <[email protected]>

Cc Fengguang Wu <[email protected]>

---

 b/kernel/resource.c   |   10 ++++++++++
 b/mm/memory_hotplug.c |    5 -----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff -puN kernel/resource.c~move-request_region-check kernel/resource.c

kernel/resource.c renamed from kernel/resource.c~move-request_region-check +10 -0

@@ -1123,6 +1123,16 @@ struct resource * __request_region(struc
 		conflict = __request_resource(parent, res);
 		if (!conflict)
 			break;
+		/*
+		 * mm/hmm.c reserves physical addresses which then
+		 * become unavailable to other users.  Conflicts are
+		 * not expected.  Be verbose if one is encountered.
+		 */
+		if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {
+			pr_debug("Resource conflict with unaddressable "
+				 "device memory at %#010llx !\n",
+				 (unsigned long long)start);
+		}
 		if (conflict != parent) {
 			if (!(conflict->flags & IORESOURCE_BUSY)) {
 				parent = conflict;

diff -puN mm/memory_hotplug.c~move-request_region-check mm/memory_hotplug.c

mm/memory_hotplug.c renamed from mm/memory_hotplug.c~move-request_region-check +0 -5

@@ -109,11 +109,6 @@ static struct resource *register_memory_
 	res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
 	conflict =  request_resource_conflict(&iomem_resource, res);
 	if (conflict) {
-		if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {
-			pr_debug("Device unaddressable memory block "
-				 "memory hotplug at %#010llx !\n",
-				 (unsigned long long)start);
-		}
 		pr_debug("System RAM resource %pR cannot be added\n", res);
 		kfree(res);
 		return ERR_PTR(-EEXIST);

_

From: Dave Hansen <[email protected]>

HMM consumes physical address space for its own use, even though nothing is mapped or accessible there. It uses a special resource description (IORES_DESC_DEVICE_PRIVATE_MEMORY) to uniquely identify these areas.

When HMM consumes address space, it makes a best guess about what to consume. However, it is possible that a future memory or device hotplug can collide with the reserved area. In the case of these conflicts, there is an error message in register_memory_resource().

Later patches in this series move register_memory_resource() from using request_resource_conflict() to __request_region(). Unfortunately, __request_region() does not return the conflict like the previous function did, which makes it impossible to check for IORES_DESC_DEVICE_PRIVATE_MEMORY in a conflicting resource.

Instead of warning in register_memory_resource(), move the check into the core resource code itself (__request_region()) where the conflicting resource _is_ available. This has the added bonus of producing a warning in case of HMM conflicts with devices *or* RAM address space, as opposed to the RAM- only warnings that were there previously.

Signed-off-by Dave Hansen <[email protected]>

Cc Dan Williams <[email protected]>

Cc Dave Jiang <[email protected]>

Cc Ross Zwisler <[email protected]>

Cc Vishal Verma <[email protected]>

Cc Tom Lendacky <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Michal Hocko <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Huang Ying <[email protected]>

Cc Fengguang Wu <[email protected]>

Cc Jerome Glisse <[email protected]>

---

 b/kernel/resource.c   |   10 ++++++++++
 b/mm/memory_hotplug.c |    5 -----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff -puN kernel/resource.c~move-request_region-check kernel/resource.c

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +0 -0

@@ -1123,6 +1123,16 @@ struct resource * __request_region(struc
                conflict = __request_resource(parent, res);
                if (!conflict)

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +1 -0

+               }
                if (conflict != parent) {
                        if (!(conflict->flags & IORESOURCE_BUSY)) {
                                parent = conflict;

diff -puN mm/memory_hotplug.c~move-request_region-check mm/memory_hotplug.c

mm/memory_hotplug.c 2019-01-24 15:13:14.459199539 -0800 renamed from mm/memory_hotplug.c~move-request_region-check 2019-01-24 15:13:14.455199539 -0800 +0 -5

@@ -109,11 +109,6 @@ static struct resource *register_memory_
        res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
        conflict =  request_resource_conflict(&iomem_resource, res);
        if (conflict) {
-               if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {
-                       pr_debug("Device unaddressable memory block "
-                                "memory hotplug at %#010llx !\n",
-                                (unsigned long long)start);
-               }
                pr_debug("System RAM resource %pR cannot be added\n", res);
                kfree(res);
                return ERR_PTR(-EEXIST);

_

Quoted 13 lines · Dave Hansen

diff -puN kernel/resource.c~move-request_region-check kernel/resource.c

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +6 -0

@@ -1123,6 +1123,16 @@ struct resource * __request_region(struc
                conflict = __request_resource(parent, res);
                if (!conflict)
                        break;
+               /*
+                * mm/hmm.c reserves physical addresses which then
+                * become unavailable to other users.  Conflicts are
+                * not expected.  Be verbose if one is encountered.
+                */
+               if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 struct resource * __request_region(struc · line 1134

Quoted 3 lines

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +3 -0

+                       pr_debug("Resource conflict with unaddressable "
+                                "device memory at %#010llx !\n",
+                                (unsigned long long)start);

I don't object to the change, but are you really OK with this being a pr_debug() message that is only emitted when enabled via either the dynamic debug mechanism or DEBUG being defined? From the comments, it seems more like a KERN_INFO sort of message.

Also, maybe the message would be more useful if it included the conflicting resource as well as the region you're requesting? Many of the other callers of request_resource_conflict() have something like this:

dev_err(dev, "resource collision: %pR conflicts with %s %pR\n",

        new, conflict->name, conflict);

Quoted 14 lines · Bjorn Helgaas

Quoted 13 lines · Dave Hansen

diff -puN kernel/resource.c~move-request_region-check kernel/resource.c

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +6 -0

@@ -1123,6 +1123,16 @@ struct resource * __request_region(struc
                conflict = __request_resource(parent, res);
                if (!conflict)
                        break;
+               /*
+                * mm/hmm.c reserves physical addresses which then
+                * become unavailable to other users.  Conflicts are
+                * not expected.  Be verbose if one is encountered.
+                */
+               if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {

Quoted 8 lines

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 struct resource * __request_region(struc · line 1134

Quoted 3 lines

kernel/resource.c 2019-01-24 15:13:14.458199539 -0800 renamed from kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800 +3 -0

+                       pr_debug("Resource conflict with unaddressable "
+                                "device memory at %#010llx !\n",
+                                (unsigned long long)start);

I don't object to the change, but are you really OK with this being a pr_debug() message that is only emitted when enabled via either the dynamic debug mechanism or DEBUG being defined? From the comments, it seems more like a KERN_INFO sort of message.

I left it consistent with the original message that was in the code. I'm happy to change it, though, if the consumers of it (Jerome, basically) want something different.

From: Dave Hansen <[email protected]>

This is intended for use with NVDIMMs that are physically persistent (physically like flash) so that they can be used as a cost-effective RAM replacement. Intel Optane DC persistent memory is one implementation of this kind of NVDIMM.

Currently, a persistent memory region is "owned" by a device driver, either the "Direct DAX" or "Filesystem DAX" drivers. These drivers allow applications to explicitly use persistent memory, generally by being modified to use special, new libraries. (DIMM-based persistent memory hardware/software is described in great detail
here: Documentation/nvdimm/nvdimm.txt).

However, this limits persistent memory use to applications which *have* been modified. To make it more broadly usable, this driver "hotplugs" memory into the kernel, to be managed and used just like normal RAM would be.

To make this work, management software must remove the device from being controlled by the "Device DAX" infrastructure:

echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind

and then bind it to this new driver:

echo -n dax0.0 > /sys/bus/dax/drivers/kmem/new_id echo -n dax0.0 > /sys/bus/dax/drivers/kmem/bind

After this, there will be a number of new memory sections visible in sysfs that can be onlined, or that may get onlined by existing udev-initiated memory hotplug rules.

This rebinding procedure is currently a one-way trip. Once memory is bound to "kmem", it's there permanently and can not be unbound and assigned back to device_dax.

The kmem driver will never bind to a dax device unless the device is *explicitly* bound to the driver. There are two reasons for
this: One, since it is a one-way trip, it can not be undone if bound incorrectly. Two, the kmem driver destroys data on the device. Think of if you had good data on a pmem device. It would be catastrophic if you compile-in "kmem", but leave out the "device_dax" driver. kmem would take over the device and write volatile data all over your good data.

This inherits any existing NUMA information for the newly-added memory from the persistent memory device that came from the firmware. On Intel platforms, the firmware has guarantees that require each socket's persistent memory to be in a separate memory-only NUMA node. That means that this patch is not expected to create NUMA nodes, but will simply hotplug memory into existing nodes.

Because NUMA nodes are created, the existing NUMA APIs and tools are sufficient to create policies for applications or memory areas to have affinity for or an aversion to using this memory.

There is currently some metadata at the beginning of pmem regions. The section-size memory hotplug restrictions, plus this small reserved area can cause the "loss" of a section or two of capacity. This should be fixable in follow-on patches. But, as a first step, losing 256MB of memory (worst case) out of hundreds of gigabytes is a good tradeoff vs. the required code to fix this up precisely. This calculation is also the reason we export memory_block_size_bytes().

Signed-off-by Dave Hansen <[email protected]>

Cc Dan Williams <[email protected]>

Cc Dave Jiang <[email protected]>

Cc Ross Zwisler <[email protected]>

Cc Vishal Verma <[email protected]>

Cc Tom Lendacky <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Michal Hocko <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc Huang Ying <[email protected]>

Cc Fengguang Wu <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Yaowei Bai <[email protected]>

Cc Takashi Iwai <[email protected]>

Cc Jerome Glisse <[email protected]>

---

  b/drivers/base/memory.c |    1
  b/drivers/dax/Kconfig   |   16 +++++++
  b/drivers/dax/Makefile  |    1
  b/drivers/dax/kmem.c    |  108 ++++++++++++++++++++++++++++++++++++++++++++++++
  4 files changed, 126 insertions(+)

diff -puN drivers/base/memory.c~dax-kmem-try-4 drivers/base/memory.c

drivers/base/memory.c renamed from drivers/base/memory.c~dax-kmem-try-4 +1 -0

@@ -88,6 +88,7 @@ unsigned long __weak memory_block_size_b
  {
  	return MIN_MEMORY_BLOCK_SIZE;
  }
+EXPORT_SYMBOL_GPL(memory_block_size_bytes);
 
  static unsigned long get_memory_block_size(void)
  {

diff -puN drivers/dax/Kconfig~dax-kmem-try-4 drivers/dax/Kconfig

drivers/dax/Kconfig renamed from drivers/dax/Kconfig~dax-kmem-try-4 +16 -0

@@ -32,6 +32,22 @@ config DEV_DAX_PMEM
 
  	  Say M if unsure
 
+config DEV_DAX_KMEM
+	tristate "KMEM DAX: volatile-use of persistent memory"
+	default DEV_DAX
+	depends on DEV_DAX
+	depends on MEMORY_HOTPLUG # for add_memory() and friends
+	help
+	  Support access to persistent memory as if it were RAM.  This
+	  allows easier use of persistent memory by unmodified
+	  applications.
+
+	  To use this feature, a DAX device must be unbound from the
+	  device_dax driver (PMEM DAX) and bound to this kmem driver
+	  on each boot.
+
+	  Say N if unsure.
+
  config DEV_DAX_PMEM_COMPAT
  	tristate "PMEM DAX: support the deprecated /sys/class/dax interface"
  	depends on DEV_DAX_PMEM

diff -puN /dev/null drivers/dax/kmem.c

drivers/dax/kmem.c new file +108 -0

@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2016-2018 Intel Corporation. All rights reserved. */
+#include <linux/memremap.h>
+#include <linux/pagemap.h>
+#include <linux/memory.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/pfn_t.h>
+#include <linux/slab.h>
+#include <linux/dax.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include "dax-private.h"
+#include "bus.h"
+
+int dev_dax_kmem_probe(struct device *dev)
+{
+	struct dev_dax *dev_dax = to_dev_dax(dev);
+	struct resource *res = &dev_dax->region->res;
+	resource_size_t kmem_start;
+	resource_size_t kmem_size;
+	resource_size_t kmem_end;
+	struct resource *new_res;
+	int numa_node;
+	int rc;
+
+	/*
+	 * Ensure good NUMA information for the persistent memory.
+	 * Without this check, there is a risk that slow memory
+	 * could be mixed in a node with faster memory, causing
+	 * unavoidable performance issues.
+	 */
+	numa_node = dev_dax->target_node;
+	if (numa_node < 0) {
+		dev_warn(dev, "rejecting DAX region %pR with invalid node: %d\n",
+			 res, numa_node);
+		return -EINVAL;
+	}
+
+	/* Hotplug starting at the beginning of the next block: */
+	kmem_start = ALIGN(res->start, memory_block_size_bytes());
+
+	kmem_size = resource_size(res);
+	/* Adjust the size down to compensate for moving up kmem_start: */
+        kmem_size -= kmem_start - res->start;
+	/* Align the size down to cover only complete blocks: */
+	kmem_size &= ~(memory_block_size_bytes() - 1);
+	kmem_end = kmem_start+kmem_size;
+
+	/* Region is permanently reserved.  Hot-remove not yet implemented. */
+	new_res = request_mem_region(kmem_start, kmem_size, dev_name(dev));
+	if (!new_res) {
+		dev_warn(dev, "could not reserve region [%pa-%pa]\n",
+			 &kmem_start, &kmem_end);
+		return -EBUSY;
+	}
+
+	/*
+	 * Set flags appropriate for System RAM.  Leave ..._BUSY clear
+	 * so that add_memory() can add a child resource.  Do not
+	 * inherit flags from the parent since it may set new flags
+	 * unknown to us that will break add_memory() below.
+	 */
+	new_res->flags = IORESOURCE_SYSTEM_RAM;
+	new_res->name = dev_name(dev);
+
+	rc = add_memory(numa_node, new_res->start, resource_size(new_res));
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+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
+	 * request_mem_region().
+	 */
+	return -EBUSY;
+}
+
+static struct dax_device_driver device_dax_kmem_driver = {
+	.drv = {
+		.probe = dev_dax_kmem_probe,
+		.remove = dev_dax_kmem_remove,
+	},
+};
+
+static int __init dax_kmem_init(void)
+{
+	return dax_driver_register(&device_dax_kmem_driver);
+}
+
+static void __exit dax_kmem_exit(void)
+{
+	dax_driver_unregister(&device_dax_kmem_driver);
+}
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
+module_init(dax_kmem_init);
+module_exit(dax_kmem_exit);
+MODULE_ALIAS_DAX_DEVICE(0);

diff -puN drivers/dax/Makefile~dax-kmem-try-4 drivers/dax/Makefile

drivers/dax/Makefile renamed from drivers/dax/Makefile~dax-kmem-try-4 +1 -0

@@ -1,6 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
  obj-$(CONFIG_DAX) += dax.o
  obj-$(CONFIG_DEV_DAX) += device_dax.o
+obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o
 
  dax-y := super.o
  dax-y += bus.o

_
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Hi, Dave,

While chatting with my colleague Erwin about the patchset, it occurred that we're not clear about the error handling part. Specifically,

1. If an uncorrectable error is detected during a 'load' in the hot plugged pmem region, how will the error be handled? will it be handled like PMEM or DRAM?

2. If a poison is set, and is persistent, which entity should clear the poison, and badblock(if applicable)? If it's user's responsibility, does ndctl support the clearing in this mode?

-----Original Message-----
From: Linux-nvdimm [mailto:[email protected]] On Behalf Of Dan Williams
Sent: Friday, January 25, 2019 2:28 PM
To: Jane Chu <[email protected]>
Cc: Tom Lendacky <[email protected]>; Michal Hocko <[email protected]>; linux-nvdimm <[email protected]>; Takashi Iwai <[email protected]>; Dave Hansen <[email protected]>; Huang, Ying <[email protected]>; Linux Kernel Mailing List <[email protected]>; Linux MM <[email protected]>; Jérôme Glisse <[email protected]>; Borislav Petkov <[email protected]>; Yaowei Bai <[email protected]>; Ross Zwisler <[email protected]>; Bjorn Helgaas <[email protected]>; Andrew Morton <[email protected]>; Wu, Fengguang <[email protected]>
Subject: Re: [PATCH 5/5] dax: "Hotplug" persistent memory for use like normal RAM

Quoted 9 lines · Jane Chu

Hi, Dave,

While chatting with my colleague Erwin about the patchset, it occurred that we're not clear about the error handling part. Specifically,

1. If an uncorrectable error is detected during a 'load' in the hot plugged pmem region, how will the error be handled? will it be handled like PMEM or DRAM?

DRAM.

Quoted 3 lines

2. If a poison is set, and is persistent, which entity should clear the poison, and badblock(if applicable)? If it's user's responsibility, does ndctl support the clearing in this mode?

With persistent memory advertised via a static logical-to-physical storage/dax device mapping, once an error develops it destroys a physical *and* logical part of a device address space. That loss of logical address space makes error clearing a necessity. However, with the DRAM / "System RAM" error handling model, the OS can just offline the page and map a different one to repair the logical address space. So, no, ndctl will not have explicit enabling to clear volatile errors, the OS will just dynamically offline problematic pages.
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Dan

Thanks for the insights!

Can I say, the UCE is delivered from h/w to OS in a single way in case of machine check, only PMEM/DAX stuff filter out UC address and managed in its own way by badblocks, if PMEM/DAX doesn't do so, then common RAS workflow will kick in, right?

And how about when ARS is involved but no machine check fired for the function of this patchset?

Quoted 11 lines · Fan

Dan

Thanks for the insights!

Can I say, the UCE is delivered from h/w to OS in a single way in case of machine
check, only PMEM/DAX stuff filter out UC address and managed in its own way by
badblocks, if PMEM/DAX doesn't do so, then common RAS workflow will kick in,
right?

The common RAS workflow always kicks in, it's just the page state presented by a DAX mapping needs distinct handling. Once it is hot-plugged it no longer needs to be treated differently than "System RAM".

Quoted 3 lines

And how about when ARS is involved but no machine check fired for the function
of this patchset?

The hotplug effectively disconnects this address range from the ARS results. They will still be reported in the libnvdimm "region" level badblocks instance, but there's no safe / coordinated way to go clear those errors without additional kernel enabling. There is no "clear error" semantic for "System RAM".

Quoted 22 lines · Dan Williams

Quoted 11 lines · Fan

Dan

Thanks for the insights!

Can I say, the UCE is delivered from h/w to OS in a single way in case of machine
check, only PMEM/DAX stuff filter out UC address and managed in its own way by
badblocks, if PMEM/DAX doesn't do so, then common RAS workflow will kick in,
right?

The common RAS workflow always kicks in, it's just the page state presented by a DAX mapping needs distinct handling. Once it is hot-plugged it no longer needs to be treated differently than "System RAM".

Quoted 3 lines

And how about when ARS is involved but no machine check fired for the function
of this patchset?

Quoted 6 lines

The hotplug effectively disconnects this address range from the ARS results. They will still be reported in the libnvdimm "region" level badblocks instance, but there's no safe / coordinated way to go clear those errors without additional kernel enabling. There is no "clear error" semantic for "System RAM".

Perhaps as future enabling, the kernel can go perform "clear error" for offlined pages, and make them usable again. But I'm not sure how prepared mm is to re-accept pages previously offlined.

Quoted 30 lines · Vishal L

Quoted 22 lines · Dan Williams

Quoted 11 lines · Fan

Dan

Thanks for the insights!

Can I say, the UCE is delivered from h/w to OS in a single way in case of machine
check, only PMEM/DAX stuff filter out UC address and managed in its own way by
badblocks, if PMEM/DAX doesn't do so, then common RAS workflow will kick in,
right?

The common RAS workflow always kicks in, it's just the page state presented by a DAX mapping needs distinct handling. Once it is hot-plugged it no longer needs to be treated differently than "System RAM".

Quoted 3 lines

And how about when ARS is involved but no machine check fired for the function
of this patchset?

Quoted 6 lines

The hotplug effectively disconnects this address range from the ARS results. They will still be reported in the libnvdimm "region" level badblocks instance, but there's no safe / coordinated way to go clear those errors without additional kernel enabling. There is no "clear error" semantic for "System RAM".

Quoted 4 lines

Perhaps as future enabling, the kernel can go perform "clear error" for offlined pages, and make them usable again. But I'm not sure how prepared mm is to re-accept pages previously offlined.

Offlining a DRAM backed page due to an UC makes sense because
a. the physical DRAM cell might still have an error
b. power cycle, scrubing could potentially 'repair' the DRAM cell,
making the page usable again.

But for a PMEM backed page, neither is true. If a poison bit is set in a page, that indicates the underlying hardware has completed the repair work, all that's left is for software to recover. Secondly, because poison is persistent, unless software explicitly clear the bit, the page is permanently unusable.

Quoted 43 lines · Jane Chu

Quoted 30 lines · Vishal L

Quoted 22 lines · Dan Williams

Quoted 11 lines · Fan

Dan

Thanks for the insights!

Can I say, the UCE is delivered from h/w to OS in a single way in case of machine
check, only PMEM/DAX stuff filter out UC address and managed in its own way by
badblocks, if PMEM/DAX doesn't do so, then common RAS workflow will kick in,
right?

The common RAS workflow always kicks in, it's just the page state presented by a DAX mapping needs distinct handling. Once it is hot-plugged it no longer needs to be treated differently than "System RAM".

Quoted 3 lines

And how about when ARS is involved but no machine check fired for the function
of this patchset?

Quoted 6 lines

The hotplug effectively disconnects this address range from the ARS results. They will still be reported in the libnvdimm "region" level badblocks instance, but there's no safe / coordinated way to go clear those errors without additional kernel enabling. There is no "clear error" semantic for "System RAM".

Quoted 4 lines

Perhaps as future enabling, the kernel can go perform "clear error" for offlined pages, and make them usable again. But I'm not sure how prepared mm is to re-accept pages previously offlined.

Offlining a DRAM backed page due to an UC makes sense because
a. the physical DRAM cell might still have an error
b. power cycle, scrubing could potentially 'repair' the DRAM cell,
making the page usable again.

Quoted 5 lines

But for a PMEM backed page, neither is true. If a poison bit is set in a page, that indicates the underlying hardware has completed the repair work, all that's left is for software to recover. Secondly, because poison is persistent, unless software explicitly clear the bit, the page is permanently unusable.

Not permanently... system-owner always has the option to use the device-DAX and ARS mechanisms to clear errors at the next boot. There's just no kernel enabling to do that automatically as a part of this patch set.

However, we should consider this along with the userspace enabling to control which device-dax instances are set aside for hotplug. It would make sense to have a "clear errors before hotplug" configuration option.

However, we should consider this along with the userspace enabling to control which device-dax instances are set aside for hotplug. It would make sense to have a "clear errors before hotplug" configuration option.

On Fri 25-01-19 11:15:08, Dan Williams wrote:
[...]

Quoted 4 lines

However, we should consider this along with the userspace enabling to control which device-dax instances are set aside for hotplug. It would make sense to have a "clear errors before hotplug" configuration option.

I am not sure I understand. Do you mean to clear HWPoison when the memory is hotadded (add_pages) or onlined (resp. move_pfn_range_to_zone)?

From: Dave Hansen <[email protected]>

This is intended for use with NVDIMMs that are physically persistent (physically like flash) so that they can be used as a cost-effective RAM replacement. Intel Optane DC persistent memory is one implementation of this kind of NVDIMM.

Currently, a persistent memory region is "owned" by a device driver, either the "Direct DAX" or "Filesystem DAX" drivers. These drivers allow applications to explicitly use persistent memory, generally by being modified to use special, new libraries. (DIMM-based persistent memory hardware/software is described in great detail
here: Documentation/nvdimm/nvdimm.txt).

However, this limits persistent memory use to applications which *have* been modified. To make it more broadly usable, this driver "hotplugs" memory into the kernel, to be managed and used just like normal RAM would be.

To make this work, management software must remove the device from being controlled by the "Device DAX" infrastructure:

echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind

I've used your patches on fake hardware (memmap=xx!yy) with an older nvdimm-pending branch (without Keith's patches). It worked fine. This time I am running on real Intel hardware. Any idea where to look ?

Quoted 3 lines

I've used your patches on fake hardware (memmap=xx!yy) with an older nvdimm-pending branch (without Keith's patches). It worked fine. This time I am running on real Intel hardware. Any idea where to look ?

I've run them on real Intel hardware too.

Could you share the exact sequence of commands you're issuing to reproduce the hang? My guess would be that there's some odd interaction between Dan's latest branch and my now (slightly) stale patches.

I'll refresh them this week and see if I can reproduce what you're seeing.

Quoted 11 lines · Dave Hansen

Quoted 3 lines · Brice Goglin

I've used your patches on fake hardware (memmap=xx!yy) with an older nvdimm-pending branch (without Keith's patches). It worked fine. This time I am running on real Intel hardware. Any idea where to look ?

I've run them on real Intel hardware too.

Could you share the exact sequence of commands you're issuing to reproduce the hang? My guess would be that there's some odd interaction between Dan's latest branch and my now (slightly) stale patches.

I'll refresh them this week and see if I can reproduce what you're seeing.

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

I tried with and without dax_pmem_compat loaded, but it doesn't help.

Quoted 27 lines

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

Quoted 1 line

I tried with and without dax_pmem_compat loaded, but it doesn't help.

I think this is due to:

a9f1ffdb6a20 device-dax: Auto-bind device after successful new_id

I missed that this path is also called in the remove_id path. Thanks for the bug report! I'll get this fixed up.

Quoted 28 lines · Dan Williams

Quoted 27 lines · Brice Goglin

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

Quoted 7 lines

Quoted 1 line

I tried with and without dax_pmem_compat loaded, but it doesn't help.

I think this is due to:

a9f1ffdb6a20 device-dax: Auto-bind device after successful new_id

I missed that this path is also called in the remove_id path. Thanks for the bug report! I'll get this fixed up.

Now that remove_id is fixed, things fails later in Dave's procedure:

# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/new_id

# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/bind
-bash: echo: write error: No such device

(And nothing seems to have changed in /sys/devices/system/memory/*/state)

Quoted 34 lines · Dan Williams

Quoted 33 lines · Brice Goglin

Quoted 27 lines

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

Quoted 1 line

I tried with and without dax_pmem_compat loaded, but it doesn't help.

I think this is due to:

a9f1ffdb6a20 device-dax: Auto-bind device after successful new_id

Quoted 8 lines

Quoted 2 lines

I missed that this path is also called in the remove_id path. Thanks for the bug report! I'll get this fixed up.

Now that remove_id is fixed, things fails later in Dave's procedure:

# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/new_id

In the current version of the code the bind is not necessary, so the lack of error messages here means the bind succeeded.

Quoted 2 lines

# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/bind
-bash: echo: write error: No such device

This also happens when the device is already bound.

Quoted 1 line

(And nothing seems to have changed in /sys/devices/system/memory/*/state)

What does "cat /proc/iomem" say?

Le 13/02/2019 à 09:24, Dan Williams a écrit :

Quoted 44 lines · Brice Goglin

Quoted 34 lines · Dan Williams

Quoted 33 lines · Brice Goglin

Quoted 27 lines

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

Quoted 1 line

I tried with and without dax_pmem_compat loaded, but it doesn't help.

I think this is due to:

a9f1ffdb6a20 device-dax: Auto-bind device after successful new_id

Quoted 7 lines

Quoted 2 lines

I missed that this path is also called in the remove_id path. Thanks for the bug report! I'll get this fixed up.

Now that remove_id is fixed, things fails later in Dave's procedure:

# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/new_id

In the current version of the code the bind is not necessary, so the lack of error messages here means the bind succeeded.

Quoted 7 lines

Quoted 3 lines

# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/bind
-bash: echo: write error: No such device

This also happens when the device is already bound.

Quoted 1 line

(And nothing seems to have changed in /sys/devices/system/memory/*/state)

What does "cat /proc/iomem" say?

3060000000-1aa5fffffff : Persistent Memory 3060000000-36481fffff : namespace0.0 3680000000-1a9ffffffff : dax0.0 3680000000-1a9ffffffff : System RAM
(the last line wasn't here before attaching to kmem)

I said nothing changed in memory/*/state, I actually meant that nothing was offline. But things are actually working!

First, node4 appeared, all memory is already attached to it without having to write to memory/*/state

Node 4 MemTotal:       1558183936 kB
Node 4 MemFree:        1558068564 kB
Node 4 MemUsed:          115372 kB

I wasn't expecting node4 to appear because the machine has no /sys/firmware/acpi/tables/HMAT when running in 1LM (there's one in 2LM). I thought you said in the past that no HMAT would mean memory would be added to the existing DDR node?

Thanks!

Brice

Quoted 38 lines · Brice Goglin

Quoted 37 lines · Dan Williams

Quoted 36 lines · Brice Goglin

Quoted 35 lines · Dan Williams

Quoted 27 lines · Brice Goglin

# ndctl disable-region all
# ndctl zero-labels all
# ndctl enable-region region0
# ndctl create-namespace -r region0 -t pmem -m devdax
{
  "dev":"namespace0.0",
  "mode":"devdax",
  "map":"dev",
  "size":"1488.37 GiB (1598.13 GB)",
  "uuid":"ad0096d7-3fe7-4402-b529-ad64ed0bf789",
  "daxregion":{
    "id":0,
    "size":"1488.37 GiB (1598.13 GB)",
    "align":2097152,
    "devices":[
      {
        "chardev":"dax0.0",
        "size":"1488.37 GiB (1598.13 GB)"
      }
    ]
  },
  "align":2097152
}
# ndctl enable-namespace namespace0.0
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
<hang>

Quoted 1 line

I tried with and without dax_pmem_compat loaded, but it doesn't help.

I think this is due to:

a9f1ffdb6a20 device-dax: Auto-bind device after successful new_id

I missed that this path is also called in the remove_id path. Thanks for the bug report! I'll get this fixed up.

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Now that remove_id is fixed, things fails later in Dave's procedure:

# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/remove_id
# echo -n dax0.0 > /sys/bus/dax/drivers/device_dax/unbind
# echo -n dax0.0 > /sys/bus/dax/drivers/kmem/new_id

In the current version of the code the bind is not necessary, so the lack of error messages here means the bind succeeded.

It looks like "unbind" is required to make the PMEM appear as a new node. If I remove_id from devdax and new_id to kmem without "unbind" in the middle, nothing appears.

Writing to "kmem/bind" didn't seem necessary.

v3 spurred a bunch of really good discussion. Thanks to everybody that made comments and suggestions!

I would still love some Acks on this from the folks on cc, even if it is on just the patch touching your area.

Note: these are based on commit d2f33c19644 in:

git://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git libnvdimm-pending

Changes since v3:
* Move HMM-related resource warning instead of removing it
* Use __request_resource() directly instead of devm.
* Create a separate DAX_PMEM Kconfig option, complete with help text
* Update patch descriptions and cover letter to give a better overview of use-cases and hardware where this might be useful.

v3 spurred a bunch of really good discussion. Thanks to everybody that made comments and suggestions!

I would still love some Acks on this from the folks on cc, even if it is on just the patch touching your area.

Note: these are based on commit d2f33c19644 in:

git://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git libnvdimm-pending

Changes since v3:
* Move HMM-related resource warning instead of removing it
* Use __request_resource() directly instead of devm.
* Create a separate DAX_PMEM Kconfig option, complete with help text
* Update patch descriptions and cover letter to give a better overview of use-cases and hardware where this might be useful.

Changes since v2:
* Updates to dev_dax_kmem_probe() in patch 5:
* Reject probes for devices with bad NUMA nodes. Keeps slow memory from being added to node 0.
* Use raw request_mem_region()
* Add comments about permanent reservation
* use dev_*() instead of printk's
* Add references to nvdimm documentation in descriptions
* Remove unneeded GPL export
* Add Kconfig prompt and help text

Changes since v1:
* Now based on git://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm.git
* Use binding/unbinding from "dax bus" code
* Move over to a "dax bus" driver from being an nvdimm driver

--

Persistent memory is cool. But, currently, you have to rewrite your applications to use it. Wouldn't it be cool if you could just have it show up in your system like normal RAM and get to it like a slow blob of memory? Well... have I got the patch series for you!

== Background / Use Cases ==

Persistent Memory (aka Non-Volatile DIMMs / NVDIMMS) themselves are described in detail in Documentation/nvdimm/nvdimm.txt. However, this documentation focuses on actually using them as storage. This set is focused on using NVDIMMs as DRAM replacement.

This is intended for Intel-style NVDIMMs (aka. Intel Optane DC persistent memory) NVDIMMs. These DIMMs are physically persistent, more akin to flash than traditional RAM. They are also expected to be more cost-effective than using RAM, which is why folks want this set in the first place.

This set is not intended for RAM-based NVDIMMs. Those are not cost-effective vs. plain RAM, and this using them here would simply be a waste.

But, why would you bother with this approach? Intel itself [1] has announced a hardware feature that does something very similar:
"Memory Mode" which turns DRAM into a cache in front of persistent memory, which is then as a whole used as normal "RAM"?

Here are a few reasons:
1. The capacity of memory mode is the size of your persistent memory that you dedicate. DRAM capacity is "lost" because it is used for cache. With this, you get PMEM+DRAM capacity for memory.
2. DRAM acts as a cache with memory mode, and caches can lead to unpredictable latencies. Since memory mode is all-or-nothing (either all your DRAM is used as cache or none is), your entire memory space is exposed to these unpredictable latencies. This solution lets you guarantee DRAM latencies if you need them.
3. The new "tier" of memory is exposed to software. That means that you can build tiered applications or infrastructure. A cloud provider could sell cheaper VMs that use more PMEM and more expensive ones that use DRAM. That's impossible with memory mode.

Don't take this as criticism of memory mode. Memory mode is awesome, and doesn't strictly require *any* software changes (we have software changes proposed for optimizing it though). It has tons of other advantages over *this* approach. Basically, we believe that the approach in these patches is complementary to memory mode and that both can live side-by-side in harmony.

== Patch Set Overview ==

This series adds a new "driver" to which pmem devices can be attached. Once attached, the memory "owned" by the device is hot-added to the kernel and managed like any other memory. On systems with an HMAT (a new ACPI table), each socket (roughly) will have a separate NUMA node for its persistent memory so this newly-added memory can be selected by its unique NUMA node.

Quoted 5 lines

This is intended for Intel-style NVDIMMs (aka. Intel Optane DC persistent memory) NVDIMMs. These DIMMs are physically persistent, more akin to flash than traditional RAM. They are also expected to be more cost-effective than using RAM, which is why folks want this set in the first place.

What variant of NVDIMM's F/P or both?

Quoted 9 lines

== Patch Set Overview ==

This series adds a new "driver" to which pmem devices can be attached. Once attached, the memory "owned" by the device is hot-added to the kernel and managed like any other memory. On systems with an HMAT (a new ACPI table), each socket (roughly) will have a separate NUMA node for its persistent memory so this newly-added memory can be selected by its unique NUMA node.

NUMA is distance based topology, does HMAT solve these problems?

How do we prevent fallback nodes of normal nodes being pmem nodes?

On an unexpected crash/failure is there a scrubbing mechanism or do we rely on the allocator to do the right thing prior to reallocating any memory.

Will frequent zero'ing hurt NVDIMM/pmem's life times?