Messages in this thread | | | Date | Sun, 17 Jan 2010 22:30:14 -0800 | Subject | Re: [PATCH] x86/mm/srat_64.c: nodes_parsed should include all nodes detected by ACPI. | From | Yinghai Lu <> |
| |
On Sun, Jan 17, 2010 at 1:53 PM, David Rientjes <rientjes@google.com> wrote: > On Fri, 15 Jan 2010, Haicheng Li wrote: > >> This is to fix the bug discussed in email thread: >> http://patchwork.kernel.org/patch/69499/. >> > > It would be better to include a brief synopsis of the problem in the > changelog instead of a citation, otherwise it obfuscates the issue it > solves. > >> Currently node_possible_map won't include the offlined node that has neither >> CPU onlined nor MEM onlined at booting time. As a result, nr_node_ids won't be >> equal to possible nodes. >> >> CC: Thomas Gleixner <tglx@linutronix.de> >> CC: Ingo Molnar <mingo@redhat.com> >> CC: H. Peter Anvin <hpa@zytor.com> >> CC: Andi Kleen <andi@firstfloor.org> >> Signed-off-by: Haicheng Li <haicheng.li@linux.intel.com> >> --- >> arch/x86/mm/srat_64.c | 10 ++-------- >> 1 files changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/arch/x86/mm/srat_64.c b/arch/x86/mm/srat_64.c >> index a271241..a5bc297 100644 >> --- a/arch/x86/mm/srat_64.c >> +++ b/arch/x86/mm/srat_64.c >> @@ -238,7 +238,7 @@ update_nodes_add(int node, unsigned long start, unsigned >> long end) >> void __init >> acpi_numa_memory_affinity_init(struct acpi_srat_mem_affinity *ma) >> { >> - struct bootnode *nd, oldnode; >> + struct bootnode *nd; >> unsigned long start, end; >> int node, pxm; >> int i; >> @@ -277,7 +277,6 @@ acpi_numa_memory_affinity_init(struct >> acpi_srat_mem_affinity *ma) >> return; >> } >> nd = &nodes[node]; >> - oldnode = *nd; >> if (!node_test_and_set(node, nodes_parsed)) { >> nd->start = start; >> nd->end = end; >> @@ -291,13 +290,8 @@ acpi_numa_memory_affinity_init(struct >> acpi_srat_mem_affinity *ma) >> printk(KERN_INFO "SRAT: Node %u PXM %u %lx-%lx\n", node, pxm, >> start, end); >> >> - if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) { >> + if (ma->flags & ACPI_SRAT_MEM_HOT_PLUGGABLE) >> update_nodes_add(node, start, end); >> - /* restore nodes[node] */ >> - *nd = oldnode; >> - if ((nd->start | nd->end) == 0) >> - node_clear(node, nodes_parsed); >> - } >> >> node_memblk_range[num_node_memblks].start = start; >> node_memblk_range[num_node_memblks].end = end; > > There're a couple of issues with this patch: > > - it breaks CONFIG_MEMORY_HOTPLUG_SPARSE kernels when the SRAT reports > more than one entry for a single node id and the later entry does not > specify an address range, and
not sure this one.
> > - carrying the bit for the node over in nodes_parsed is inappropriate > since other x86 code depends on that map including only nodes with > memory such as e820_register_active_regions() and nodes_cover_memory().
assume if e820 is right, even srat table have a range for hot plug aka the e820 have hole already. then e820_register_active_regions() and nodes_cover_memory should be ok even bootnode have some pre reserved range.
YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |