Messages in this thread | | | Date | Fri, 30 Aug 2019 13:13:43 +0200 | From | Michal Hocko <> | Subject | Re: [PATCH] arm64: numa: check the node id before accessing node_to_cpumask_map |
| |
On Fri 30-08-19 17:49:46, Yunsheng Lin wrote: > On 2019/8/30 16:39, Michal Hocko wrote: > > On Fri 30-08-19 16:08:14, Yunsheng Lin wrote: [...] > >> It seems the cpumask_of_node with CONFIG_DEBUG_PER_CPU_MAPS is used > >> to catch the erorr case and give a warning to user when node id is not > >> valid. > > > > Yeah the config help text > > config DEBUG_PER_CPU_MAPS > > bool "Debug access to per_cpu maps" > > depends on DEBUG_KERNEL > > depends on SMP > > help > > Say Y to verify that the per_cpu map being accessed has > > been set up. This adds a fair amount of code to kernel memory > > and decreases performance. > > > > Say N if unsure. > > > > suggests that this is intentionally hidden behind a config so a normal > > path shouldn't really duplicate it. If those checks make sense in > > general then the config option should be dropped I think. > > It seems cpumask_of_node with CONFIG_DEBUG_PER_CPU_MAPS on may be used to > debug some early use of cpumask_of_node problem, see below: > > /* > * Allocate node_to_cpumask_map based on number of available nodes > * Requires node_possible_map to be valid. > * > * Note: cpumask_of_node() is not valid until after this is done. > * (Use CONFIG_DEBUG_PER_CPU_MAPS to check this.) > */ > static void __init setup_node_to_cpumask_map(void) > { > int node; > > /* setup nr_node_ids if not done yet */ > if (nr_node_ids == MAX_NUMNODES) > setup_nr_node_ids(); > > /* allocate and clear the mapping */ > for (node = 0; node < nr_node_ids; node++) { > alloc_bootmem_cpumask_var(&node_to_cpumask_map[node]); > cpumask_clear(node_to_cpumask_map[node]); > } > > /* cpumask_of_node() will now work */ > pr_debug("Node to cpumask map for %u nodes\n", nr_node_ids); > } > > So I prefer to keep it as two implementations for arm64 and x86, but > ensure the two implementations be consistent. It can be cleaned up later > when there is no use at all. > > Is it ok with you?
I am not really sure what you are asking here TBH. You want both CONFIG_DEBUG_PER_CPU_MAPS implementations to use the same (duplicated) code? If that is the case then no objections from me. I would obviously preferred a shared code but that might be a larger change indeed and can be done later.
-- Michal Hocko SUSE Labs
| |