RE: [RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is invalid
Hi David > -Original Message- > From: David Hildenbrand > Sent: Tuesday, July 7, 2020 7:34 PM > To: Justin He ; Catalin Marinas > ; Will Deacon ; Dan Williams > ; Vishal Verma ; Dave > Jiang > Cc: Michal Hocko ; Andrew Morton foundation.org>; Mike Rapoport ; Baoquan He > ; Chuhong Yuan ; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; linux- > m...@kvack.org; linux-nvd...@lists.01.org; Kaly Xin > Subject: Re: [RFC PATCH v2 2/3] device-dax: use fallback nid when > numa_node is invalid > > On 07.07.20 07:59, Jia He wrote: > > Previously, numa_off is set unconditionally at the end of > dummy_numa_init(), > > even with a fake numa node. Then ACPI detects node id as NUMA_NO_NODE(-1) > in > > acpi_map_pxm_to_node() because it regards numa_off as turning off the > numa > > node. Hence dev_dax->target_node is NUMA_NO_NODE on arm64 with fake numa. > > > > Without this patch, pmem can't be probed as a RAM device on arm64 if > SRAT table > > isn't present: > > $ndctl create-namespace -fe namespace0.0 --mode=devdax --map=dev -s 1g - > a 64K > > kmem dax0.0: rejecting DAX region [mem 0x24040-0x2bfff] with > invalid node: -1 > > kmem: probe of dax0.0 failed with error -22 > > > > This fixes it by using fallback memory_add_physaddr_to_nid() as nid. > > > > Suggested-by: David Hildenbrand > > Signed-off-by: Jia He > > --- > > I noticed that on powerpc memory_add_physaddr_to_nid is not exported for > module > > driver. Set it to RFC due to this concern. > > > > drivers/dax/kmem.c | 22 ++ > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > > index 275aa5f87399..68e693ca6d59 100644 > > --- a/drivers/dax/kmem.c > > +++ b/drivers/dax/kmem.c > > @@ -28,20 +28,22 @@ int dev_dax_kmem_probe(struct device *dev) > > resource_size_t kmem_end; > > struct resource *new_res; > > const char *new_res_name; > > - int numa_node; > > + int numa_node, new_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. > > +* Without this check, there is a risk but not fatal that slow > > +* memory could be mixed in a node with faster memory, causing > > +* unavoidable performance issues. Furthermore, fallback node > > +* id can be used when numa_node is invalid. > > */ > > 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; > > + new_node = memory_add_physaddr_to_nid(kmem_start); > > + dev_info(dev, "changing nid from %d to %d for DAX > region %pR\n", > > + numa_node, new_node, res); > > + numa_node = new_node; > > Now, the warning does not really make sense. We have NUMA_NO_NODE (< 0), > that is not a change in the nid, but a selection of a nid. Printing > NUMA_NO_NODE does not make too much sense. I suggest just getting rid of > new_node and turning the dev_info() into something like > > dev_info(dev, "using nid %d for DAX region with undefined nid %pR\n", > numa_node, res); > Okay, I will update it as your sugguestion. Thanks -- Cheers, Justin (Jia He)
Re: [RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is invalid
On 07.07.20 07:59, Jia He wrote: > Previously, numa_off is set unconditionally at the end of dummy_numa_init(), > even with a fake numa node. Then ACPI detects node id as NUMA_NO_NODE(-1) in > acpi_map_pxm_to_node() because it regards numa_off as turning off the numa > node. Hence dev_dax->target_node is NUMA_NO_NODE on arm64 with fake numa. > > Without this patch, pmem can't be probed as a RAM device on arm64 if SRAT > table > isn't present: > $ndctl create-namespace -fe namespace0.0 --mode=devdax --map=dev -s 1g -a 64K > kmem dax0.0: rejecting DAX region [mem 0x24040-0x2bfff] with invalid > node: -1 > kmem: probe of dax0.0 failed with error -22 > > This fixes it by using fallback memory_add_physaddr_to_nid() as nid. > > Suggested-by: David Hildenbrand > Signed-off-by: Jia He > --- > I noticed that on powerpc memory_add_physaddr_to_nid is not exported for > module > driver. Set it to RFC due to this concern. > > drivers/dax/kmem.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 275aa5f87399..68e693ca6d59 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c > @@ -28,20 +28,22 @@ int dev_dax_kmem_probe(struct device *dev) > resource_size_t kmem_end; > struct resource *new_res; > const char *new_res_name; > - int numa_node; > + int numa_node, new_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. > + * Without this check, there is a risk but not fatal that slow > + * memory could be mixed in a node with faster memory, causing > + * unavoidable performance issues. Furthermore, fallback node > + * id can be used when numa_node is invalid. >*/ > 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; > + new_node = memory_add_physaddr_to_nid(kmem_start); > + dev_info(dev, "changing nid from %d to %d for DAX region %pR\n", > + numa_node, new_node, res); > + numa_node = new_node; Now, the warning does not really make sense. We have NUMA_NO_NODE (< 0), that is not a change in the nid, but a selection of a nid. Printing NUMA_NO_NODE does not make too much sense. I suggest just getting rid of new_node and turning the dev_info() into something like dev_info(dev, "using nid %d for DAX region with undefined nid %pR\n", numa_node, res); -- Thanks, David / dhildenb
RE: [RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is invalid
[+] add Powerpc maintainers to check my concern about memory_add_physaddr_to_nid Exporting -- Cheers, Justin (Jia He) > -Original Message- > From: Jia He > Sent: Tuesday, July 7, 2020 1:59 PM > To: Catalin Marinas ; Will Deacon > ; Dan Williams ; Vishal Verma > ; Dave Jiang > Cc: Michal Hocko ; Andrew Morton foundation.org>; Mike Rapoport ; Baoquan He > ; Chuhong Yuan ; linux-arm- > ker...@lists.infradead.org; linux-kernel@vger.kernel.org; linux- > m...@kvack.org; linux-nvd...@lists.01.org; Kaly Xin ; > Justin He > Subject: [RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is > invalid > > Previously, numa_off is set unconditionally at the end of > dummy_numa_init(), > even with a fake numa node. Then ACPI detects node id as NUMA_NO_NODE(-1) > in > acpi_map_pxm_to_node() because it regards numa_off as turning off the numa > node. Hence dev_dax->target_node is NUMA_NO_NODE on arm64 with fake numa. > > Without this patch, pmem can't be probed as a RAM device on arm64 if SRAT > table > isn't present: > $ndctl create-namespace -fe namespace0.0 --mode=devdax --map=dev -s 1g -a > 64K > kmem dax0.0: rejecting DAX region [mem 0x24040-0x2bfff] with > invalid node: -1 > kmem: probe of dax0.0 failed with error -22 > > This fixes it by using fallback memory_add_physaddr_to_nid() as nid. > > Suggested-by: David Hildenbrand > Signed-off-by: Jia He > --- > I noticed that on powerpc memory_add_physaddr_to_nid is not exported for > module > driver. Set it to RFC due to this concern. > > drivers/dax/kmem.c | 22 ++ > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 275aa5f87399..68e693ca6d59 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c > @@ -28,20 +28,22 @@ int dev_dax_kmem_probe(struct device *dev) > resource_size_t kmem_end; > struct resource *new_res; > const char *new_res_name; > - int numa_node; > + int numa_node, new_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. > + * Without this check, there is a risk but not fatal that slow > + * memory could be mixed in a node with faster memory, causing > + * unavoidable performance issues. Furthermore, fallback node > + * id can be used when numa_node is invalid. >*/ > 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; > + new_node = memory_add_physaddr_to_nid(kmem_start); > + dev_info(dev, "changing nid from %d to %d for DAX > region %pR\n", > + numa_node, new_node, res); > + numa_node = new_node; > } > > /* Hotplug starting at the beginning of the next block: */ > @@ -100,6 +102,7 @@ static int dev_dax_kmem_remove(struct device *dev) > resource_size_t kmem_start = res->start; > resource_size_t kmem_size = resource_size(res); > const char *res_name = res->name; > + int numa_node = dev_dax->target_node; > int rc; > > /* > @@ -108,7 +111,10 @@ static int dev_dax_kmem_remove(struct device *dev) >* there is no way to hotremove this memory until reboot because > device >* unbind will succeed even if we return failure. >*/ > - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); > + if (numa_node < 0) > + numa_node = memory_add_physaddr_to_nid(kmem_start); > + > + rc = remove_memory(numa_node, kmem_start, kmem_size); > if (rc) { > any_hotremove_failed = true; > dev_err(dev, > -- > 2.17.1
[RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is invalid
Previously, numa_off is set unconditionally at the end of dummy_numa_init(), even with a fake numa node. Then ACPI detects node id as NUMA_NO_NODE(-1) in acpi_map_pxm_to_node() because it regards numa_off as turning off the numa node. Hence dev_dax->target_node is NUMA_NO_NODE on arm64 with fake numa. Without this patch, pmem can't be probed as a RAM device on arm64 if SRAT table isn't present: $ndctl create-namespace -fe namespace0.0 --mode=devdax --map=dev -s 1g -a 64K kmem dax0.0: rejecting DAX region [mem 0x24040-0x2bfff] with invalid node: -1 kmem: probe of dax0.0 failed with error -22 This fixes it by using fallback memory_add_physaddr_to_nid() as nid. Suggested-by: David Hildenbrand Signed-off-by: Jia He --- I noticed that on powerpc memory_add_physaddr_to_nid is not exported for module driver. Set it to RFC due to this concern. drivers/dax/kmem.c | 22 ++ 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c index 275aa5f87399..68e693ca6d59 100644 --- a/drivers/dax/kmem.c +++ b/drivers/dax/kmem.c @@ -28,20 +28,22 @@ int dev_dax_kmem_probe(struct device *dev) resource_size_t kmem_end; struct resource *new_res; const char *new_res_name; - int numa_node; + int numa_node, new_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. +* Without this check, there is a risk but not fatal that slow +* memory could be mixed in a node with faster memory, causing +* unavoidable performance issues. Furthermore, fallback node +* id can be used when numa_node is invalid. */ 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; + new_node = memory_add_physaddr_to_nid(kmem_start); + dev_info(dev, "changing nid from %d to %d for DAX region %pR\n", + numa_node, new_node, res); + numa_node = new_node; } /* Hotplug starting at the beginning of the next block: */ @@ -100,6 +102,7 @@ static int dev_dax_kmem_remove(struct device *dev) resource_size_t kmem_start = res->start; resource_size_t kmem_size = resource_size(res); const char *res_name = res->name; + int numa_node = dev_dax->target_node; int rc; /* @@ -108,7 +111,10 @@ static int dev_dax_kmem_remove(struct device *dev) * there is no way to hotremove this memory until reboot because device * unbind will succeed even if we return failure. */ - rc = remove_memory(dev_dax->target_node, kmem_start, kmem_size); + if (numa_node < 0) + numa_node = memory_add_physaddr_to_nid(kmem_start); + + rc = remove_memory(numa_node, kmem_start, kmem_size); if (rc) { any_hotremove_failed = true; dev_err(dev, -- 2.17.1