Free swap suspend from dependency on PageReserved

PATCH In review Nigel Cunningham 4 participants 6 messages Oct 5, 2005 – Oct 7, 2005

Subsystems (3)

Memory management Suspend to RAM Hibernation

lore

Share & tools

lore

Nigel Cunningham

author

2005-10-05 21:06 UTC #0 lore raw

PATCH 1/1 Free swap suspend from dependency on PageReserved

From: Nigel Cunningham <[email protected]>

This patch removes the dependency that swap suspend currently has on PageReserved. In the places where PageReserved is currently set and cleared, we also set and clear PageNosave, and in swap suspend itself, we only reference PageNosave. The ongoing effort at freeing PageReserved thus achieves another step forward.

Signed-off-by Nigel Cunningham <[email protected]>

 arch/i386/mm/init.c   |   36 ++++++++++++++++++++++++++++--------
 arch/ppc/mm/init.c    |   22 +++++++++++++++++++---
 arch/x86_64/mm/init.c |    2 ++
 kernel/power/swsusp.c |    7 +++----
 mm/bootmem.c          |    4 ++++
 5 files changed, 56 insertions(+), 15 deletions(-)

arch/i386/mm/init.c +28 -8

diff -ruNp 3120-e820-table-support.patch-old/arch/i386/mm/init.c 3120-e820-table-support.patch-new/arch/i386/mm/init.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/proc_fs.h>
 #include <linux/efi.h>
+#include <linux/suspend.h>
 
 #include <asm/processor.h>
 #include <asm/system.h>

@@ -270,11 +271,14 @@ void __init one_highpage_init(struct pag
 {
 	if (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))) {
 		ClearPageReserved(page);
+		ClearPageNosave(page);
 		set_page_count(page, 1);
 		__free_page(page);
 		totalhigh_pages++;
-	} else
+	} else {
 		SetPageReserved(page);
+		SetPageNosave(page);
+	}
 }
 
 #ifdef CONFIG_NUMA

@@ -353,7 +357,7 @@ static void __init pagetable_init (void)
 #endif
 }
 
-#ifdef CONFIG_SOFTWARE_SUSPEND
+#ifdef CONFIG_PM
 /*
  * Swap suspend & friends need this for resume because things like the intel-agp
  * driver might have split up a kernel 4MB mapping.

@@ -540,6 +544,7 @@ void __init mem_init(void)
 	int codesize, reservedpages, datasize, initsize;
 	int tmp;
 	int bad_ppro;
+	void * addr;
 
 #ifdef CONFIG_FLATMEM
 	if (!mem_map)

@@ -570,12 +575,25 @@ void __init mem_init(void)
 	totalram_pages += free_all_bootmem();
 
 	reservedpages = 0;
-	for (tmp = 0; tmp < max_low_pfn; tmp++)
-		/*
-		 * Only count reserved RAM pages
-		 */
-		if (page_is_ram(tmp) && PageReserved(pfn_to_page(tmp)))
-			reservedpages++;
+	addr = __va(0);
+	for (tmp = 0; tmp < max_low_pfn; tmp++, addr += PAGE_SIZE) {
+		if (page_is_ram(tmp)) {
+			/*
+			 * Only count reserved RAM pages
+			 */
+			if (PageReserved(mem_map+tmp))
+				reservedpages++;
+			/*
+			 * Mark nosave pages
+			 */
+			if (addr >= (void *)&__nosave_begin && addr < (void *)&__nosave_end)
+				SetPageNosave(mem_map+tmp);
+		} else
+			/*
+			 * Non-RAM pages are always nosave
+			 */
+			SetPageNosave(mem_map+tmp);
+	}
 
 	set_highmem_pages_init(bad_ppro);

@@ -674,6 +692,7 @@ void free_initmem(void)
 	addr = (unsigned long)(&__init_begin);
 	for (; addr < (unsigned long)(&__init_end); addr += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(addr));
+		ClearPageNosave(virt_to_page(addr));
 		set_page_count(virt_to_page(addr), 1);
 		memset((void *)addr, 0xcc, PAGE_SIZE);
 		free_page(addr);

