On Fri 30-08-19 14:35:26, Yunsheng Lin wrote:
> On 2019/8/30 13:55, Michal Hocko wrote:
> > On Fri 30-08-19 10:26:31, Yunsheng Lin wrote:
> >> Some buggy bios may not set the device' numa id, and dev_to_node
> >> will return -1, which may cause global-out-of-bounds error
> >> detected by KASAN.
> >
> > Why should we workaround a buggy bios like that? Is it so widespread and
> > no BIOS update available? Also, why is this arm64 specific?
>
> For our case, there is BIOS update available. I just thought it might
> be better to protect from this case when BIOS has not implemented the
> device' numa id setting feature or the feature from BIOS has some bug.
>
> It is not arm64 specific, right now I only have arm64 board. If it is
> ok to protect this from the buggy BIOS, maybe all other arch can be
> changed too.
If we are to really care then this should be consistent among
architectures IMHO. But I am not really sure this is really worth it.
The code is quite old and I do not really remember any reports.
> >> This patch changes cpumask_of_node to return cpu_none_mask if the
> >> node is not valid, and sync the cpumask_of_node between the
> >> cpumask_of_node function in numa.h and numa.c.
> >
> > Why?
>
> When CONFIG_DEBUG_PER_CPU_MAPS is defined, the cpumask_of_node() in
> numa.c is used, if not, the cpumask_of_node() in numa.h is used.
>
> I am not sure why there is difference between them, and it is there
> when since the below commit:
> 1a2db300348b ("arm64, numa: Add NUMA support for arm64 platforms.")
>
> I synced them to keep them consistent whether CONFIG_DEBUG_PER_CPU_MAPS
> is defined.
Such a change should be made in a separate patch with a full
clarification/justification. From the above it is still not clear why
this is needed though.
> >> Signed-off-by: Yunsheng Lin <linyunsh...@huawei.com>
> >> ---
> >> arch/arm64/include/asm/numa.h | 6 ++++++
> >> arch/arm64/mm/numa.c | 2 +-
> >> 2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
> >> index 626ad01..da891ed 100644
> >> --- a/arch/arm64/include/asm/numa.h
> >> +++ b/arch/arm64/include/asm/numa.h
> >> @@ -25,6 +25,12 @@ const struct cpumask *cpumask_of_node(int node);
> >> /* Returns a pointer to the cpumask of CPUs on Node 'node'. */
> >> static inline const struct cpumask *cpumask_of_node(int node)
> >> {
> >> + if (node >= nr_node_ids || node < 0)
> >> + return cpu_none_mask;
> >> +
> >> + if (!node_to_cpumask_map[node])
> >> + return cpu_online_mask;
> >> +
> >> return node_to_cpumask_map[node];
> >> }
> >> #endif
> >> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> >> index 4f241cc..3846313 100644
> >> --- a/arch/arm64/mm/numa.c
> >> +++ b/arch/arm64/mm/numa.c
> >> @@ -46,7 +46,7 @@ EXPORT_SYMBOL(node_to_cpumask_map);
> >> */
> >> const struct cpumask *cpumask_of_node(int node)
> >> {
> >> - if (WARN_ON(node >= nr_node_ids))
> >> + if (WARN_ON(node >= nr_node_ids || node < 0))
> >> return cpu_none_mask;
> >>
> >> if (WARN_ON(node_to_cpumask_map[node] == NULL))
> >> --
> >> 2.8.1
> >
--
Michal Hocko
SUSE Labs
