| Message ID | 1551011649-30103-4-git-send-email-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | make memblock allocator utilize the node's fallback info | expand |
On 2/24/19 4:34 AM, Pingfan Liu wrote: > +#ifdef CONFIG_NUMA > /* > * There are unfortunately some poorly designed mainboards around that > * only connect memory to a single CPU. This breaks the 1:1 cpu->node > @@ -618,6 +619,9 @@ static void __init numa_init_array(void) > rr = next_node_in(rr, node_online_map); > } > } > +#else > +static void __init numa_init_array(void) {} > +#endif What functional effect does this #ifdef have? Let's look at the code: > static void __init numa_init_array(void) > { > int rr, i; > > rr = first_node(node_online_map); > for (i = 0; i < nr_cpu_ids; i++) { > if (early_cpu_to_node(i) != NUMA_NO_NODE) > continue; > numa_set_node(i, rr); > rr = next_node_in(rr, node_online_map); > } > } and "play compiler" for a bit. The first iteration will see early_cpu_to_node(i)==1 because: static inline int early_cpu_to_node(int cpu) { return 0; } if CONFIG_NUMA=n. In other words, I'm not sure this patch does *anything*.
On Mon, Feb 25, 2019 at 11:24 PM Dave Hansen <dave.hansen@intel.com> wrote: > > On 2/24/19 4:34 AM, Pingfan Liu wrote: > > +#ifdef CONFIG_NUMA > > /* > > * There are unfortunately some poorly designed mainboards around that > > * only connect memory to a single CPU. This breaks the 1:1 cpu->node > > @@ -618,6 +619,9 @@ static void __init numa_init_array(void) > > rr = next_node_in(rr, node_online_map); > > } > > } > > +#else > > +static void __init numa_init_array(void) {} > > +#endif > > What functional effect does this #ifdef have? > > Let's look at the code: > > > static void __init numa_init_array(void) > > { > > int rr, i; > > > > rr = first_node(node_online_map); > > for (i = 0; i < nr_cpu_ids; i++) { > > if (early_cpu_to_node(i) != NUMA_NO_NODE) > > continue; > > numa_set_node(i, rr); > > rr = next_node_in(rr, node_online_map); > > } > > } > > and "play compiler" for a bit. > > The first iteration will see early_cpu_to_node(i)==1 because: > > static inline int early_cpu_to_node(int cpu) > { > return 0; > } > > if CONFIG_NUMA=n. > > In other words, I'm not sure this patch does *anything*. I had thought separating [3/6] and [4/6] can ease the review. And I will merge them in next version. Thanks and regards, Pingfan
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c index 1308f54..bfe6732 100644 --- a/arch/x86/mm/numa.c +++ b/arch/x86/mm/numa.c @@ -599,6 +599,7 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) return 0; } +#ifdef CONFIG_NUMA /* * There are unfortunately some poorly designed mainboards around that * only connect memory to a single CPU. This breaks the 1:1 cpu->node @@ -618,6 +619,9 @@ static void __init numa_init_array(void) rr = next_node_in(rr, node_online_map); } } +#else +static void __init numa_init_array(void) {} +#endif static int __init numa_init(int (*init_func)(void)) {
For non-NUMA, it turns out that numa_init_array() has no operations. Make separated definition for non-NUMA and NUMA, so later they can be combined into their counterpart init_cpu_to_node(). Signed-off-by: Pingfan Liu <kernelfans@gmail.com> CC: Thomas Gleixner <tglx@linutronix.de> CC: Ingo Molnar <mingo@redhat.com> CC: Borislav Petkov <bp@alien8.de> CC: "H. Peter Anvin" <hpa@zytor.com> CC: Dave Hansen <dave.hansen@linux.intel.com> CC: Vlastimil Babka <vbabka@suse.cz> CC: Mike Rapoport <rppt@linux.vnet.ibm.com> CC: Andrew Morton <akpm@linux-foundation.org> CC: Mel Gorman <mgorman@suse.de> CC: Joonsoo Kim <iamjoonsoo.kim@lge.com> CC: Andy Lutomirski <luto@kernel.org> CC: Andi Kleen <ak@linux.intel.com> CC: Petr Tesarik <ptesarik@suse.cz> CC: Michal Hocko <mhocko@suse.com> CC: Stephen Rothwell <sfr@canb.auug.org.au> CC: Jonathan Corbet <corbet@lwn.net> CC: Nicholas Piggin <npiggin@gmail.com> CC: Daniel Vacek <neelx@redhat.com> CC: linux-kernel@vger.kernel.org --- arch/x86/mm/numa.c | 4 ++++ 1 file changed, 4 insertions(+)