mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 11 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

Quoted 3 lines

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Quoted 18 lines · Andrew Morton

Quoted 14 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 3 lines

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Quoted 19 lines · On Mon, 10 Oct 2011 16:24:03 -0700 Greg KH <[email protected]> wrote:

Quoted 18 lines · Andrew Morton

Quoted 14 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 6 lines

Quoted 3 lines

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Drop

commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd

Author: Nathan Fontenot <[email protected]>
Date:   Mon Sep 26 10:22:33 2011 -0500

memory hotplug: Correct page reservation checking

and replace it with start-of-this-thread.

That's assuming that Mel's update passes Nathan's review and testing :)

Quoted 11 lines

Quoted 2 lines

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Drop

commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd

Author: Nathan Fontenot <[email protected]>
Date:   Mon Sep 26 10:22:33 2011 -0500

memory hotplug: Correct page reservation checking

Quoted 3 lines

and replace it with start-of-this-thread.

That's assuming that Mel's update passes Nathan's review and testing :)

Ok, I'll wait for that review and testing, and then can someone send me the patch at the start-of-this-thread as I no longer seem to be able to find it :(

drivers/base/memory.c +12 -0

index 2840ed4..ffb69cd 100644
@@ -224,13 +224,48 @@ int memory_isolate_notify(unsigned long val, void *v)
 }
 
 /*
+ * The probe routines leave the pages reserved, just as the bootmem code does.
+ * Make sure they're still that way.
+ */
+static bool pages_correctly_reserved(unsigned long start_pfn,
+					unsigned long nr_pages)
+{
+	int i, j;
+	struct page *page;
+	unsigned long pfn = start_pfn;
+
+	/*
+	 * memmap between sections is not contiguous except with

drivers/base/memory.c +18 -18

+				continue;
+
+			printk(KERN_WARNING "section number %ld page number %d "
+				"not reserved, was it already online?\n",
+				pfn_to_section_nr(pfn), j);
+
+			return false;
+		}
+	}
+
+	return true;
+}
+
+/*
  * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
  * OK to have direct references to sparsemem variables in here.
  */
 static int
 memory_block_action(unsigned long phys_index, unsigned long action)
 {
-	int i;
 	unsigned long start_pfn, start_paddr;
 	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
 	struct page *first_page;

@@ -238,26 +273,13 @@ memory_block_action(unsigned long phys_index, unsigned long action)
 
 	first_page = pfn_to_page(phys_index << PFN_SECTION_SHIFT);
 
-	/*
-	 * The probe routines leave the pages reserved, just
-	 * as the bootmem code does.  Make sure they're still
-	 * that way.
-	 */
-	if (action == MEM_ONLINE) {
-		for (i = 0; i < nr_pages; i++) {
-			if (PageReserved(first_page+i))
-				continue;
-
-			printk(KERN_WARNING "section number %ld page number %d "
-				"not reserved, was it already online?\n",
-				phys_index, i);
-			return -EBUSY;
-		}
-	}
-
 	switch (action) {
 		case MEM_ONLINE:
 			start_pfn = page_to_pfn(first_page);
+
+			if (!pages_correctly_reserved(start_pfn, nr_pages))
+				return -EBUSY;
+
 			ret = online_pages(start_pfn, nr_pages);
 			break;
 		case MEM_OFFLINE:

Quoted 19 lines · On Mon, 10 Oct 2011 16:24:03 -0700 Greg KH <[email protected]> wrote:

Quoted 18 lines · Andrew Morton

Quoted 14 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 6 lines

Quoted 3 lines

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Drop

commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd

Author: Nathan Fontenot <[email protected]>
Date:   Mon Sep 26 10:22:33 2011 -0500

memory hotplug: Correct page reservation checking

and replace it with start-of-this-thread.

That's assuming that Mel's update passes Nathan's review and testing :)

Quoted 32 lines · Andrew Morton

Quoted 19 lines · On Mon, 10 Oct 2011 16:24:03 -0700 Greg KH <[email protected]> wrote:

Quoted 18 lines · Andrew Morton

Quoted 14 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 6 lines

Quoted 3 lines

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Drop

commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd

Quoted 8 lines

Author: Nathan Fontenot <[email protected]>
Date:   Mon Sep 26 10:22:33 2011 -0500

memory hotplug: Correct page reservation checking

and replace it with start-of-this-thread.

That's assuming that Mel's update passes Nathan's review and testing :)

It passed review and testing with IBM based on a SUSE bug. I thought Nathan's patch had been lost as it was posted to linuxppc-dev instead of linux-mm. This rework was to improve the changelog and readability.

David correctly pointed out a bug that passed testing because it was still checking one page per section. As long as that page was reserved, memory hot-add would go ahead. Here is a corrected version.

Thanks

==== CUT HERE ====
mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on those configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

[Check pages within sections properly: [email protected]]

[original patch by: [email protected]]

Signed-off-by Mel Gorman <[email protected]>

---
 drivers/base/memory.c |   58 +++++++++++++++++++++++++++++++++---------------
 1 files changed, 40 insertions(+), 18 deletions(-)

drivers/base/memory.c +40 -18

index 2840ed4..ffb69cd 100644
@@ -224,13 +224,48 @@ int memory_isolate_notify(unsigned long val, void *v)
 }
 
 /*
+ * The probe routines leave the pages reserved, just as the bootmem code does.
+ * Make sure they're still that way.
+ */
+static bool pages_correctly_reserved(unsigned long start_pfn,
+					unsigned long nr_pages)
+{
+	int i, j;
+	struct page *page;
+	unsigned long pfn = start_pfn;
+
+	/*
+	 * memmap between sections is not contiguous except with
+	 * SPARSEMEM_VMEMMAP. We lookup the page once per section
+	 * and assume memmap is contiguous within each section
+	 */
+	for (i = 0; i < sections_per_block; i++, pfn += PAGES_PER_SECTION) {
+		if (WARN_ON_ONCE(!pfn_valid(pfn)))
+			return false;
+		page = pfn_to_page(pfn);
+
+		for (j = 0; j < PAGES_PER_SECTION; j++) {
+			if (PageReserved(page + j))
+				continue;
+
+			printk(KERN_WARNING "section number %ld page number %d "
+				"not reserved, was it already online?\n",
+				pfn_to_section_nr(pfn), j);
+
+			return false;
+		}
+	}
+
+	return true;
+}
+
+/*
  * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
  * OK to have direct references to sparsemem variables in here.
  */
 static int
 memory_block_action(unsigned long phys_index, unsigned long action)
 {
-	int i;
 	unsigned long start_pfn, start_paddr;
 	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
 	struct page *first_page;

@@ -238,26 +273,13 @@ memory_block_action(unsigned long phys_index, unsigned long action)
 
 	first_page = pfn_to_page(phys_index << PFN_SECTION_SHIFT);
 
-	/*
-	 * The probe routines leave the pages reserved, just
-	 * as the bootmem code does.  Make sure they're still
-	 * that way.
-	 */
-	if (action == MEM_ONLINE) {
-		for (i = 0; i < nr_pages; i++) {
-			if (PageReserved(first_page+i))
-				continue;
-
-			printk(KERN_WARNING "section number %ld page number %d "
-				"not reserved, was it already online?\n",
-				phys_index, i);
-			return -EBUSY;
-		}
-	}
-
 	switch (action) {
 		case MEM_ONLINE:
 			start_pfn = page_to_pfn(first_page);
+
+			if (!pages_correctly_reserved(start_pfn, nr_pages))
+				return -EBUSY;
+
 			ret = online_pages(start_pfn, nr_pages);
 			break;
 		case MEM_OFFLINE:

Quoted 32 lines · Andrew Morton

Quoted 19 lines · On Mon, 10 Oct 2011 16:24:03 -0700 Greg KH <[email protected]> wrote:

Quoted 18 lines · Andrew Morton

Quoted 14 lines · On Mon, 10 Oct 2011 08:11:19 +0100 Mel Gorman <[email protected]> wrote:

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on !VMEMMAP configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

Quoted 6 lines

Quoted 3 lines

Nathan's earlier version of this patch is already in linux-next, via Greg. We should drop the old version and get the new one merged instead.

Ok, care to send me what exactly needs to be reverted and what needs to be added?

Drop

commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd

Quoted 8 lines

Author: Nathan Fontenot <[email protected]>
Date:   Mon Sep 26 10:22:33 2011 -0500

memory hotplug: Correct page reservation checking

and replace it with start-of-this-thread.

That's assuming that Mel's update passes Nathan's review and testing :)

It passed review and testing with IBM based on a SUSE bug. I thought Nathan's patch had been lost as it was posted to linuxppc-dev instead of linux-mm. This rework was to improve the changelog and readability.

David correctly pointed out a bug that passed testing because it was still checking one page per section. As long as that page was reserved, memory hot-add would go ahead. Here is a corrected version.

Thanks

==== CUT HERE ====
mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on those configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

[Check pages within sections properly: [email protected]]

[original patch by: [email protected]]

Signed-off-by Mel Gorman <[email protected]>

---
drivers/base/memory.c | 58 +++++++++++++++++++++++++++++++++---------------

1 files changed, 40 insertions(+), 18 deletions(-)

drivers/base/memory.c +36 -0

index 2840ed4..ffb69cd 100644
@@ -224,13 +224,48 @@ int memory_isolate_notify(unsigned long val, void *v)
 }
 
 /*
+ * The probe routines leave the pages reserved, just as the bootmem code does.
+ * Make sure they're still that way.
+ */
+static bool pages_correctly_reserved(unsigned long start_pfn,
+                    unsigned long nr_pages)
+{
+    int i, j;
+    struct page *page;
+    unsigned long pfn = start_pfn;
+
+    /*
+     * memmap between sections is not contiguous except with
+     * SPARSEMEM_VMEMMAP. We lookup the page once per section
+     * and assume memmap is contiguous within each section
+     */
+    for (i = 0; i < sections_per_block; i++, pfn += PAGES_PER_SECTION) {
+        if (WARN_ON_ONCE(!pfn_valid(pfn)))
+            return false;
+        page = pfn_to_page(pfn);
+
+        for (j = 0; j < PAGES_PER_SECTION; j++) {
+            if (PageReserved(page + j))
+                continue;
+
+            printk(KERN_WARNING "section number %ld page number %d "
+                "not reserved, was it already online?\n",
+                pfn_to_section_nr(pfn), j);
+
+            return false;
+        }
+    }
+
+    return true;
+}
+
+/*
 * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
 * OK to have direct references to sparsemem variables in here.
 */

static int
memory_block_action(unsigned long phys_index, unsigned long action) {

drivers/base/memory.c +4 -18

-    int i;
   unsigned long start_pfn, start_paddr;
   unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
   struct page *first_page;

@@ -238,26 +273,13 @@ memory_block_action(unsigned long phys_index, unsigned long action)
 
   first_page = pfn_to_page(phys_index << PFN_SECTION_SHIFT);
 
-    /*
-     * The probe routines leave the pages reserved, just
-     * as the bootmem code does.  Make sure they're still
-     * that way.
-     */
-    if (action == MEM_ONLINE) {
-        for (i = 0; i < nr_pages; i++) {
-            if (PageReserved(first_page+i))
-                continue;
-
-            printk(KERN_WARNING "section number %ld page number %d "
-                "not reserved, was it already online?\n",
-                phys_index, i);
-            return -EBUSY;
-        }
-    }
-
   switch (action) {
       case MEM_ONLINE:
           start_pfn = page_to_pfn(first_page);
+
+            if (!pages_correctly_reserved(start_pfn, nr_pages))
+                return -EBUSY;
+
           ret = online_pages(start_pfn, nr_pages);
           break;
       case MEM_OFFLINE:

(Resending as I am not seeing it in -next so maybe it got lost)

mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on those configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

[Check pages within sections properly: [email protected]]

[original patch by: [email protected]]

Signed-off-by Mel Gorman <[email protected]>

(Resending as I am not seeing it in -next so maybe it got lost)

mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

It is expected that memory being brought online is PageReserved similar to what happens when the page allocator is being brought up. Memory is onlined in "memory blocks" which consist of one or more sections. Unfortunately, the code that verifies PageReserved is currently assuming that the memmap backing all these pages is virtually contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set. As a result, memory hot-add is failing on those configurations with the message;

kernel: section number XXX page number 256 not reserved, was it already online?

This patch updates the PageReserved check to lookup struct page once per section to guarantee the correct struct page is being checked.

[Check pages within sections properly: [email protected]]

[original patch by: [email protected]]

Signed-off-by Mel Gorman <[email protected]>