On Sat, Sep 15, 2012 at 4:14 AM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
On Fri, 14 Sep 2012 20:00:09 +0900
Yasuaki Ishimatsu <isimatu.yasuaki@xxxxxxxxxxxxxx> wrote:
> > @@ -187,9 +184,10 @@ void register_page_bootmem_info_node(struct pglist_data *pgdat)
> > end_pfn = pfn + pgdat->node_spanned_pages;
> >
> > /* register_section info */
> > - for (; pfn < end_pfn; pfn += PAGES_PER_SECTION)
> > - register_page_bootmem_info_section(pfn);
> > -
> > + for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> > + if (pfn_valid(pfn) && (pfn_to_nid(pfn) == node))
>
> I cannot judge whether your configuration is correct or not.
> Thus if it is correct, I want a comment of why the node check is
> needed. In usual configuration, a node does not span the other one.
> So it is natural that "pfn_to_nid(pfn) is same as "pgdat->node_id".
> Thus we may remove the node check in the future.
yup. How does this look?
It looks fine to me. Some platforms can have this unusual configuration, not only in IA64.
And register_page_bootmem_info_section() is only called by register_page_bootmem_info_node(),
so we can delete pfn_valid() check in register_page_bootmem_info_section()
Thanks
Xishi Qiu
--- a/mm/memory_hotplug.c~memory-hotplug-fix-a-double-register-section-info-bug-fix
+++ a/mm/memory_hotplug.c
@@ -185,6 +185,12 @@ void register_page_bootmem_info_node(str
/* register_section info */
for (; pfn < end_pfn; pfn += PAGES_PER_SECTION) {+ /*
+ * Some platforms can assign the same pfn to multiple nodes - on
+ * node0 as well as nodeN. To avoid registering a pfn against
+ * multiple nodes we check that this pfn does not already
+ * reside in some other node.
+ */
if (pfn_valid(pfn) && (pfn_to_nid(pfn) == node))register_page_bootmem_info_section(pfn);
}
_