This fixies a few issues when I tried to enable pmem as RAM device on arm64.
To use memory_add_physaddr_to_nid as a fallback nid, it would be better
implement a general version (__weak) in mm/memory_hotplug. After that, arm64/
sh/s390 can simply use the general version, and PowerPC/ia64/x86 will use
arch specific version.
Tested on ThunderX2 host/qemu "-M virt" guest with a nvdimm device. The
memblocks from the dax pmem device can be either hot-added or hot-removed
on arm64 guest. Also passed the compilation test on x86.
Changes:
v3: - introduce general version memory_add_physaddr_to_nid, refine the arch
specific one
- fix an uninitialization bug in v2 device-dax patch
v2: https://lkml.org/lkml/2020/7/7/71
- Drop unnecessary patch to harden try_offline_node
- Use new solution(by David) to fix dev->target_node=-1 during probing
- Refine the mem_hotplug_begin/done patch
v1: https://lkml.org/lkml/2020/7/5/381
Jia He (6):
mm/memory_hotplug: introduce default dummy
memory_add_physaddr_to_nid()
arm64/mm: use default dummy memory_add_physaddr_to_nid()
sh/mm: use default dummy memory_add_physaddr_to_nid()
mm: don't export memory_add_physaddr_to_nid in arch specific directory
device-dax: use fallback nid when numa_node is invalid
mm/memory_hotplug: fix unpaired mem_hotplug_begin/done
arch/arm64/mm/numa.c | 10 ----------
arch/ia64/mm/numa.c | 2 --
arch/sh/mm/init.c | 9 ---------
arch/x86/mm/numa.c | 1 -
drivers/dax/kmem.c | 21 +++++++++++++--------
mm/memory_hotplug.c | 15 ++++++++++++---
6 files changed, 25 insertions(+), 33 deletions(-)
--
2.17.1
This is to introduce a general dummy helper. memory_add_physaddr_to_nid()
is a fallback option to get the nid in case NUMA_NO_NID is detected.
After this patch, arm64/sh/s390 can simply use the general dummy version.
PowerPC/x86/ia64 will still use their specific version.
Signed-off-by: Jia He <[email protected]>
---
mm/memory_hotplug.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index da374cd3d45b..b49ab743d914 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -350,6 +350,16 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
return err;
}
+#ifdef CONFIG_NUMA
+int __weak memory_add_physaddr_to_nid(u64 start)
+{
+ pr_info_once("Unknown target node for memory at 0x%llx, assuming node 0\n",
+ start);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
+#endif
+
/* find the smallest valid pfn in the range [start_pfn, end_pfn) */
static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
unsigned long start_pfn,
--
2.17.1
After making default memory_add_physaddr_to_nid() in mm/memory_hotplug,
it is no use defining a similar one in arch specific directory.
Signed-off-by: Jia He <[email protected]>
---
arch/arm64/mm/numa.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
index aafcee3e3f7e..73f8b49d485c 100644
--- a/arch/arm64/mm/numa.c
+++ b/arch/arm64/mm/numa.c
@@ -461,13 +461,3 @@ void __init arm64_numa_init(void)
numa_init(dummy_numa_init);
}
-
-/*
- * We hope that we will be hotplugging memory on nodes we already know about,
- * such that acpi_get_node() succeeds and we never fall back to this...
- */
-int memory_add_physaddr_to_nid(u64 addr)
-{
- pr_warn("Unknown node for memory at 0x%llx, assuming node 0\n", addr);
- return 0;
-}
--
2.17.1
After making default memory_add_physaddr_to_nid in mm/memory_hotplug,
there is no use to define a similar one in arch specific directory.
Signed-off-by: Jia He <[email protected]>
---
arch/sh/mm/init.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index a70ba0fdd0b3..f75932ba87a6 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -430,15 +430,6 @@ int arch_add_memory(int nid, u64 start, u64 size,
return ret;
}
-#ifdef CONFIG_NUMA
-int memory_add_physaddr_to_nid(u64 addr)
-{
- /* Node 0 for now.. */
- return 0;
-}
-EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
-#endif
-
void arch_remove_memory(int nid, u64 start, u64 size,
struct vmem_altmap *altmap)
{
--
2.17.1
After a general version of __weak memory_add_physaddr_to_nid implemented
and exported , it is no use exporting twice in arch directory even if
e,g, ia64/x86 have their specific version.
This is to suppress the modpost warning:
WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
Previous export was in vmlinux
Suggested-by: David Hildenbrand <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
arch/ia64/mm/numa.c | 2 --
arch/x86/mm/numa.c | 1 -
2 files changed, 3 deletions(-)
diff --git a/arch/ia64/mm/numa.c b/arch/ia64/mm/numa.c
index 5e1015eb6d0d..f34964271101 100644
--- a/arch/ia64/mm/numa.c
+++ b/arch/ia64/mm/numa.c
@@ -106,7 +106,5 @@ int memory_add_physaddr_to_nid(u64 addr)
return 0;
return nid;
}
-
-EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
#endif
#endif
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index 8ee952038c80..2a6e62af4636 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -929,5 +929,4 @@ int memory_add_physaddr_to_nid(u64 start)
nid = numa_meminfo.blk[0].nid;
return nid;
}
-EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
#endif
--
2.17.1
numa_off is set unconditionally at the end of dummy_numa_init(),
even with a fake numa node. 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 0x240400000-0x2bfffffff] 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 <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
drivers/dax/kmem.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
index 275aa5f87399..218f66057994 100644
--- a/drivers/dax/kmem.c
+++ b/drivers/dax/kmem.c
@@ -31,22 +31,23 @@ int dev_dax_kmem_probe(struct device *dev)
int numa_node;
int rc;
+ /* Hotplug starting at the beginning of the next block: */
+ kmem_start = ALIGN(res->start, memory_block_size_bytes());
+
/*
* 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.
+ * 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;
+ numa_node = memory_add_physaddr_to_nid(kmem_start);
+ dev_info(dev, "using nid %d for DAX region with undefined nid %pR\n",
+ numa_node, res);
}
- /* 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;
@@ -100,15 +101,19 @@ 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;
+ if (numa_node < 0)
+ numa_node = memory_add_physaddr_to_nid(kmem_start);
+
/*
* We have one shot for removing memory, if some memory blocks were not
* offline prior to calling this function remove_memory() will fail, and
* 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);
+ rc = remove_memory(numa_node, kmem_start, kmem_size);
if (rc) {
any_hotremove_failed = true;
dev_err(dev,
--
2.17.1
When check_memblock_offlined_cb() returns failed rc(e.g. the memblock is
online at that time), mem_hotplug_begin/done is unpaired in such case.
Therefore a warning:
Call Trace:
percpu_up_write+0x33/0x40
try_remove_memory+0x66/0x120
? _cond_resched+0x19/0x30
remove_memory+0x2b/0x40
dev_dax_kmem_remove+0x36/0x72 [kmem]
device_release_driver_internal+0xf0/0x1c0
device_release_driver+0x12/0x20
bus_remove_device+0xe1/0x150
device_del+0x17b/0x3e0
unregister_dev_dax+0x29/0x60
devm_action_release+0x15/0x20
release_nodes+0x19a/0x1e0
devres_release_all+0x3f/0x50
device_release_driver_internal+0x100/0x1c0
driver_detach+0x4c/0x8f
bus_remove_driver+0x5c/0xd0
driver_unregister+0x31/0x50
dax_pmem_exit+0x10/0xfe0 [dax_pmem]
Fixes: f1037ec0cc8a ("mm/memory_hotplug: fix remove_memory() lockdep splat")
Cc: [email protected] # v5.6+
Signed-off-by: Jia He <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Acked-by: Dan Williams <[email protected]>
---
mm/memory_hotplug.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b49ab743d914..3e0645387daf 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1752,7 +1752,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
*/
rc = walk_memory_blocks(start, size, NULL, check_memblock_offlined_cb);
if (rc)
- goto done;
+ return rc;
/* remove memmap entry */
firmware_map_remove(start, start + size, "System RAM");
@@ -1776,9 +1776,8 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
try_offline_node(nid);
-done:
mem_hotplug_done();
- return rc;
+ return 0;
}
/**
--
2.17.1
On Thu, Jul 09, 2020 at 10:06:27AM +0800, Jia He wrote:
> After a general version of __weak memory_add_physaddr_to_nid implemented
> and exported , it is no use exporting twice in arch directory even if
> e,g, ia64/x86 have their specific version.
>
> This is to suppress the modpost warning:
> WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
> Previous export was in vmlinux
It's bad form to introduce a warning and then send a follow-up patch to
fix the warning. Just fold this patch into patch 1/6.
Hi Matthew
> -----Original Message-----
> From: Matthew Wilcox <[email protected]>
> Sent: Thursday, July 9, 2020 10:11 AM
> To: Justin He <[email protected]>
> Cc: Catalin Marinas <[email protected]>; Will Deacon
> <[email protected]>; Tony Luck <[email protected]>; Fenghua Yu
> <[email protected]>; Yoshinori Sato <[email protected]>; Rich
> Felker <[email protected]>; Dave Hansen <[email protected]>; Andy
> Lutomirski <[email protected]>; Peter Zijlstra <[email protected]>;
> Thomas Gleixner <[email protected]>; Ingo Molnar <[email protected]>;
> Borislav Petkov <[email protected]>; David Hildenbrand <[email protected]>;
> [email protected]; H. Peter Anvin <[email protected]>; Dan Williams
> <[email protected]>; Vishal Verma <[email protected]>; Dave
> Jiang <[email protected]>; Andrew Morton <[email protected]>;
> Baoquan He <[email protected]>; Chuhong Yuan <[email protected]>; Mike
> Rapoport <[email protected]>; Logan Gunthorpe <[email protected]>;
> Masahiro Yamada <[email protected]>; Michal Hocko <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; Jonathan Cameron <[email protected]>; Kaly
> Xin <[email protected]>
> Subject: Re: [PATCH v3 4/6] mm: don't export memory_add_physaddr_to_nid in
> arch specific directory
>
> On Thu, Jul 09, 2020 at 10:06:27AM +0800, Jia He wrote:
> > After a general version of __weak memory_add_physaddr_to_nid implemented
> > and exported , it is no use exporting twice in arch directory even if
> > e,g, ia64/x86 have their specific version.
> >
> > This is to suppress the modpost warning:
> > WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
> > Previous export was in vmlinux
>
> It's bad form to introduce a warning and then send a follow-up patch to
> fix the warning. Just fold this patch into patch 1/6.
Thanks, will do
Cheers,
Justin He
On Wed, Jul 8, 2020 at 7:07 PM Jia He <[email protected]> wrote:
>
> numa_off is set unconditionally at the end of dummy_numa_init(),
> even with a fake numa node. 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 0x240400000-0x2bfffffff] 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 <[email protected]>
> Signed-off-by: Jia He <[email protected]>
> ---
> drivers/dax/kmem.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> index 275aa5f87399..218f66057994 100644
> --- a/drivers/dax/kmem.c
> +++ b/drivers/dax/kmem.c
> @@ -31,22 +31,23 @@ int dev_dax_kmem_probe(struct device *dev)
> int numa_node;
> int rc;
>
> + /* Hotplug starting at the beginning of the next block: */
> + kmem_start = ALIGN(res->start, memory_block_size_bytes());
> +
> /*
> * 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.
> + * 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;
> + numa_node = memory_add_physaddr_to_nid(kmem_start);
I think this fixup belongs to the core to set a fallback value for
dev_dax->target_node.
I'm close to having patches to provide a functional
phys_addr_to_target_node() for arm64.
Hi Dan
> -----Original Message-----
> From: Dan Williams <[email protected]>
> Sent: Thursday, July 9, 2020 11:39 AM
> To: Justin He <[email protected]>
> Cc: Catalin Marinas <[email protected]>; Will Deacon
> <[email protected]>; Tony Luck <[email protected]>; Fenghua Yu
> <[email protected]>; Yoshinori Sato <[email protected]>; Rich
> Felker <[email protected]>; Dave Hansen <[email protected]>; Andy
> Lutomirski <[email protected]>; Peter Zijlstra <[email protected]>;
> Thomas Gleixner <[email protected]>; Ingo Molnar <[email protected]>;
> Borislav Petkov <[email protected]>; David Hildenbrand <[email protected]>; X86
> ML <[email protected]>; H. Peter Anvin <[email protected]>; Vishal Verma
> <[email protected]>; Dave Jiang <[email protected]>; Andrew
> Morton <[email protected]>; Baoquan He <[email protected]>; Chuhong
> Yuan <[email protected]>; Mike Rapoport <[email protected]>; Logan
> Gunthorpe <[email protected]>; Masahiro Yamada <[email protected]>;
> Michal Hocko <[email protected]>; Linux ARM <linux-arm-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; [email protected]; Linux-sh <linux-
> [email protected]>; linux-nvdimm <[email protected]>; Linux MM
> <[email protected]>; Jonathan Cameron <[email protected]>; Kaly
> Xin <[email protected]>
> Subject: Re: [PATCH v3 5/6] device-dax: use fallback nid when numa_node is
> invalid
>
> On Wed, Jul 8, 2020 at 7:07 PM Jia He <[email protected]> wrote:
> >
> > numa_off is set unconditionally at the end of dummy_numa_init(),
> > even with a fake numa node. 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 0x240400000-0x2bfffffff] 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 <[email protected]>
> > Signed-off-by: Jia He <[email protected]>
> > ---
> > drivers/dax/kmem.c | 21 +++++++++++++--------
> > 1 file changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c
> > index 275aa5f87399..218f66057994 100644
> > --- a/drivers/dax/kmem.c
> > +++ b/drivers/dax/kmem.c
> > @@ -31,22 +31,23 @@ int dev_dax_kmem_probe(struct device *dev)
> > int numa_node;
> > int rc;
> >
> > + /* Hotplug starting at the beginning of the next block: */
> > + kmem_start = ALIGN(res->start, memory_block_size_bytes());
> > +
> > /*
> > * 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.
> > + * 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;
> > + numa_node = memory_add_physaddr_to_nid(kmem_start);
>
> I think this fixup belongs to the core to set a fallback value for
> dev_dax->target_node.
>
> I'm close to having patches to provide a functional
> phys_addr_to_target_node() for arm64.
Should My this patch(5/6) wait on your new phys_addr_to_target_node() patch?
Thanks for the clarification.
--
Cheers,
Justin (Jia He)
On 09.07.20 04:06, Jia He wrote:
> This is to introduce a general dummy helper. memory_add_physaddr_to_nid()
> is a fallback option to get the nid in case NUMA_NO_NID is detected.
>
> After this patch, arm64/sh/s390 can simply use the general dummy version.
> PowerPC/x86/ia64 will still use their specific version.
>
> Signed-off-by: Jia He <[email protected]>
> ---
> mm/memory_hotplug.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index da374cd3d45b..b49ab743d914 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -350,6 +350,16 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> return err;
> }
>
> +#ifdef CONFIG_NUMA
> +int __weak memory_add_physaddr_to_nid(u64 start)
> +{
> + pr_info_once("Unknown target node for memory at 0x%llx, assuming node 0\n",
> + start);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
> +#endif
> +
> /* find the smallest valid pfn in the range [start_pfn, end_pfn) */
> static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
> unsigned long start_pfn,
>
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 09.07.20 04:06, Jia He wrote:
> After making default memory_add_physaddr_to_nid() in mm/memory_hotplug,
> it is no use defining a similar one in arch specific directory.
>
> Signed-off-by: Jia He <[email protected]>
> ---
> arch/arm64/mm/numa.c | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index aafcee3e3f7e..73f8b49d485c 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -461,13 +461,3 @@ void __init arm64_numa_init(void)
>
> numa_init(dummy_numa_init);
> }
> -
> -/*
> - * We hope that we will be hotplugging memory on nodes we already know about,
> - * such that acpi_get_node() succeeds and we never fall back to this...
> - */
> -int memory_add_physaddr_to_nid(u64 addr)
> -{
> - pr_warn("Unknown node for memory at 0x%llx, assuming node 0\n", addr);
> - return 0;
> -}
>
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 09.07.20 04:06, Jia He wrote:
> After making default memory_add_physaddr_to_nid in mm/memory_hotplug,
> there is no use to define a similar one in arch specific directory.
>
> Signed-off-by: Jia He <[email protected]>
> ---
> arch/sh/mm/init.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index a70ba0fdd0b3..f75932ba87a6 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -430,15 +430,6 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -#ifdef CONFIG_NUMA
> -int memory_add_physaddr_to_nid(u64 addr)
> -{
> - /* Node 0 for now.. */
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
> -#endif
> -
> void arch_remove_memory(int nid, u64 start, u64 size,
> struct vmem_altmap *altmap)
> {
>
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 09.07.20 04:06, Jia He wrote:
> When check_memblock_offlined_cb() returns failed rc(e.g. the memblock is
> online at that time), mem_hotplug_begin/done is unpaired in such case.
>
> Therefore a warning:
> Call Trace:
> percpu_up_write+0x33/0x40
> try_remove_memory+0x66/0x120
> ? _cond_resched+0x19/0x30
> remove_memory+0x2b/0x40
> dev_dax_kmem_remove+0x36/0x72 [kmem]
> device_release_driver_internal+0xf0/0x1c0
> device_release_driver+0x12/0x20
> bus_remove_device+0xe1/0x150
> device_del+0x17b/0x3e0
> unregister_dev_dax+0x29/0x60
> devm_action_release+0x15/0x20
> release_nodes+0x19a/0x1e0
> devres_release_all+0x3f/0x50
> device_release_driver_internal+0x100/0x1c0
> driver_detach+0x4c/0x8f
> bus_remove_driver+0x5c/0xd0
> driver_unregister+0x31/0x50
> dax_pmem_exit+0x10/0xfe0 [dax_pmem]
>
> Fixes: f1037ec0cc8a ("mm/memory_hotplug: fix remove_memory() lockdep splat")
> Cc: [email protected] # v5.6+
> Signed-off-by: Jia He <[email protected]>
> Reviewed-by: David Hildenbrand <[email protected]>
> Acked-by: Michal Hocko <[email protected]>
> Acked-by: Dan Williams <[email protected]>
> ---
> mm/memory_hotplug.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index b49ab743d914..3e0645387daf 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1752,7 +1752,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
> */
> rc = walk_memory_blocks(start, size, NULL, check_memblock_offlined_cb);
> if (rc)
> - goto done;
> + return rc;
>
> /* remove memmap entry */
> firmware_map_remove(start, start + size, "System RAM");
> @@ -1776,9 +1776,8 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
>
> try_offline_node(nid);
>
> -done:
> mem_hotplug_done();
> - return rc;
> + return 0;
> }
>
> /**
>
@Andrew, can you pick that up directly? It's independent of the other stuff.
--
Thanks,
David / dhildenb
On Thu, Jul 09, 2020 at 03:11:04AM +0100, Matthew Wilcox wrote:
> On Thu, Jul 09, 2020 at 10:06:27AM +0800, Jia He wrote:
> > After a general version of __weak memory_add_physaddr_to_nid implemented
> > and exported , it is no use exporting twice in arch directory even if
> > e,g, ia64/x86 have their specific version.
> >
> > This is to suppress the modpost warning:
> > WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
> > Previous export was in vmlinux
>
> It's bad form to introduce a warning and then send a follow-up patch to
> fix the warning. Just fold this patch into patch 1/6.
Moreover, I think that patches 1-4 can be merged into one.
--
Sincerely yours,
Mike.
On 09.07.20 11:18, Mike Rapoport wrote:
> On Thu, Jul 09, 2020 at 03:11:04AM +0100, Matthew Wilcox wrote:
>> On Thu, Jul 09, 2020 at 10:06:27AM +0800, Jia He wrote:
>>> After a general version of __weak memory_add_physaddr_to_nid implemented
>>> and exported , it is no use exporting twice in arch directory even if
>>> e,g, ia64/x86 have their specific version.
>>>
>>> This is to suppress the modpost warning:
>>> WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
>>> Previous export was in vmlinux
>>
>> It's bad form to introduce a warning and then send a follow-up patch to
>> fix the warning. Just fold this patch into patch 1/6.
>
> Moreover, I think that patches 1-4 can be merged into one.
>
+1
--
Thanks,
David / dhildenb
Hi David
> -----Original Message-----
> From: David Hildenbrand <[email protected]>
> Sent: Thursday, July 9, 2020 5:19 PM
> To: Mike Rapoport <[email protected]>; Matthew Wilcox
> <[email protected]>
> Cc: Justin He <[email protected]>; Catalin Marinas
> <[email protected]>; Will Deacon <[email protected]>; Tony Luck
> <[email protected]>; Fenghua Yu <[email protected]>; Yoshinori Sato
> <[email protected]>; Rich Felker <[email protected]>; Dave Hansen
> <[email protected]>; Andy Lutomirski <[email protected]>; Peter
> Zijlstra <[email protected]>; Thomas Gleixner <[email protected]>;
> Ingo Molnar <[email protected]>; Borislav Petkov <[email protected]>;
> [email protected]; H. Peter Anvin <[email protected]>; Dan Williams
> <[email protected]>; Vishal Verma <[email protected]>; Dave
> Jiang <[email protected]>; Andrew Morton <[email protected]>;
> Baoquan He <[email protected]>; Chuhong Yuan <[email protected]>; Logan
> Gunthorpe <[email protected]>; Masahiro Yamada <[email protected]>;
> Michal Hocko <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> Jonathan Cameron <[email protected]>; Kaly Xin <[email protected]>
> Subject: Re: [PATCH v3 4/6] mm: don't export memory_add_physaddr_to_nid in
> arch specific directory
>
> On 09.07.20 11:18, Mike Rapoport wrote:
> > On Thu, Jul 09, 2020 at 03:11:04AM +0100, Matthew Wilcox wrote:
> >> On Thu, Jul 09, 2020 at 10:06:27AM +0800, Jia He wrote:
> >>> After a general version of __weak memory_add_physaddr_to_nid
> implemented
> >>> and exported , it is no use exporting twice in arch directory even if
> >>> e,g, ia64/x86 have their specific version.
> >>>
> >>> This is to suppress the modpost warning:
> >>> WARNING: modpost: vmlinux: 'memory_add_physaddr_to_nid' exported twice.
> >>> Previous export was in vmlinux
> >>
> >> It's bad form to introduce a warning and then send a follow-up patch to
> >> fix the warning. Just fold this patch into patch 1/6.
> >
> > Moreover, I think that patches 1-4 can be merged into one.
> >
>
> +1
Okay, will update, thanks
--
Cheers,
Justin (Jia He)
On Thu, Jul 09, 2020 at 11:10:36AM +0200, David Hildenbrand wrote:
> On 09.07.20 04:06, Jia He wrote:
> > After making default memory_add_physaddr_to_nid in mm/memory_hotplug,
> > there is no use to define a similar one in arch specific directory.
> >
> > Signed-off-by: Jia He <[email protected]>
> > ---
> > arch/sh/mm/init.c | 9 ---------
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> > index a70ba0fdd0b3..f75932ba87a6 100644
> > --- a/arch/sh/mm/init.c
> > +++ b/arch/sh/mm/init.c
> > @@ -430,15 +430,6 @@ int arch_add_memory(int nid, u64 start, u64 size,
> > return ret;
> > }
> >
> > -#ifdef CONFIG_NUMA
> > -int memory_add_physaddr_to_nid(u64 addr)
> > -{
> > - /* Node 0 for now.. */
> > - return 0;
> > -}
> > -EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
> > -#endif
> > -
> > void arch_remove_memory(int nid, u64 start, u64 size,
> > struct vmem_altmap *altmap)
> > {
> >
>
> Reviewed-by: David Hildenbrand <[email protected]>
Acked-by: Rich Felker <[email protected]>
> After making default memory_add_physaddr_to_nid in mm/memory_hotplug,
> there is no use to define a similar one in arch specific directory.
>
> Signed-off-by: Jia He <[email protected]>
> ---
> arch/sh/mm/init.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index a70ba0fdd0b3..f75932ba87a6 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -430,15 +430,6 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -#ifdef CONFIG_NUMA
> -int memory_add_physaddr_to_nid(u64 addr)
> -{
> - /* Node 0 for now.. */
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
> -#endif
> -
> void arch_remove_memory(int nid, u64 start, u64 size,
> struct vmem_altmap *altmap)
> {
Reviewed-by: Pankaj Gupta <[email protected]>
On Wed, Jul 8, 2020 at 7:06 PM Jia He <[email protected]> wrote:
>
> This fixies a few issues when I tried to enable pmem as RAM device on arm64.
What NVDIMM bus driver is being used in this case? The ACPI NFIT
driver? I'm just looking to see if currently deployed
phys_to_target_node() is sufficient, or if this is coming in a new
driver?