Hi David > -----Original Message----- > From: David Hildenbrand <[email protected]> > Sent: Tuesday, July 7, 2020 7:34 PM > To: Justin He <[email protected]>; Catalin Marinas > <[email protected]>; Will Deacon <[email protected]>; Dan Williams > <[email protected]>; Vishal Verma <[email protected]>; Dave > Jiang <[email protected]> > Cc: Michal Hocko <[email protected]>; Andrew Morton <akpm@linux- > foundation.org>; Mike Rapoport <[email protected]>; Baoquan He > <[email protected]>; Chuhong Yuan <[email protected]>; linux-arm- > [email protected]; [email protected]; linux- > [email protected]; [email protected]; Kaly Xin <[email protected]> > 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 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]> > > --- > > 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)
