* Re: [tip:x86/mm] x86-64, NUMA: Restructure initmem_init() [not found] <tip-ffe77a4605fb2588f8666850ad3e3b196241658f@git.kernel.org> @ 2011-02-20 3:34 ` David Rientjes 2011-02-21 8:35 ` Tejun Heo 0 siblings, 1 reply; 13+ messages in thread From: David Rientjes @ 2011-02-20 3:34 UTC (permalink / raw) To: Ingo Molnar, H. Peter Anvin, tglx, Tejun Heo Cc: yinghai, brgerst, gorcunov, shaohui.zheng, linux-kernel On Wed, 16 Feb 2011, tip-bot for Tejun Heo wrote: > diff --git a/arch/x86/mm/numa_64.c b/arch/x86/mm/numa_64.c > index 656b0cf..c984e34 100644 > --- a/arch/x86/mm/numa_64.c > +++ b/arch/x86/mm/numa_64.c > @@ -580,65 +580,73 @@ static int __init numa_emulation(unsigned long start_pfn, > } > #endif /* CONFIG_NUMA_EMU */ > > -void __init initmem_init(void) > +static int dummy_numa_init(void) > { > - int acpi = 0, amd = 0; > - int i; > - > -#ifdef CONFIG_ACPI_NUMA > - /* > - * Parse SRAT to discover nodes. > - */ > - acpi = !x86_acpi_numa_init(); > -#endif > - > -#ifdef CONFIG_AMD_NUMA > - if (!acpi) > - amd = !amd_numa_init(); > -#endif > - > - nodes_clear(node_possible_map); > - nodes_clear(node_online_map); > - > -#ifdef CONFIG_NUMA_EMU > - setup_physnodes(0, max_pfn << PAGE_SHIFT, acpi, amd); > - if (cmdline && !numa_emulation(0, max_pfn, acpi, amd)) > - return; > - setup_physnodes(0, max_pfn << PAGE_SHIFT, acpi, amd); > - nodes_clear(node_possible_map); > - nodes_clear(node_online_map); > -#endif > - > -#ifdef CONFIG_ACPI_NUMA > - if (!numa_off && acpi && !acpi_scan_nodes()) > - return; > - nodes_clear(node_possible_map); > - nodes_clear(node_online_map); > -#endif > + return 0; > +} > > -#ifdef CONFIG_AMD_NUMA > - if (!numa_off && amd && !amd_scan_nodes()) > - return; > - nodes_clear(node_possible_map); > - nodes_clear(node_online_map); > -#endif > +static int dummy_scan_nodes(void) > +{ > printk(KERN_INFO "%s\n", > numa_off ? "NUMA turned off" : "No NUMA configuration found"); > - > printk(KERN_INFO "Faking a node at %016lx-%016lx\n", > 0LU, max_pfn << PAGE_SHIFT); > + > /* setup dummy node covering all memory */ > memnode_shift = 63; > memnodemap = memnode.embedded_map; > memnodemap[0] = 0; > node_set_online(0); > node_set(0, node_possible_map); > - for (i = 0; i < MAX_LOCAL_APIC; i++) > - set_apicid_to_node(i, NUMA_NO_NODE); > memblock_x86_register_active_regions(0, 0, max_pfn); > init_memory_mapping_high(); > setup_node_bootmem(0, 0, max_pfn << PAGE_SHIFT); > numa_init_array(); > + > + return 0; > +} > + > +void __init initmem_init(void) > +{ > + int (*numa_init[])(void) = { [2] = dummy_numa_init }; > + int (*scan_nodes[])(void) = { [2] = dummy_scan_nodes }; > + int i, j; > + > + if (!numa_off) { > +#ifdef CONFIG_ACPI_NUMA > + numa_init[0] = x86_acpi_numa_init; > + scan_nodes[0] = acpi_scan_nodes; > +#endif > +#ifdef CONFIG_AMD_NUMA > + numa_init[1] = amd_numa_init; > + scan_nodes[1] = amd_scan_nodes; > +#endif > + } > + > + for (i = 0; i < ARRAY_SIZE(numa_init); i++) { > + if (!numa_init[i]) > + continue; > + > + for (j = 0; j < MAX_LOCAL_APIC; j++) > + set_apicid_to_node(j, NUMA_NO_NODE); > + > + nodes_clear(node_possible_map); > + nodes_clear(node_online_map); > + > + if (numa_init[i]() < 0) > + continue; > +#ifdef CONFIG_NUMA_EMU > + setup_physnodes(0, max_pfn << PAGE_SHIFT, i == 0, i == 1); > + if (cmdline && !numa_emulation(0, max_pfn, i == 0, i == 1)) > + return; > + setup_physnodes(0, max_pfn << PAGE_SHIFT, i == 0, i == 1); > + nodes_clear(node_possible_map); > + nodes_clear(node_online_map); > +#endif > + if (!scan_nodes[i]()) > + return; > + } > + BUG(); > } This doesn't look very clean, I think it would be much better to extract the iteration out to a function of its own which takes an init and scan function as arguments and then deal only with numa_off, CONFIG_ACPI_NUMA, and CONFIG_AMD_NUMA callers in initmem_init(). If that's done, it's clear that the dummy implementation will never fail and so the BUG() in this case is never possible (which is intended if all else fails). ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [tip:x86/mm] x86-64, NUMA: Restructure initmem_init() 2011-02-20 3:34 ` [tip:x86/mm] x86-64, NUMA: Restructure initmem_init() David Rientjes @ 2011-02-21 8:35 ` Tejun Heo 2011-03-03 21:15 ` [patch] x86, mm: Clean up initmem_init David Rientjes 0 siblings, 1 reply; 13+ messages in thread From: Tejun Heo @ 2011-02-21 8:35 UTC (permalink / raw) To: David Rientjes Cc: Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, gorcunov, shaohui.zheng, linux-kernel On Sat, Feb 19, 2011 at 07:34:57PM -0800, David Rientjes wrote: > This doesn't look very clean, I think it would be much better to extract > the iteration out to a function of its own which takes an init and scan > function as arguments and then deal only with numa_off, CONFIG_ACPI_NUMA, > and CONFIG_AMD_NUMA callers in initmem_init(). If that's done, it's clear > that the dummy implementation will never fail and so the BUG() in this > case is never possible (which is intended if all else fails). I don't know. Maybe. Care to send a patch? -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* [patch] x86, mm: Clean up initmem_init 2011-02-21 8:35 ` Tejun Heo @ 2011-03-03 21:15 ` David Rientjes 2011-03-03 21:43 ` Cyrill Gorcunov 2011-03-04 10:16 ` Tejun Heo 0 siblings, 2 replies; 13+ messages in thread From: David Rientjes @ 2011-03-03 21:15 UTC (permalink / raw) To: Tejun Heo Cc: Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, gorcunov, shaohui.zheng, linux-kernel This patch cleans initmem_init() so that it is more readable and doesn't use an unnecessary array of function pointers to convolute the flow of the code. It also makes it obvious that dummy_numa_init() will always succeed (and documents that requirement) so that the existing BUG() is never actually reached. No functional change. Signed-off-by: David Rientjes <rientjes@google.com> --- arch/x86/mm/numa_64.c | 92 ++++++++++++++++++++++++++++--------------------- 1 files changed, 53 insertions(+), 39 deletions(-) diff --git a/arch/x86/mm/numa_64.c b/arch/x86/mm/numa_64.c --- a/arch/x86/mm/numa_64.c +++ b/arch/x86/mm/numa_64.c @@ -564,6 +564,13 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) return 0; } +/* + * Used if there's no underlying NUMA architecture, NUMA initialization fails, + * or NUMA is disabled on the command line. + * + * Must online at least one node and add memory blocks that cover all allowed + * memory. + */ static int __init dummy_numa_init(void) { printk(KERN_INFO "%s\n", @@ -577,57 +584,64 @@ static int __init dummy_numa_init(void) return 0; } -void __init initmem_init(void) +static int __init numa_init(int (*init_func)(void)) { - int (*numa_init[])(void) = { [2] = dummy_numa_init }; - int i, j; - - if (!numa_off) { -#ifdef CONFIG_ACPI_NUMA - numa_init[0] = x86_acpi_numa_init; -#endif -#ifdef CONFIG_AMD_NUMA - numa_init[1] = amd_numa_init; -#endif - } + int i; + int ret; - for (i = 0; i < ARRAY_SIZE(numa_init); i++) { - if (!numa_init[i]) - continue; + for (i = 0; i < MAX_LOCAL_APIC; i++) + set_apicid_to_node(i, NUMA_NO_NODE); - for (j = 0; j < MAX_LOCAL_APIC; j++) - set_apicid_to_node(j, NUMA_NO_NODE); + nodes_clear(numa_nodes_parsed); + nodes_clear(node_possible_map); + nodes_clear(node_online_map); + memset(&numa_meminfo, 0, sizeof(numa_meminfo)); + remove_all_active_ranges(); + numa_reset_distance(); - nodes_clear(numa_nodes_parsed); - nodes_clear(node_possible_map); - nodes_clear(node_online_map); - memset(&numa_meminfo, 0, sizeof(numa_meminfo)); - remove_all_active_ranges(); - numa_reset_distance(); + ret = init_func(); + if (ret < 0) + return ret; + ret = numa_cleanup_meminfo(&numa_meminfo); + if (ret < 0) + return ret; - if (numa_init[i]() < 0) - continue; + numa_emulation(&numa_meminfo, numa_distance_cnt); - if (numa_cleanup_meminfo(&numa_meminfo) < 0) - continue; + ret = numa_register_memblks(&numa_meminfo); + if (ret < 0) + return ret; - numa_emulation(&numa_meminfo, numa_distance_cnt); + for (i = 0; i < nr_cpu_ids; i++) { + int nid = early_cpu_to_node(i); - if (numa_register_memblks(&numa_meminfo) < 0) + if (nid == NUMA_NO_NODE) continue; + if (!node_online(nid)) + numa_clear_node(i); + } + numa_init_array(); + return 0; +} - for (j = 0; j < nr_cpu_ids; j++) { - int nid = early_cpu_to_node(j); +void __init initmem_init(void) +{ + int ret; - if (nid == NUMA_NO_NODE) - continue; - if (!node_online(nid)) - numa_clear_node(j); - } - numa_init_array(); - return; + if (!numa_off) { +#ifdef CONFIG_ACPI_NUMA + ret = numa_init(x86_acpi_numa_init); + if (!ret) + return; +#endif +#ifdef CONFIG_AMD_NUMA + ret = numa_init(amd_numa_init); + if (!ret) + return; +#endif } - BUG(); + + numa_init(dummy_numa_init); } unsigned long __init numa_free_all_bootmem(void) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 21:15 ` [patch] x86, mm: Clean up initmem_init David Rientjes @ 2011-03-03 21:43 ` Cyrill Gorcunov 2011-03-03 22:00 ` David Rientjes 2011-03-04 10:16 ` Tejun Heo 1 sibling, 1 reply; 13+ messages in thread From: Cyrill Gorcunov @ 2011-03-03 21:43 UTC (permalink / raw) To: David Rientjes Cc: Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, shaohui.zheng, linux-kernel On 03/04/2011 12:15 AM, David Rientjes wrote: > This patch cleans initmem_init() so that it is more readable and doesn't > use an unnecessary array of function pointers to convolute the flow of > the code. It also makes it obvious that dummy_numa_init() will always > succeed (and documents that requirement) so that the existing BUG() is > never actually reached. > > No functional change. > > Signed-off-by: David Rientjes <rientjes@google.com> > --- > arch/x86/mm/numa_64.c | 92 ++++++++++++++++++++++++++++--------------------- > 1 files changed, 53 insertions(+), 39 deletions(-) > ... > +void __init initmem_init(void) > +{ > + int ret; > > - if (nid == NUMA_NO_NODE) > - continue; > - if (!node_online(nid)) > - numa_clear_node(j); > - } > - numa_init_array(); > - return; > + if (!numa_off) { > +#ifdef CONFIG_ACPI_NUMA > + ret = numa_init(x86_acpi_numa_init); > + if (!ret) > + return; > +#endif > +#ifdef CONFIG_AMD_NUMA > + ret = numa_init(amd_numa_init); > + if (!ret) > + return; > +#endif > } > - BUG(); > + > + numa_init(dummy_numa_init); > } > > unsigned long __init numa_free_all_bootmem(void) Divid, I suspect it's due to diff format and we still need "ret" here, right? -- Cyrill ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 21:43 ` Cyrill Gorcunov @ 2011-03-03 22:00 ` David Rientjes 2011-03-03 22:03 ` Yinghai Lu 0 siblings, 1 reply; 13+ messages in thread From: David Rientjes @ 2011-03-03 22:00 UTC (permalink / raw) To: Cyrill Gorcunov Cc: Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, shaohui.zheng, linux-kernel On Fri, 4 Mar 2011, Cyrill Gorcunov wrote: > > This patch cleans initmem_init() so that it is more readable and doesn't > > use an unnecessary array of function pointers to convolute the flow of > > the code. It also makes it obvious that dummy_numa_init() will always > > succeed (and documents that requirement) so that the existing BUG() is > > never actually reached. > > > > No functional change. > > > > Signed-off-by: David Rientjes <rientjes@google.com> > > --- > > arch/x86/mm/numa_64.c | 92 ++++++++++++++++++++++++++++--------------------- > > 1 files changed, 53 insertions(+), 39 deletions(-) > > > ... > > +void __init initmem_init(void) > > +{ > > + int ret; > > > > - if (nid == NUMA_NO_NODE) > > - continue; > > - if (!node_online(nid)) > > - numa_clear_node(j); > > - } > > - numa_init_array(); > > - return; > > + if (!numa_off) { > > +#ifdef CONFIG_ACPI_NUMA > > + ret = numa_init(x86_acpi_numa_init); > > + if (!ret) > > + return; > > +#endif > > +#ifdef CONFIG_AMD_NUMA > > + ret = numa_init(amd_numa_init); > > + if (!ret) > > + return; > > +#endif > > } > > - BUG(); > > + > > + numa_init(dummy_numa_init); > > } > > > > unsigned long __init numa_free_all_bootmem(void) > > Divid, I suspect it's due to diff format and we still need "ret" here, right? > Not sure what you mean. "ret" is a temporary variable to test the return values of numa_init() and return from initmem_init() on success. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 22:00 ` David Rientjes @ 2011-03-03 22:03 ` Yinghai Lu 2011-03-03 22:05 ` Cyrill Gorcunov 2011-03-03 22:05 ` David Rientjes 0 siblings, 2 replies; 13+ messages in thread From: Yinghai Lu @ 2011-03-03 22:03 UTC (permalink / raw) To: David Rientjes Cc: Cyrill Gorcunov, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, shaohui.zheng, linux-kernel On 03/03/2011 02:00 PM, David Rientjes wrote: he want > +#ifdef CONFIG_ACPI_NUMA >> > + ret = numa_init(x86_acpi_numa_init); >> > + if (!ret) >> > + return; >> > +#endif >> > +#ifdef CONFIG_AMD_NUMA >> > + ret = numa_init(amd_numa_init); >> > + if (!ret) >> > + return; >> > +#endif to be replaced by: > +#ifdef CONFIG_ACPI_NUMA >> > + if (!numa_init(x86_acpi_numa_init)) >> > + return; >> > +#endif >> > +#ifdef CONFIG_AMD_NUMA >> > + if (!numa_init(amd_numa_init)) >> > + return; >> > +#endif ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 22:03 ` Yinghai Lu @ 2011-03-03 22:05 ` Cyrill Gorcunov 2011-03-03 22:05 ` David Rientjes 1 sibling, 0 replies; 13+ messages in thread From: Cyrill Gorcunov @ 2011-03-03 22:05 UTC (permalink / raw) To: Yinghai Lu Cc: David Rientjes, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, shaohui.zheng, linux-kernel On 03/04/2011 01:03 AM, Yinghai Lu wrote: > On 03/03/2011 02:00 PM, David Rientjes wrote: > > he want > yeah, if only i didn't miss something -- Cyrill ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 22:03 ` Yinghai Lu 2011-03-03 22:05 ` Cyrill Gorcunov @ 2011-03-03 22:05 ` David Rientjes 2011-03-03 22:10 ` Cyrill Gorcunov 2011-03-04 7:08 ` Ingo Molnar 1 sibling, 2 replies; 13+ messages in thread From: David Rientjes @ 2011-03-03 22:05 UTC (permalink / raw) To: Yinghai Lu Cc: Cyrill Gorcunov, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, shaohui.zheng, linux-kernel On Thu, 3 Mar 2011, Yinghai Lu wrote: > he want > > > +#ifdef CONFIG_ACPI_NUMA > >> > + ret = numa_init(x86_acpi_numa_init); > >> > + if (!ret) > >> > + return; > >> > +#endif > >> > +#ifdef CONFIG_AMD_NUMA > >> > + ret = numa_init(amd_numa_init); > >> > + if (!ret) > >> > + return; > >> > +#endif > > to be replaced by: > > > +#ifdef CONFIG_ACPI_NUMA > >> > + if (!numa_init(x86_acpi_numa_init)) > >> > + return; > >> > +#endif > >> > +#ifdef CONFIG_AMD_NUMA > >> > + if (!numa_init(amd_numa_init)) > >> > + return; > >> > +#endif It's a matter of style and I think it's up to Ingo what he'd prefer to see. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 22:05 ` David Rientjes @ 2011-03-03 22:10 ` Cyrill Gorcunov 2011-03-04 7:08 ` Ingo Molnar 1 sibling, 0 replies; 13+ messages in thread From: Cyrill Gorcunov @ 2011-03-03 22:10 UTC (permalink / raw) To: David Rientjes Cc: Yinghai Lu, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, linux-kernel On 03/04/2011 01:05 AM, David Rientjes wrote: > On Thu, 3 Mar 2011, Yinghai Lu wrote: > >> he want >> >>> +#ifdef CONFIG_ACPI_NUMA >>>>> + ret = numa_init(x86_acpi_numa_init); >>>>> + if (!ret) >>>>> + return; >>>>> +#endif >>>>> +#ifdef CONFIG_AMD_NUMA >>>>> + ret = numa_init(amd_numa_init); >>>>> + if (!ret) >>>>> + return; >>>>> +#endif >> >> to be replaced by: >> >>> +#ifdef CONFIG_ACPI_NUMA >>>>> + if (!numa_init(x86_acpi_numa_init)) >>>>> + return; >>>>> +#endif >>>>> +#ifdef CONFIG_AMD_NUMA >>>>> + if (!numa_init(amd_numa_init)) >>>>> + return; >>>>> +#endif > > It's a matter of style and I think it's up to Ingo what he'd prefer to > see. i'm fine with either, just though it would shrink the code a bit (since there is no need to track var witch i guess will be eliminated by gcc anyway). nb: am i only the one who obtain bounce from shaohui.zheng@intel.com (so i'd to remove him from cc'ing)? -- Cyrill ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 22:05 ` David Rientjes 2011-03-03 22:10 ` Cyrill Gorcunov @ 2011-03-04 7:08 ` Ingo Molnar 2011-03-04 12:38 ` Cyrill Gorcunov 1 sibling, 1 reply; 13+ messages in thread From: Ingo Molnar @ 2011-03-04 7:08 UTC (permalink / raw) To: David Rientjes Cc: Yinghai Lu, Cyrill Gorcunov, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, shaohui.zheng, linux-kernel * David Rientjes <rientjes@google.com> wrote: > On Thu, 3 Mar 2011, Yinghai Lu wrote: > > > he want > > > > > +#ifdef CONFIG_ACPI_NUMA > > >> > + ret = numa_init(x86_acpi_numa_init); > > >> > + if (!ret) > > >> > + return; > > >> > +#endif > > >> > +#ifdef CONFIG_AMD_NUMA > > >> > + ret = numa_init(amd_numa_init); > > >> > + if (!ret) > > >> > + return; > > >> > +#endif > > > > to be replaced by: > > > > > +#ifdef CONFIG_ACPI_NUMA > > >> > + if (!numa_init(x86_acpi_numa_init)) > > >> > + return; > > >> > +#endif > > >> > +#ifdef CONFIG_AMD_NUMA > > >> > + if (!numa_init(amd_numa_init)) > > >> > + return; > > >> > +#endif > > It's a matter of style and I think it's up to Ingo what he'd prefer to > see. I think your variant is cleaner: hiding function call side-effects in conditionscan be a fragile thing to do. We want constant expressions with no side-effects - so if functions are called they should be constant functions as well. Code compactness isn't everything - if it was we'd be using C to the max to create unreadable compound expressions all the time. Thanks, Ingo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-04 7:08 ` Ingo Molnar @ 2011-03-04 12:38 ` Cyrill Gorcunov 0 siblings, 0 replies; 13+ messages in thread From: Cyrill Gorcunov @ 2011-03-04 12:38 UTC (permalink / raw) To: Ingo Molnar Cc: David Rientjes, Yinghai Lu, Tejun Heo, Ingo Molnar, H. Peter Anvin, tglx, brgerst, shaohui.zheng, linux-kernel On Fri, Mar 4, 2011 at 10:08 AM, Ingo Molnar <mingo@elte.hu> wrote: ... >> >> It's a matter of style and I think it's up to Ingo what he'd prefer to >> see. > > I think your variant is cleaner: hiding function call side-effects in conditionscan > be a fragile thing to do. We want constant expressions with no side-effects - so if > functions are called they should be constant functions as well. Well, we do it a lot with say strcmp and others so I would use rather semantic argument, but I agree that David's version is easier to read and mark same time that these functions are important steps in bootup procedure. So I'm sorry for proposing this change in first place. > > Code compactness isn't everything - if it was we'd be using C to the max to create > unreadable compound expressions all the time. > > Thanks, > > Ingo > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [patch] x86, mm: Clean up initmem_init 2011-03-03 21:15 ` [patch] x86, mm: Clean up initmem_init David Rientjes 2011-03-03 21:43 ` Cyrill Gorcunov @ 2011-03-04 10:16 ` Tejun Heo 2011-03-04 14:19 ` [PATCH x86/mm] x86-64, NUMA: Clean up initmem_init() Tejun Heo 1 sibling, 1 reply; 13+ messages in thread From: Tejun Heo @ 2011-03-04 10:16 UTC (permalink / raw) To: David Rientjes Cc: Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, gorcunov, shaohui.zheng, linux-kernel Hello, On Thu, Mar 03, 2011 at 01:15:04PM -0800, David Rientjes wrote: > This patch cleans initmem_init() so that it is more readable and doesn't > use an unnecessary array of function pointers to convolute the flow of > the code. It also makes it obvious that dummy_numa_init() will always > succeed (and documents that requirement) so that the existing BUG() is > never actually reached. > > No functional change. > > Signed-off-by: David Rientjes <rientjes@google.com> Just a nitpick. > +/* > + * Used if there's no underlying NUMA architecture, NUMA initialization fails, > + * or NUMA is disabled on the command line. > + * > + * Must online at least one node and add memory blocks that cover all allowed > + * memory. > + */ Can you please reformat to docbook style? Thanks. -- tejun ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH x86/mm] x86-64, NUMA: Clean up initmem_init() 2011-03-04 10:16 ` Tejun Heo @ 2011-03-04 14:19 ` Tejun Heo 0 siblings, 0 replies; 13+ messages in thread From: Tejun Heo @ 2011-03-04 14:19 UTC (permalink / raw) To: David Rientjes Cc: Ingo Molnar, H. Peter Anvin, tglx, yinghai, brgerst, gorcunov, shaohui.zheng, linux-kernel >From c09cedf4f75f1e47ea17f55e18e9cfb81bec8575 Mon Sep 17 00:00:00 2001 From: David Rientjes <rientjes@google.com> Date: Fri, 4 Mar 2011 15:17:21 +0100 This patch cleans initmem_init() so that it is more readable and doesn't use an unnecessary array of function pointers to convolute the flow of the code. It also makes it obvious that dummy_numa_init() will always succeed (and documents that requirement) so that the existing BUG() is never actually reached. No functional change. -tj: Updated comment for dummy_numa_init() slightly. Signed-off-by: David Rientjes <rientjes@google.com> Signed-off-by: Tejun Heo <tj@kernel.org> --- Never mind the previous message. I just updated and applied it. Thanks. arch/x86/mm/numa_64.c | 94 ++++++++++++++++++++++++++++-------------------- 1 files changed, 55 insertions(+), 39 deletions(-) diff --git a/arch/x86/mm/numa_64.c b/arch/x86/mm/numa_64.c index 86491ba..9ec0f20 100644 --- a/arch/x86/mm/numa_64.c +++ b/arch/x86/mm/numa_64.c @@ -562,6 +562,15 @@ static int __init numa_register_memblks(struct numa_meminfo *mi) return 0; } +/** + * dummy_numma_init - Fallback dummy NUMA init + * + * Used if there's no underlying NUMA architecture, NUMA initialization + * fails, or NUMA is disabled on the command line. + * + * Must online at least one node and add memory blocks that cover all + * allowed memory. This function must not fail. + */ static int __init dummy_numa_init(void) { printk(KERN_INFO "%s\n", @@ -575,57 +584,64 @@ static int __init dummy_numa_init(void) return 0; } -void __init initmem_init(void) +static int __init numa_init(int (*init_func)(void)) { - int (*numa_init[])(void) = { [2] = dummy_numa_init }; - int i, j; - - if (!numa_off) { -#ifdef CONFIG_ACPI_NUMA - numa_init[0] = x86_acpi_numa_init; -#endif -#ifdef CONFIG_AMD_NUMA - numa_init[1] = amd_numa_init; -#endif - } + int i; + int ret; - for (i = 0; i < ARRAY_SIZE(numa_init); i++) { - if (!numa_init[i]) - continue; + for (i = 0; i < MAX_LOCAL_APIC; i++) + set_apicid_to_node(i, NUMA_NO_NODE); - for (j = 0; j < MAX_LOCAL_APIC; j++) - set_apicid_to_node(j, NUMA_NO_NODE); + nodes_clear(numa_nodes_parsed); + nodes_clear(node_possible_map); + nodes_clear(node_online_map); + memset(&numa_meminfo, 0, sizeof(numa_meminfo)); + remove_all_active_ranges(); + numa_reset_distance(); - nodes_clear(numa_nodes_parsed); - nodes_clear(node_possible_map); - nodes_clear(node_online_map); - memset(&numa_meminfo, 0, sizeof(numa_meminfo)); - remove_all_active_ranges(); - numa_reset_distance(); + ret = init_func(); + if (ret < 0) + return ret; + ret = numa_cleanup_meminfo(&numa_meminfo); + if (ret < 0) + return ret; - if (numa_init[i]() < 0) - continue; + numa_emulation(&numa_meminfo, numa_distance_cnt); - if (numa_cleanup_meminfo(&numa_meminfo) < 0) - continue; + ret = numa_register_memblks(&numa_meminfo); + if (ret < 0) + return ret; - numa_emulation(&numa_meminfo, numa_distance_cnt); + for (i = 0; i < nr_cpu_ids; i++) { + int nid = early_cpu_to_node(i); - if (numa_register_memblks(&numa_meminfo) < 0) + if (nid == NUMA_NO_NODE) continue; + if (!node_online(nid)) + numa_clear_node(i); + } + numa_init_array(); + return 0; +} - for (j = 0; j < nr_cpu_ids; j++) { - int nid = early_cpu_to_node(j); +void __init initmem_init(void) +{ + int ret; - if (nid == NUMA_NO_NODE) - continue; - if (!node_online(nid)) - numa_clear_node(j); - } - numa_init_array(); - return; + if (!numa_off) { +#ifdef CONFIG_ACPI_NUMA + ret = numa_init(x86_acpi_numa_init); + if (!ret) + return; +#endif +#ifdef CONFIG_AMD_NUMA + ret = numa_init(amd_numa_init); + if (!ret) + return; +#endif } - BUG(); + + numa_init(dummy_numa_init); } unsigned long __init numa_free_all_bootmem(void) -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-03-04 14:21 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <tip-ffe77a4605fb2588f8666850ad3e3b196241658f@git.kernel.org>
2011-02-20 3:34 ` [tip:x86/mm] x86-64, NUMA: Restructure initmem_init() David Rientjes
2011-02-21 8:35 ` Tejun Heo
2011-03-03 21:15 ` [patch] x86, mm: Clean up initmem_init David Rientjes
2011-03-03 21:43 ` Cyrill Gorcunov
2011-03-03 22:00 ` David Rientjes
2011-03-03 22:03 ` Yinghai Lu
2011-03-03 22:05 ` Cyrill Gorcunov
2011-03-03 22:05 ` David Rientjes
2011-03-03 22:10 ` Cyrill Gorcunov
2011-03-04 7:08 ` Ingo Molnar
2011-03-04 12:38 ` Cyrill Gorcunov
2011-03-04 10:16 ` Tejun Heo
2011-03-04 14:19 ` [PATCH x86/mm] x86-64, NUMA: Clean up initmem_init() Tejun Heo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox