Re: [PATCH v2 2/9] x86: numa: check the node id consistently for x86
From: Yunsheng Lin <hidden>
Date: 2019-09-03 06:19:55
Also in:
linux-alpha, linux-mips, linux-s390, linux-sh, lkml, sparclinux
On 2019/9/2 20:56, Peter Zijlstra wrote:
On Mon, Sep 02, 2019 at 08:25:24PM +0800, Yunsheng Lin wrote:quoted
On 2019/9/2 15:25, Peter Zijlstra wrote:quoted
On Mon, Sep 02, 2019 at 01:46:51PM +0800, Yunsheng Lin wrote:quoted
On 2019/9/1 0:12, Peter Zijlstra wrote:quoted
quoted
1) because even it is not set, the device really does belong to a node. It is impossible a device will have magic uniform access to memory when CPUs cannot.So it means dev_to_node() will return either NUMA_NO_NODE or a valid node id?NUMA_NO_NODE := -1, which is not a valid node number. It is also, like I said, not a valid device location on a NUMA system. Just because ACPI/BIOS is shit, doesn't mean the device doesn't have a node association. It just means we don't know and might have to guess.How do we guess the device's location when ACPI/BIOS does not set it?See device_add(), it looks to the device's parent and on NO_NODE, puts it there. Lacking any hints, just stick it to node0 and print a FW_BUG or something.quoted
It seems dev_to_node() does not do anything about that and leave the job to the caller or whatever function that get called with its return value, such as cpumask_of_node().Well, dev_to_node() doesn't do anything; nor should it. It are the callers of set_dev_node() that should be taking care. Also note how device_add() sets the device node to the parent device's node on NUMA_NO_NODE. Arguably we should change it to complain when it finds NUMA_NO_NODE and !parent.
Is it possible that the node id set by device_add() become invalid
if the node is offlined, then dev_to_node() may return a invalid
node id.
From the comment in select_fallback_rq(), it seems that a node can
be offlined, not sure if node offline process has taken cared of that?
/*
* If the node that the CPU is on has been offlined, cpu_to_node()
* will return -1. There is no CPU on the node, and we should
* select the CPU on the other node.
*/
With the above assumption that a device is always on a valid node,
the node id returned from dev_to_node() can be safely passed to
cpumask_of_node() without any checking?
quoted hunk ↗ jump to hunk
--- drivers/base/core.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)diff --git a/drivers/base/core.c b/drivers/base/core.c index f0dd8e38fee3..2caf204966a0 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c@@ -2120,8 +2120,16 @@ int device_add(struct device *dev) dev->kobj.parent = kobj; /* use parent numa_node */ - if (parent && (dev_to_node(dev) == NUMA_NO_NODE)) - set_dev_node(dev, dev_to_node(parent)); + if (dev_to_node(dev) == NUMA_NO_NODE) { + if (parent) + set_dev_node(dev, dev_to_node(parent)); +#ifdef CONFIG_NUMA + else { + pr_err("device: '%s': has no assigned NUMA node\n", dev_name(dev)); + set_dev_node(dev, 0); + } +#endif + } /* first, register with generic layer. */ /* we require the name to be set before, and pass NULL */.