@@ -689,6 +708,7 @@ void free_initrd_mem(unsigned long start
 		printk (KERN_INFO "Freeing initrd memory: %ldk freed\n", (end - start) >> 10);
 	for (; start < end; start += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(start));
+		ClearPageNosave(virt_to_page(start));
 		set_page_count(virt_to_page(start), 1);
 		free_page(start);
 		totalram_pages++;

arch/ppc/mm/init.c +19 -3

diff -ruNp 3120-e820-table-support.patch-old/arch/ppc/mm/init.c 3120-e820-table-support.patch-new/arch/ppc/mm/init.c
@@ -32,6 +32,7 @@
 #include <linux/highmem.h>
 #include <linux/initrd.h>
 #include <linux/pagemap.h>
+#include <linux/suspend.h>
 
 #include <asm/pgalloc.h>
 #include <asm/prom.h>

@@ -147,6 +148,7 @@ static void free_sec(unsigned long start
 
 	while (start < end) {
 		ClearPageReserved(virt_to_page(start));
+		ClearPageNosave(virt_to_page(start));
 		set_page_count(virt_to_page(start), 1);
 		free_page(start);
 		cnt++;

@@ -187,6 +189,7 @@ void free_initrd_mem(unsigned long start
 
 	for (; start < end; start += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(start));
+		ClearPageNosave(virt_to_page(start));
 		set_page_count(virt_to_page(start), 1);
 		free_page(start);
 		totalram_pages++;

@@ -422,8 +425,10 @@ void __init mem_init(void)
 	/* if we are booted from BootX with an initial ramdisk,
 	   make sure the ramdisk pages aren't reserved. */
 	if (initrd_start) {
-		for (addr = initrd_start; addr < initrd_end; addr += PAGE_SIZE)
+		for (addr = initrd_start; addr < initrd_end; addr += PAGE_SIZE) {
 			ClearPageReserved(virt_to_page(addr));
+			ClearPageNosave(virt_to_page(addr));
+		}
 	}
 #endif /* CONFIG_BLK_DEV_INITRD */

@@ -432,17 +437,27 @@ void __init mem_init(void)
 	if ( rtas_data )
 		for (addr = (ulong)__va(rtas_data);
 		     addr < PAGE_ALIGN((ulong)__va(rtas_data)+rtas_size) ;
-		     addr += PAGE_SIZE)
+		     addr += PAGE_SIZE) {
 			SetPageReserved(virt_to_page(addr));
+			SetPageNosave(virt_to_page(addr));
+		}
 #endif
 #ifdef CONFIG_PPC_PMAC
-	if (agp_special_page)
+	if (agp_special_page) {
 		SetPageReserved(virt_to_page(agp_special_page));
+		SetPageNosave(virt_to_page(agp_special_page));
+	}
 #endif
 	for (addr = PAGE_OFFSET; addr < (unsigned long)high_memory;
 	     addr += PAGE_SIZE) {
 		if (!PageReserved(virt_to_page(addr)))
 			continue;
+		/*
+		 * Mark nosave pages
+		 */
+		if (addr >= (void *)&__nosave_begin && addr < (void *)&__nosave_end)
+			SetPageNosave(virt_to_page(addr));
+
 		if (addr < (ulong) etext)
 			codepages++;
 		else if (addr >= (unsigned long)&__init_begin

@@ -460,6 +475,7 @@ void __init mem_init(void)
 			struct page *page = mem_map + pfn;
 
 			ClearPageReserved(page);
+			ClearPageNosave(page);
 			set_page_count(page, 1);
 			__free_page(page);
 			totalhigh_pages++;

arch/x86_64/mm/init.c +2 -0

diff -ruNp 3120-e820-table-support.patch-old/arch/x86_64/mm/init.c 3120-e820-table-support.patch-new/arch/x86_64/mm/init.c
@@ -458,6 +458,7 @@ void free_initmem(void)
 	addr = (unsigned long)(&__init_begin);
 	for (; addr < (unsigned long)(&__init_end); addr += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(addr));
+		ClearPageNosave(virt_to_page(addr));
 		set_page_count(virt_to_page(addr), 1);
 		memset((void *)(addr & ~(PAGE_SIZE-1)), 0xcc, PAGE_SIZE); 
 		free_page(addr);

@@ -475,6 +476,7 @@ void free_initrd_mem(unsigned long start
 	printk ("Freeing initrd memory: %ldk freed\n", (end - start) >> 10);
 	for (; start < end; start += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(start));
+		ClearPageNosave(virt_to_page(start));
 		set_page_count(virt_to_page(start), 1);
 		free_page(start);
 		totalram_pages++;

kernel/power/swsusp.c +3 -4

diff -ruNp 3120-e820-table-support.patch-old/kernel/power/swsusp.c 3120-e820-table-support.patch-new/kernel/power/swsusp.c
@@ -672,13 +672,12 @@ static int saveable(struct zone * zone, 
 		return 0;
 
 	page = pfn_to_page(pfn);
-	BUG_ON(PageReserved(page) && PageNosave(page));
-	if (PageNosave(page))
-		return 0;
-	if (PageReserved(page) && pfn_is_nosave(pfn)) {
+	if (pfn_is_nosave(pfn)) {
 		pr_debug("[nosave pfn 0x%lx]", pfn);
 		return 0;
 	}
+	if (PageNosave(page))
+		return 0;
 	if (PageNosaveFree(page))
 		return 0;

mm/bootmem.c +4 -0

diff -ruNp 3120-e820-table-support.patch-old/mm/bootmem.c 3120-e820-table-support.patch-new/mm/bootmem.c
@@ -291,12 +291,14 @@ static unsigned long __init free_all_boo
 			page = pfn_to_page(pfn);
 			count += BITS_PER_LONG;
 			__ClearPageReserved(page);
+			ClearPageNosave(page);
 			order = ffs(BITS_PER_LONG) - 1;
 			set_page_refs(page, order);
 			for (j = 1; j < BITS_PER_LONG; j++) {
 				if (j + 16 < BITS_PER_LONG)
 					prefetchw(page + j + 16);
 				__ClearPageReserved(page + j);
+				ClearPageNosave(page + j);
 			}
 			__free_pages(page, order);
 			i += BITS_PER_LONG;

@@ -309,6 +311,7 @@ static unsigned long __init free_all_boo
 				if (v & m) {
 					count++;
 					__ClearPageReserved(page);
+					ClearPageNosave(page);
 					set_page_refs(page, 0);
 					__free_page(page);
 				}

@@ -329,6 +332,7 @@ static unsigned long __init free_all_boo
 	for (i = 0; i < ((bdata->node_low_pfn-(bdata->node_boot_start >> PAGE_SHIFT))/8 + PAGE_SIZE-1)/PAGE_SIZE; i++,page++) {
 		count++;
 		__ClearPageReserved(page);
+		ClearPageNosave(page);
 		set_page_count(page, 1);
 		__free_page(page);
 	}

Dave Hansen

2005-10-05 22:03 UTC #1 lore raw

arch/i386/mm/init.c void __init mem_init(void) · line 585

Quoted 8 lines · Nigel Cunningham

arch/i386/mm/init.c +7 -0

+       for (tmp = 0; tmp < max_low_pfn; tmp++, addr += PAGE_SIZE) {
+               if (page_is_ram(tmp)) {
+                       /*
+                        * Only count reserved RAM pages
+                        */
+                       if (PageReserved(mem_map+tmp))
+                               reservedpages++;

Please don't reference mem_map[] directly outside of #ifdef CONFIG_FLATMEM areas. It is not defined for all config cases. Please use pfn_to_page(), instead. It should work in all cases where the page is valid.

Also, instead of keeping addr defined like you do, and comparing it during each run of the loop, why not just use pfn_is_nosave(), which is already defined? Then, you won't even need the variable.

-- Dave

Nigel Cunningham

author

2005-10-05 22:26 UTC #2 lore raw

Hi Dave.

Quoted 10 lines · Dave Hansen

arch/i386/mm/init.c void __init mem_init(void) · line 585

Quoted 8 lines · Nigel Cunningham

arch/i386/mm/init.c +7 -0

+       for (tmp = 0; tmp < max_low_pfn; tmp++, addr += PAGE_SIZE) {
+               if (page_is_ram(tmp)) {
+                       /*
+                        * Only count reserved RAM pages
+                        */
+                       if (PageReserved(mem_map+tmp))
+                               reservedpages++;

Quoted 8 lines

Please don't reference mem_map[] directly outside of #ifdef CONFIG_FLATMEM areas. It is not defined for all config cases. Please use pfn_to_page(), instead. It should work in all cases where the page is valid.
Also, instead of keeping addr defined like you do, and comparing it during each run of the loop, why not just use pfn_is_nosave(), which is already defined? Then, you won't even need the variable.

Thanks for the comments. Will revise and resend.

Regards,

Nigel

Quoted 1 line

-- Dave

Rafael J. Wysocki

2005-10-06 08:27 UTC #3 lore raw

Hi Nigel,

Quoted 18 lines · Nigel Cunningham

Hi Dave.
Quoted 15 lines · Dave Hansen
arch/i386/mm/init.c void __init mem_init(void) · line 585
Quoted 8 lines · Nigel Cunningham
arch/i386/mm/init.c +7 -0
+       for (tmp = 0; tmp < max_low_pfn; tmp++, addr += PAGE_SIZE) {
+               if (page_is_ram(tmp)) {
+                       /*
+                        * Only count reserved RAM pages
+                        */
+                       if (PageReserved(mem_map+tmp))
+                               reservedpages++;
Please don't reference mem_map[] directly outside of #ifdef CONFIG_FLATMEM areas. It is not defined for all config cases. Please use pfn_to_page(), instead. It should work in all cases where the page is valid.

Quoted 5 lines

Quoted 3 lines
Also, instead of keeping addr defined like you do, and comparing it during each run of the loop, why not just use pfn_is_nosave(), which is already defined? Then, you won't even need the variable.
Thanks for the comments. Will revise and resend.

There's one more reference to PageReserved() in swsusp.c, which is in save_highmem_zone(). You seem to omit this one (or I have missed it in your patch in which case please ignore this message). Is that intentional?

Rafael

Nick Piggin

2005-10-06 10:13 UTC #4 lore raw

In reply to Nigel Cunningham

Quoted 8 lines · Nigel Cunningham

From: Nigel Cunningham <[email protected]>
This patch removes the dependency that swap suspend currently has on PageReserved. In the places where PageReserved is currently set and cleared, we also set and clear PageNosave, and in swap suspend itself, we only reference PageNosave. The ongoing effort at freeing PageReserved thus achieves another step forward.

Any reason you can't use page_is_ram directly? I would rather you do this than moving swsusp specific flags out into the wider tree. The reason is that these flags now become just as hard to kill as PageReserved is.

You'll have to slightly modify i386's page_is_ram, because it appears that you'll actually want

  'page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))'

Thanks, glad someone is looking into this!

Nick

Signature · 3 lines

SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com

Nigel Cunningham

author

2005-10-07 05:35 UTC #5 lore raw

Hi Nick.

Quoted 15 lines · Nick Piggin

Quoted 8 lines · Nigel Cunningham
From: Nigel Cunningham <[email protected]>
This patch removes the dependency that swap suspend currently has on PageReserved. In the places where PageReserved is currently set and cleared, we also set and clear PageNosave, and in swap suspend itself, we only reference PageNosave. The ongoing effort at freeing PageReserved thus achieves another step forward.
Any reason you can't use page_is_ram directly? I would rather you do this than moving swsusp specific flags out into the wider tree. The reason is that these flags now become just as hard to kill as PageReserved is.

Quoted 4 lines

You'll have to slightly modify i386's page_is_ram, because it appears that you'll actually want
   'page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))'

Thanks for the suggestion!

I suppose we could do something like what you are suggesting. I'll take a look. I'm not promising to do it immediately though - too busy trying to finish off Suspend2 at the mo.

Quoted 1 line

Thanks, glad someone is looking into this!

Always glad to be of service!

Nigel

Quoted 1 line

Nick