| Message ID | 1567231103-13237-8-git-send-email-linyunsheng@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | check the node id consistently across different arches | expand |
From: Yunsheng Lin <linyunsheng@huawei.com> Date: Sat, 31 Aug 2019 13:58:21 +0800 > According to Section 6.2.14 from ACPI spec 6.3 [1], the setting > of proximity domain is optional, as below: What in the world does the ACPI spec have to do with sparc64 NUMA node ID checking?
On 2019/8/31 14:53, David Miller wrote: > From: Yunsheng Lin <linyunsheng@huawei.com> > Date: Sat, 31 Aug 2019 13:58:21 +0800 > >> According to Section 6.2.14 from ACPI spec 6.3 [1], the setting >> of proximity domain is optional, as below: > > What in the world does the ACPI spec have to do with sparc64 NUMA > node ID checking? I am not sure I understand your question fully here. Here is my issue when the bios does not implement the proximity domain of a device because the feature is optional according to the ACPI spec, the dev_to_node(dev) return -1, which causes out of bound access when using the value to get the device's cpu mask by calling cpumask_of_node. Did you mean sparc64 system does not has ACPI, the device's node id will not specified by ACPI, so the ACPI is unrelated here? Or did you mean the commit log is not clear enough to justify the change? Or did you mean this problem should be fixed in somewhere else? Any detail advice and suggestion will be very helpful, thanks. > > . >
From: Yunsheng Lin <linyunsheng@huawei.com> Date: Sat, 31 Aug 2019 16:57:04 +0800 > Did you mean sparc64 system does not has ACPI, the device's node id will > not specified by ACPI, so the ACPI is unrelated here? Yes, sparc64 never has and never will have ACPI. This is also true for several other platforms where you have made this change. The assumption of your entire patch set is that the semantics of the NUMA node ID are somehow completely defined by ACPI semantics. Which is not true.
On 2019/9/1 4:02, David Miller wrote: > From: Yunsheng Lin <linyunsheng@huawei.com> > Date: Sat, 31 Aug 2019 16:57:04 +0800 > >> Did you mean sparc64 system does not has ACPI, the device's node id will >> not specified by ACPI, so the ACPI is unrelated here? > > Yes, sparc64 never has and never will have ACPI. > > This is also true for several other platforms where you have made this > change. > > The assumption of your entire patch set is that the semantics of the > NUMA node ID are somehow completely defined by ACPI semantics. Which > is not true. Thanks for pointing out. The NUMA node id in sparc64 system is defined by DT semantics? > > . >
From: Yunsheng Lin <linyunsheng@huawei.com> Date: Mon, 2 Sep 2019 14:08:31 +0800 > The NUMA node id in sparc64 system is defined by DT semantics? Sometimes, and in other cases other methods are used to determine the NUMA node id.
diff --git a/arch/sparc/include/asm/topology_64.h b/arch/sparc/include/asm/topology_64.h index 34c628a..66a7917 100644 --- a/arch/sparc/include/asm/topology_64.h +++ b/arch/sparc/include/asm/topology_64.h @@ -11,9 +11,19 @@ static inline int cpu_to_node(int cpu) return numa_cpu_lookup_table[cpu]; } -#define cpumask_of_node(node) ((node) == -1 ? \ - cpu_all_mask : \ - &numa_cpumask_lookup_table[node]) +static inline const struct cpumask *cpumask_of_node(int node) +{ + if (node >= MAX_NUMNODES) + return cpu_none_mask; + + /* numa_cpumask_lookup_table[node] is not a pointer, so + * no need to check for NULL here. + */ + if (node < 0) + return cpu_online_mask; + + return &numa_cpumask_lookup_table[node]; +} struct pci_bus; #ifdef CONFIG_PCI
According to Section 6.2.14 from ACPI spec 6.3 [1], the setting of proximity domain is optional, as below: This optional object is used to describe proximity domain associations within a machine. _PXM evaluates to an integer that identifies a device as belonging to a Proximity Domain defined in the System Resource Affinity Table (SRAT). This patch checks node id with the below case before returning &numa_cpumask_lookup_table[node]: 1. if node_id >= nr_node_ids, return cpu_none_mask 2. if node_id < 0, return cpu_online_mask 3. Since numa_cpumask_lookup_table is not a pointer, a comment is added to indicate that [1] https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com> --- arch/sparc/include/asm/topology_64.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)