[PATCH v3] mm: Check if section present during memory block registering

[PATCH v3] mm: Check if section present during memory block registering

[Yinghai Lu]

Tony found on his setup, if memory block size 512M will cause crash
during booting.

 BUG: unable to handle kernel paging request at ffffea0074000020
 IP: [<ffffffff81670527>] get_nid_for_pfn+0x17/0x40
 PGD 128ffcb067 PUD 128ffc9067 PMD 0 
 Oops: 0000 [#1] SMP 
 Modules linked in:
 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc8 #1
...
 Call Trace:
  [<ffffffff81453b56>] ? register_mem_sect_under_node+0x66/0xe0
  [<ffffffff81453eeb>] register_one_node+0x17b/0x240
  [<ffffffff81b1f1ed>] ? pci_iommu_alloc+0x6e/0x6e
  [<ffffffff81b1f229>] topology_init+0x3c/0x95
  [<ffffffff8100213d>] do_one_initcall+0xcd/0x1f0

The system has non continuous RAM address:
 BIOS-e820: [mem 0x0000001300000000-0x0000001cffffffff] usable
 BIOS-e820: [mem 0x0000001d70000000-0x0000001ec7ffefff] usable
 BIOS-e820: [mem 0x0000001f00000000-0x0000002bffffffff] usable
 BIOS-e820: [mem 0x0000002c18000000-0x0000002d6fffefff] usable
 BIOS-e820: [mem 0x0000002e00000000-0x00000039ffffffff] usable

So there are start sections in memory block not present.
For example:
memory block : [0x2c18000000, 0x2c20000000) 512M
first three sections are not present.

Current register_mem_sect_under_node() assume first section is present,
but memory block section number range [start_section_nr, end_section_nr]
would include not present section.

For arch that support vmemmap, we don't setup memmap for struct page area
within not present sections area.

So skip the pfn range that belong to absent section.

Reported-by: Tony Luck <tony.luck@intel.com>
Tested-by: Tony Luck <tony.luck@intel.com>
Fixes: bdee237c0343 ("x86: mm: Use 2GB memory block size on large memory x86-64 systems")
Fixes: 982792c782ef ("x86, mm: probe memory block size for generic x86 64bit")
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: stable@vger.kernel.org #v3.15

---
-v2: add fixes and comment.
-v3: we don't need to worry about unregiser path, as unregister_memory_section()
	already check if the section is present before.
---

 drivers/base/node.c |   31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/base/node.c
===================================================================
--- linux-2.6.orig/drivers/base/node.c
+++ linux-2.6/drivers/base/node.c
@@ -390,7 +390,18 @@ int register_mem_sect_under_node(struct
 	sect_end_pfn = section_nr_to_pfn(mem_blk->end_section_nr);
 	sect_end_pfn += PAGES_PER_SECTION - 1;
 	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
-		int page_nid;
+		int page_nid, scn_nr;
+
+		/*
+		 * memory block could have several absent sections from start.
+		 * skip pfn range from absent section
+		 */
+		scn_nr = pfn_to_section_nr(pfn);
+		if (!present_section_nr(scn_nr)) {
+			pfn = round_down(pfn + PAGES_PER_SECTION,
+					 PAGES_PER_SECTION) - 1;
+			continue;
+		}
 
 		page_nid = get_nid_for_pfn(pfn);
 		if (page_nid < 0)

Re: [PATCH v3] mm: Check if section present during memory block registering

[David Rientjes]

On Wed, 26 Aug 2015, Yinghai Lu wrote:

> Tony found on his setup, if memory block size 512M will cause crash
> during booting.
> 
>  BUG: unable to handle kernel paging request at ffffea0074000020
>  IP: [<ffffffff81670527>] get_nid_for_pfn+0x17/0x40
>  PGD 128ffcb067 PUD 128ffc9067 PMD 0 
>  Oops: 0000 [#1] SMP 
>  Modules linked in:
>  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc8 #1
> ...
>  Call Trace:
>   [<ffffffff81453b56>] ? register_mem_sect_under_node+0x66/0xe0
>   [<ffffffff81453eeb>] register_one_node+0x17b/0x240
>   [<ffffffff81b1f1ed>] ? pci_iommu_alloc+0x6e/0x6e
>   [<ffffffff81b1f229>] topology_init+0x3c/0x95
>   [<ffffffff8100213d>] do_one_initcall+0xcd/0x1f0
> 
> The system has non continuous RAM address:
>  BIOS-e820: [mem 0x0000001300000000-0x0000001cffffffff] usable
>  BIOS-e820: [mem 0x0000001d70000000-0x0000001ec7ffefff] usable
>  BIOS-e820: [mem 0x0000001f00000000-0x0000002bffffffff] usable
>  BIOS-e820: [mem 0x0000002c18000000-0x0000002d6fffefff] usable
>  BIOS-e820: [mem 0x0000002e00000000-0x00000039ffffffff] usable
> 
> So there are start sections in memory block not present.
> For example:
> memory block : [0x2c18000000, 0x2c20000000) 512M
> first three sections are not present.
> 
> Current register_mem_sect_under_node() assume first section is present,
> but memory block section number range [start_section_nr, end_section_nr]
> would include not present section.
> 
> For arch that support vmemmap, we don't setup memmap for struct page area
> within not present sections area.
> 
> So skip the pfn range that belong to absent section.
> 
> Reported-by: Tony Luck <tony.luck@intel.com>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Fixes: bdee237c0343 ("x86: mm: Use 2GB memory block size on large memory x86-64 systems")
> Fixes: 982792c782ef ("x86, mm: probe memory block size for generic x86 64bit")
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@vger.kernel.org #v3.15

stable@vger.kernel.org probably never received this since it was sent to 
stable@vger.kernel.org.#v3.15

> Index: linux-2.6/drivers/base/node.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/node.c
> +++ linux-2.6/drivers/base/node.c
> @@ -390,7 +390,18 @@ int register_mem_sect_under_node(struct
>  	sect_end_pfn = section_nr_to_pfn(mem_blk->end_section_nr);
>  	sect_end_pfn += PAGES_PER_SECTION - 1;
>  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> -		int page_nid;
> +		int page_nid, scn_nr;
> +
> +		/*
> +		 * memory block could have several absent sections from start.
> +		 * skip pfn range from absent section
> +		 */
> +		scn_nr = pfn_to_section_nr(pfn);
> +		if (!present_section_nr(scn_nr)) {
> +			pfn = round_down(pfn + PAGES_PER_SECTION,
> +					 PAGES_PER_SECTION) - 1;
> +			continue;
> +		}
>  
>  		page_nid = get_nid_for_pfn(pfn);
>  		if (page_nid < 0)

scn_nr should really be unsigned long, but it would probably be better to 
simply use if (!pfn_present(pfn)) rather than store the section number.
\

Re: [PATCH v3] mm: Check if section present during memory block registering

[Andrew Morton]

On Wed, 26 Aug 2015 14:38:35 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:

> > --- linux-2.6.orig/drivers/base/node.c
> > +++ linux-2.6/drivers/base/node.c
> > @@ -390,7 +390,18 @@ int register_mem_sect_under_node(struct
> >  	sect_end_pfn = section_nr_to_pfn(mem_blk->end_section_nr);
> >  	sect_end_pfn += PAGES_PER_SECTION - 1;
> >  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> > -		int page_nid;
> > +		int page_nid, scn_nr;
> > +
> > +		/*
> > +		 * memory block could have several absent sections from start.
> > +		 * skip pfn range from absent section
> > +		 */
> > +		scn_nr = pfn_to_section_nr(pfn);
> > +		if (!present_section_nr(scn_nr)) {
> > +			pfn = round_down(pfn + PAGES_PER_SECTION,
> > +					 PAGES_PER_SECTION) - 1;
> > +			continue;
> > +		}
> >  
> >  		page_nid = get_nid_for_pfn(pfn);
> >  		if (page_nid < 0)
> 
> scn_nr should really be unsigned long, but it would probably be better to 
> simply use if (!pfn_present(pfn)) rather than store the section number.

Yup.  I was feeling lazy but you motivated me.

<grunt, wheeze>

--- a/drivers/base/node.c~mm-check-if-section-present-during-memory-block-registering-fix
+++ a/drivers/base/node.c
@@ -390,14 +390,13 @@ int register_mem_sect_under_node(struct
 	sect_end_pfn = section_nr_to_pfn(mem_blk->end_section_nr);
 	sect_end_pfn += PAGES_PER_SECTION - 1;
 	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
-		int page_nid, scn_nr;
+		int page_nid;
 
 		/*
 		 * memory block could have several absent sections from start.
 		 * skip pfn range from absent section
 		 */
-		scn_nr = pfn_to_section_nr(pfn);
-		if (!present_section_nr(scn_nr)) {
+		if (!present_section_nr(pfn_to_section_nr(pfn))) {
 			pfn = round_down(pfn + PAGES_PER_SECTION,
 					 PAGES_PER_SECTION) - 1;
 			continue;
_

Re: [PATCH v3] mm: Check if section present during memory block registering

[David Rientjes]

On Wed, 26 Aug 2015, Andrew Morton wrote:

> Yup.  I was feeling lazy but you motivated me.
> 
> <grunt, wheeze>
> 
> --- a/drivers/base/node.c~mm-check-if-section-present-during-memory-block-registering-fix
> +++ a/drivers/base/node.c
> @@ -390,14 +390,13 @@ int register_mem_sect_under_node(struct
>  	sect_end_pfn = section_nr_to_pfn(mem_blk->end_section_nr);
>  	sect_end_pfn += PAGES_PER_SECTION - 1;
>  	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> -		int page_nid, scn_nr;
> +		int page_nid;
>  
>  		/*
>  		 * memory block could have several absent sections from start.
>  		 * skip pfn range from absent section
>  		 */
> -		scn_nr = pfn_to_section_nr(pfn);
> -		if (!present_section_nr(scn_nr)) {
> +		if (!present_section_nr(pfn_to_section_nr(pfn))) {
>  			pfn = round_down(pfn + PAGES_PER_SECTION,
>  					 PAGES_PER_SECTION) - 1;
>  			continue;

Let me get in on this simplification party.

Signed-off-by: David Rientjes <rientjes@google.com>
---
diff --git a/drivers/base/node.c b/drivers/base/node.c
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -396,7 +396,7 @@ int register_mem_sect_under_node(struct memory_block *mem_blk, int nid)
 		 * memory block could have several absent sections from start.
 		 * skip pfn range from absent section
 		 */
-		if (!present_section_nr(pfn_to_section_nr(pfn))) {
+		if (!pfn_present(pfn)) {
 			pfn = round_down(pfn + PAGES_PER_SECTION,
 					 PAGES_PER_SECTION) - 1;
 			continue;

Re: [PATCH v3] mm: Check if section present during memory block registering

[David Rientjes]

On Wed, 26 Aug 2015, Yinghai Lu wrote:

> Tony found on his setup, if memory block size 512M will cause crash
> during booting.
> 
>  BUG: unable to handle kernel paging request at ffffea0074000020
>  IP: [<ffffffff81670527>] get_nid_for_pfn+0x17/0x40
>  PGD 128ffcb067 PUD 128ffc9067 PMD 0 
>  Oops: 0000 [#1] SMP 
>  Modules linked in:
>  CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.2.0-rc8 #1
> ...
>  Call Trace:
>   [<ffffffff81453b56>] ? register_mem_sect_under_node+0x66/0xe0
>   [<ffffffff81453eeb>] register_one_node+0x17b/0x240
>   [<ffffffff81b1f1ed>] ? pci_iommu_alloc+0x6e/0x6e
>   [<ffffffff81b1f229>] topology_init+0x3c/0x95
>   [<ffffffff8100213d>] do_one_initcall+0xcd/0x1f0
> 
> The system has non continuous RAM address:
>  BIOS-e820: [mem 0x0000001300000000-0x0000001cffffffff] usable
>  BIOS-e820: [mem 0x0000001d70000000-0x0000001ec7ffefff] usable
>  BIOS-e820: [mem 0x0000001f00000000-0x0000002bffffffff] usable
>  BIOS-e820: [mem 0x0000002c18000000-0x0000002d6fffefff] usable
>  BIOS-e820: [mem 0x0000002e00000000-0x00000039ffffffff] usable
> 
> So there are start sections in memory block not present.
> For example:
> memory block : [0x2c18000000, 0x2c20000000) 512M
> first three sections are not present.
> 
> Current register_mem_sect_under_node() assume first section is present,
> but memory block section number range [start_section_nr, end_section_nr]
> would include not present section.
> 
> For arch that support vmemmap, we don't setup memmap for struct page area
> within not present sections area.
> 
> So skip the pfn range that belong to absent section.
> 
> Reported-by: Tony Luck <tony.luck@intel.com>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Fixes: bdee237c0343 ("x86: mm: Use 2GB memory block size on large memory x86-64 systems")
> Fixes: 982792c782ef ("x86, mm: probe memory block size for generic x86 64bit")
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@vger.kernel.org #v3.15

With the fixes folded:

Tested-by: David Rientjes <rientjes@google.com>