lkml.org 
[lkml]   [2020]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH v2 2/3] device-dax: use fallback nid when numa_node is invalid
    From
    Date
    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 <david@redhat.com>
    > Signed-off-by: Jia He <justin.he@arm.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2020-07-07 13:35    [W:5.866 / U:0.148 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site