Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
2012/09/15 5:14, Andrew Morton wrote: On Fri, 14 Sep 2012 20:00:09 +0900 Yasuaki Ishimatsu 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? Looks good to me. Reviewed-by: Yasuaki Ishimatsu --- 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); } _ -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majord...@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: mailto:"d...@kvack.org;> em...@kvack.org -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, Sep 14, 2012 at 04:24:32PM +, Luck, Tony wrote: > > This is an unusual configuration but it's not unheard of. PPC64 in rare > > (and usually broken) configurations can have one node span another. Tony > > should know if such a configuration is normally allowed on Itanium or if > > this should be considered a platform bug. Tony? > > We definitely have platforms where the physical memory on node 0 > that we skipped to leave physical address space for PCI mem mapped > devices gets tagged back at the very top of memory, after other nodes. > > E.g. A 2-node system with 8G on each might look like this: > > 0-2G RAM on node 0 > 2G-4G PCI map space > 4G-8G RAM on node 0 > 8G-16GRAM on node 1 > 16G-18G RAM on node 0 > > Is this the situation that we are talking about? Or something different? > This is the type of situation we are talking about. The spanned range of node 0 includes node 1. The patch needs another revision with a comment explaining the situation included but otherwise the patch should be fine. -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, Sep 14, 2012 at 04:24:32PM +, Luck, Tony wrote: This is an unusual configuration but it's not unheard of. PPC64 in rare (and usually broken) configurations can have one node span another. Tony should know if such a configuration is normally allowed on Itanium or if this should be considered a platform bug. Tony? We definitely have platforms where the physical memory on node 0 that we skipped to leave physical address space for PCI mem mapped devices gets tagged back at the very top of memory, after other nodes. E.g. A 2-node system with 8G on each might look like this: 0-2G RAM on node 0 2G-4G PCI map space 4G-8G RAM on node 0 8G-16GRAM on node 1 16G-18G RAM on node 0 Is this the situation that we are talking about? Or something different? This is the type of situation we are talking about. The spanned range of node 0 includes node 1. The patch needs another revision with a comment explaining the situation included but otherwise the patch should be fine. -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
2012/09/15 5:14, Andrew Morton wrote: On Fri, 14 Sep 2012 20:00:09 +0900 Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com 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? Looks good to me. Reviewed-by: Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com --- 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); } _ -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majord...@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, 14 Sep 2012 20:00:09 +0900 Yasuaki Ishimatsu 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? --- 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); } _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH RESEND] memory hotplug: fix a double register section info bug
> This is an unusual configuration but it's not unheard of. PPC64 in rare > (and usually broken) configurations can have one node span another. Tony > should know if such a configuration is normally allowed on Itanium or if > this should be considered a platform bug. Tony? We definitely have platforms where the physical memory on node 0 that we skipped to leave physical address space for PCI mem mapped devices gets tagged back at the very top of memory, after other nodes. E.g. A 2-node system with 8G on each might look like this: 0-2G RAM on node 0 2G-4G PCI map space 4G-8G RAM on node 0 8G-16GRAM on node 1 16G-18G RAM on node 0 Is this the situation that we are talking about? Or something different? -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
HiXishi, 2012/09/14 12:43, qiuxishi wrote: There may be a bug when registering section info. For example, on my Itanium platform, the pfn range of node0 includes the other nodes, so other nodes' section info will be double registered, and memmap's page count will equal to 3. node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, => 0x000100-0x20fc00 node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, => 0x08-0x10 node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, => 0x10-0x18 node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, => 0x18-0x20 free_all_bootmem_node() register_page_bootmem_info_node() register_page_bootmem_info_section() When hot remove memory, we can't free the memmap's page because page_count() is 2 after put_page_bootmem(). sparse_remove_one_section() free_section_usemap() free_map_bootmem() put_page_bootmem() Signed-off-by: Xishi Qiu Signed-off-by: Jiang Liu --- mm/memory_hotplug.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2adbcac..cf493c7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned long start_pfn) struct mem_section *ms; struct page *page, *memmap; - if (!pfn_valid(start_pfn)) - return; - section_nr = pfn_to_section_nr(start_pfn); ms = __nr_to_section(section_nr); @@ -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. Thanks, Yasuaki Ishimatsu + register_page_bootmem_info_section(pfn); + } } #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, Sep 14, 2012 at 11:43:27AM +0800, qiuxishi wrote: > There may be a bug when registering section info. For example, on > my Itanium platform, the pfn range of node0 includes the other nodes, > so other nodes' section info will be double registered, and memmap's > page count will equal to 3. > > node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, => > 0x000100-0x20fc00 > node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, => > 0x08-0x10 > node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, => > 0x10-0x18 > node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, => > 0x18-0x20 > This is an unusual configuration but it's not unheard of. PPC64 in rare (and usually broken) configurations can have one node span another. Tony should know if such a configuration is normally allowed on Itanium or if this should be considered a platform bug. Tony? > free_all_bootmem_node() > register_page_bootmem_info_node() > register_page_bootmem_info_section() > > When hot remove memory, we can't free the memmap's page because > page_count() is 2 after put_page_bootmem(). > > sparse_remove_one_section() > free_section_usemap() > free_map_bootmem() > put_page_bootmem() > > Signed-off-by: Xishi Qiu > Signed-off-by: Jiang Liu > --- > mm/memory_hotplug.c | 10 -- > 1 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 2adbcac..cf493c7 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned > long start_pfn) > struct mem_section *ms; > struct page *page, *memmap; > > - if (!pfn_valid(start_pfn)) > - return; > - > section_nr = pfn_to_section_nr(start_pfn); > ms = __nr_to_section(section_nr); > > @@ -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)) > + register_page_bootmem_info_section(pfn); > + } Functionally what the patch does is check if the PFN is both valid *and* belongs to the expected node to catch a situation where nodes overlap. As there are no other callers of register_page_bootmem_info_section() this patch seems reasonable to me so Acked-by: Mel Gorman I think it would also be ok to consider this a -stable candidate. > } > #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ > > -- > 1.7.1 -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, Sep 14, 2012 at 11:43:27AM +0800, qiuxishi wrote: There may be a bug when registering section info. For example, on my Itanium platform, the pfn range of node0 includes the other nodes, so other nodes' section info will be double registered, and memmap's page count will equal to 3. node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, = 0x000100-0x20fc00 node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, = 0x08-0x10 node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, = 0x10-0x18 node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, = 0x18-0x20 This is an unusual configuration but it's not unheard of. PPC64 in rare (and usually broken) configurations can have one node span another. Tony should know if such a configuration is normally allowed on Itanium or if this should be considered a platform bug. Tony? free_all_bootmem_node() register_page_bootmem_info_node() register_page_bootmem_info_section() When hot remove memory, we can't free the memmap's page because page_count() is 2 after put_page_bootmem(). sparse_remove_one_section() free_section_usemap() free_map_bootmem() put_page_bootmem() Signed-off-by: Xishi Qiu qiuxi...@huawei.com Signed-off-by: Jiang Liu jiang@huawei.com --- mm/memory_hotplug.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2adbcac..cf493c7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned long start_pfn) struct mem_section *ms; struct page *page, *memmap; - if (!pfn_valid(start_pfn)) - return; - section_nr = pfn_to_section_nr(start_pfn); ms = __nr_to_section(section_nr); @@ -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)) + register_page_bootmem_info_section(pfn); + } Functionally what the patch does is check if the PFN is both valid *and* belongs to the expected node to catch a situation where nodes overlap. As there are no other callers of register_page_bootmem_info_section() this patch seems reasonable to me so Acked-by: Mel Gorman mgor...@suse.de I think it would also be ok to consider this a -stable candidate. } #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ -- 1.7.1 -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
HiXishi, 2012/09/14 12:43, qiuxishi wrote: There may be a bug when registering section info. For example, on my Itanium platform, the pfn range of node0 includes the other nodes, so other nodes' section info will be double registered, and memmap's page count will equal to 3. node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, = 0x000100-0x20fc00 node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, = 0x08-0x10 node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, = 0x10-0x18 node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, = 0x18-0x20 free_all_bootmem_node() register_page_bootmem_info_node() register_page_bootmem_info_section() When hot remove memory, we can't free the memmap's page because page_count() is 2 after put_page_bootmem(). sparse_remove_one_section() free_section_usemap() free_map_bootmem() put_page_bootmem() Signed-off-by: Xishi Qiu qiuxi...@huawei.com Signed-off-by: Jiang Liu jiang@huawei.com --- mm/memory_hotplug.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2adbcac..cf493c7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned long start_pfn) struct mem_section *ms; struct page *page, *memmap; - if (!pfn_valid(start_pfn)) - return; - section_nr = pfn_to_section_nr(start_pfn); ms = __nr_to_section(section_nr); @@ -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. Thanks, Yasuaki Ishimatsu + register_page_bootmem_info_section(pfn); + } } #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH RESEND] memory hotplug: fix a double register section info bug
This is an unusual configuration but it's not unheard of. PPC64 in rare (and usually broken) configurations can have one node span another. Tony should know if such a configuration is normally allowed on Itanium or if this should be considered a platform bug. Tony? We definitely have platforms where the physical memory on node 0 that we skipped to leave physical address space for PCI mem mapped devices gets tagged back at the very top of memory, after other nodes. E.g. A 2-node system with 8G on each might look like this: 0-2G RAM on node 0 2G-4G PCI map space 4G-8G RAM on node 0 8G-16GRAM on node 1 16G-18G RAM on node 0 Is this the situation that we are talking about? Or something different? -Tony -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] memory hotplug: fix a double register section info bug
On Fri, 14 Sep 2012 20:00:09 +0900 Yasuaki Ishimatsu isimatu.yasu...@jp.fujitsu.com 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? --- 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); } _ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] memory hotplug: fix a double register section info bug
There may be a bug when registering section info. For example, on my Itanium platform, the pfn range of node0 includes the other nodes, so other nodes' section info will be double registered, and memmap's page count will equal to 3. node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, => 0x000100-0x20fc00 node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, => 0x08-0x10 node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, => 0x10-0x18 node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, => 0x18-0x20 free_all_bootmem_node() register_page_bootmem_info_node() register_page_bootmem_info_section() When hot remove memory, we can't free the memmap's page because page_count() is 2 after put_page_bootmem(). sparse_remove_one_section() free_section_usemap() free_map_bootmem() put_page_bootmem() Signed-off-by: Xishi Qiu Signed-off-by: Jiang Liu --- mm/memory_hotplug.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2adbcac..cf493c7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned long start_pfn) struct mem_section *ms; struct page *page, *memmap; - if (!pfn_valid(start_pfn)) - return; - section_nr = pfn_to_section_nr(start_pfn); ms = __nr_to_section(section_nr); @@ -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)) + register_page_bootmem_info_section(pfn); + } } #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] memory hotplug: fix a double register section info bug
There may be a bug when registering section info. For example, on my Itanium platform, the pfn range of node0 includes the other nodes, so other nodes' section info will be double registered, and memmap's page count will equal to 3. node0: start_pfn=0x100,spanned_pfn=0x20fb00, present_pfn=0x7f8a3, = 0x000100-0x20fc00 node1: start_pfn=0x8, spanned_pfn=0x8, present_pfn=0x8, = 0x08-0x10 node2: start_pfn=0x10, spanned_pfn=0x8, present_pfn=0x8, = 0x10-0x18 node3: start_pfn=0x18, spanned_pfn=0x8, present_pfn=0x8, = 0x18-0x20 free_all_bootmem_node() register_page_bootmem_info_node() register_page_bootmem_info_section() When hot remove memory, we can't free the memmap's page because page_count() is 2 after put_page_bootmem(). sparse_remove_one_section() free_section_usemap() free_map_bootmem() put_page_bootmem() Signed-off-by: Xishi Qiu qiuxi...@huawei.com Signed-off-by: Jiang Liu jiang@huawei.com --- mm/memory_hotplug.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2adbcac..cf493c7 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -126,9 +126,6 @@ static void register_page_bootmem_info_section(unsigned long start_pfn) struct mem_section *ms; struct page *page, *memmap; - if (!pfn_valid(start_pfn)) - return; - section_nr = pfn_to_section_nr(start_pfn); ms = __nr_to_section(section_nr); @@ -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)) + register_page_bootmem_info_section(pfn); + } } #endif /* !CONFIG_SPARSEMEM_VMEMMAP */ -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/