[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.00.1102191906180.31263@chino.kir.corp.google.com>
Date: Sat, 19 Feb 2011 19:34:57 -0800 (PST)
From: David Rientjes <rientjes@...gle.com>
To: Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
tglx@...utronix.de, Tejun Heo <tj@...nel.org>
cc: yinghai@...nel.org, brgerst@...il.com, gorcunov@...il.com,
shaohui.zheng@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [tip:x86/mm] x86-64, NUMA: Restructure initmem_init()
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).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists