Suspend2 Merge: e820 table support.

Suspend2 Merge: e820 table support.

[Nigel Cunningham]

Hi.

This patch adds support for the e820 table for swsusp and Suspend2. It
does so by setting the NoSave flag for unsavable pages at boot time.

Regards,

Nigel

diff -ruN linux-2.6.9-rc1/arch/i386/mm/init.c software-suspend-linux-2.6.9-rc1-rev3/arch/i386/mm/init.c
--- linux-2.6.9-rc1/arch/i386/mm/init.c	2004-09-07 21:58:22.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/arch/i386/mm/init.c	2004-09-09 19:36:24.000000000 +1000
@@ -27,6 +27,9 @@
 #include <linux/slab.h>
 #include <linux/proc_fs.h>
 #include <linux/efi.h>
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+#include <linux/suspend-common.h>
+#endif
 
 #include <asm/processor.h>
 #include <asm/system.h>
@@ -264,12 +267,19 @@
 {
 	if (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))) {
 		ClearPageReserved(page);
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+		ClearPageNosave(page);
+#endif
 		set_bit(PG_highmem, &page->flags);
 		set_page_count(page, 1);
 		__free_page(page);
 		totalhigh_pages++;
-	} else
+	} else {
 		SetPageReserved(page);
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+		SetPageNosave(page);
+#endif
+	}
 }
 
 #ifndef CONFIG_DISCONTIGMEM
@@ -347,7 +357,7 @@
 #endif
 }
 
-#if defined(CONFIG_PM_DISK) || defined(CONFIG_SOFTWARE_SUSPEND)
+#if defined(CONFIG_PM_DISK) || defined(CONFIG_SOFTWARE_SUSPEND) || defined(CONFIG_SOFTWARE_SUSPEND2)
 /*
  * Swap suspend & friends need this for resume because things like the intel-agp
  * driver might have split up a kernel 4MB mapping.
@@ -567,6 +577,7 @@
 	int codesize, reservedpages, datasize, initsize;
 	int tmp;
 	int bad_ppro;
+	void * addr;
 
 #ifndef CONFIG_DISCONTIGMEM
 	if (!mem_map)
@@ -597,12 +608,29 @@
 	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++;
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+			/*
+			 * 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);
+#else
+		}
+#endif
+	}
 
 	set_highmem_pages_init(bad_ppro);
 
@@ -701,6 +729,9 @@
 	addr = (unsigned long)(&__init_begin);
 	for (; addr < (unsigned long)(&__init_end); addr += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(addr));
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+		ClearPageNosave(virt_to_page(addr));
+#endif
 		set_page_count(virt_to_page(addr), 1);
 		free_page(addr);
 		totalram_pages++;
@@ -715,9 +746,15 @@
 		printk (KERN_INFO "Freeing initrd memory: %ldk freed\n", (end - start) >> 10);
 	for (; start < end; start += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(start));
+#ifdef CONFIG_SOFTWARE_SUSPEND2
+		ClearPageNosave(virt_to_page(start));
+#endif
 		set_page_count(virt_to_page(start), 1);
 		free_page(start);
 		totalram_pages++;
 	}
 }
 #endif
+
+/* Exported for Software Suspend 2 */
+EXPORT_SYMBOL(highstart_pfn);
diff -ruN linux-2.6.9-rc1/mm/bootmem.c software-suspend-linux-2.6.9-rc1-rev3/mm/bootmem.c
--- linux-2.6.9-rc1/mm/bootmem.c	2004-09-07 21:59:01.000000000 +1000
+++ software-suspend-linux-2.6.9-rc1-rev3/mm/bootmem.c	2004-09-09 19:36:24.000000000 +1000
@@ -275,6 +275,7 @@
 				if (v & m) {
 					count++;
 					ClearPageReserved(page);
+					ClearPageNosave(page);
 					set_page_count(page, 1);
 					__free_page(page);
 				}
@@ -295,6 +296,7 @@
 	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);
 	}

-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.

Re: Suspend2 Merge: e820 table support.

[Pavel Machek]

Hi!

> This patch adds support for the e820 table for swsusp and Suspend2. It
> does so by setting the NoSave flag for unsavable pages at boot time.

> @@ -27,6 +27,9 @@
>  #include <linux/slab.h>
>  #include <linux/proc_fs.h>
>  #include <linux/efi.h>
> +#ifdef CONFIG_SOFTWARE_SUSPEND2
> +#include <linux/suspend-common.h>
> +#endif
>  
>  #include <asm/processor.h>
>  #include <asm/system.h>
> @@ -264,12 +267,19 @@
>  {
>  	if (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))) {
>  		ClearPageReserved(page);
> +#ifdef CONFIG_SOFTWARE_SUSPEND2
> +		ClearPageNosave(page);
> +#endif
>  		set_bit(PG_highmem, &page->flags);
>  		set_page_count(page, 1);
>  		__free_page(page);
>  		totalhigh_pages++;
> -	} else
> +	} else {
>  		SetPageReserved(page);
> +#ifdef CONFIG_SOFTWARE_SUSPEND2
> +		SetPageNosave(page);
> +#endif
> +	}
>  }
>  
>  #ifndef CONFIG_DISCONTIGMEM

Please, do not put ifdefs around #includes and statements like
ClearPageNosave. (And is it neccessary at all? I'd just say that all
pages that are Reserved are Nosave automatically.)

> +#ifdef CONFIG_SOFTWARE_SUSPEND2
> +			/*
> +			 * 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);
> +#else
> +		}
> +#endif
> +	}

Current -mm code does something funny with Nosave; I'm not sure it
will not try to free them after resume. I have fix in my tree but it
is little tested.
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: Suspend2 Merge: e820 table support.

[Nigel Cunningham]

Hi.

On Thu, 2004-09-16 at 21:14, Pavel Machek wrote:
> Please, do not put ifdefs around #includes and statements like

Ah. Sorry. Will correct.

> ClearPageNosave. (And is it neccessary at all? I'd just say that all
> pages that are Reserved are Nosave automatically.)

Hmm. Long time since I thought about that. I'll check.

> > +#ifdef CONFIG_SOFTWARE_SUSPEND2
> > +			/*
> > +			 * 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);
> > +#else
> > +		}
> > +#endif
> > +	}
> 
> Current -mm code does something funny with Nosave; I'm not sure it
> will not try to free them after resume. I have fix in my tree but it
> is little tested.

Double negative: you think current mm may try to free Nosave pages after
resume? I haven't updated to the latest -mm yet, but will see if I can
try tomorrow.

Thanks for the feedback.

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.

Re: Suspend2 Merge: e820 table support.

[Pavel Machek]

Hi!

> > > +#ifdef CONFIG_SOFTWARE_SUSPEND2
> > > +			/*
> > > +			 * 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);
> > > +#else
> > > +		}
> > > +#endif
> > > +	}
> > 
> > Current -mm code does something funny with Nosave; I'm not sure it
> > will not try to free them after resume. I have fix in my tree but it
> > is little tested.
> 
> Double negative: you think current mm may try to free Nosave pages after
> resume? I haven't updated to the latest -mm yet, but will see if I can
> try tomorrow.


Hmm, it also contains (saveable()):

        BUG_ON(PageReserved(page) && PageNosave(page));

..but that should be easy to kill. I'd be worried about this function:

static void free_suspend_pagedir_zone(struct zone *zone, unsigned long
pagedir)
{
        unsigned long zone_pfn, pagedir_end, pagedir_pfn,
pagedir_end_pfn;
        pagedir_end = pagedir + (PAGE_SIZE << pagedir_order);
        pagedir_pfn = __pa(pagedir) >> PAGE_SHIFT;
        pagedir_end_pfn = __pa(pagedir_end) >> PAGE_SHIFT;
        for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
{
                struct page *page;
                unsigned long pfn = zone_pfn + zone->zone_start_pfn;
                if (!pfn_valid(pfn))
                        continue;
                page = pfn_to_page(pfn);
                if (!TestClearPageNosave(page))
                        continue;
                else if (pfn >= pagedir_pfn && pfn < pagedir_end_pfn)
                        continue;
                __free_page(page);
        }
}

I posted diff to get rid of it, but it did not get enough testing so
it is not in mainline.
								Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: Suspend2 Merge: e820 table support.

[Nigel Cunningham]

Hi.

On Thu, 2004-09-16 at 21:27, Pavel Machek wrote:
> Hmm, it also contains (saveable()):
> 
>         BUG_ON(PageReserved(page) && PageNosave(page));

How do you cover those HighMem pages that get marked Reserved and are
unusable? (That's what the e820 logic was for, iirc. Think it was done
about February!). Not handling them resulted in MCEs when trying to do
the atomic copy or when restoring (seemed random).

> ..but that should be easy to kill. I'd be worried about this function:
> 
> static void free_suspend_pagedir_zone(struct zone *zone, unsigned long
> pagedir)
> {
>         unsigned long zone_pfn, pagedir_end, pagedir_pfn,
> pagedir_end_pfn;
>         pagedir_end = pagedir + (PAGE_SIZE << pagedir_order);
>         pagedir_pfn = __pa(pagedir) >> PAGE_SHIFT;
>         pagedir_end_pfn = __pa(pagedir_end) >> PAGE_SHIFT;
>         for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
> {
>                 struct page *page;
>                 unsigned long pfn = zone_pfn + zone->zone_start_pfn;
>                 if (!pfn_valid(pfn))
>                         continue;
>                 page = pfn_to_page(pfn);

Mmm. I should get around to using pfn_to_page. That's necessary for
discontig support, right? Haven't looked at that yet. (Yes, swsusp has
functionality suspend2 doesn't!) :>.

>                 if (!TestClearPageNosave(page))
>                         continue;
>                 else if (pfn >= pagedir_pfn && pfn < pagedir_end_pfn)
>                         continue;
>                 __free_page(page);
>         }
> }

Wow. This function is really hard to understand. Or maybe I'm really
ignorant :>. 

> I posted diff to get rid of it, but it did not get enough testing so
> it is not in mainline.
> 								Pavel

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.

Re: Suspend2 Merge: e820 table support.

[Pavel Machek]

Hi!

> > Hmm, it also contains (saveable()):
> > 
> >         BUG_ON(PageReserved(page) && PageNosave(page));
> 
> How do you cover those HighMem pages that get marked Reserved and are
> unusable? (That's what the e820 logic was for, iirc. Think it was done
> about February!). Not handling them resulted in MCEs when trying to do
> the atomic copy or when restoring (seemed random).

This function is not use for highmem, AFAICS. If page is marked
reserved we do not touch it. Do you suggest that we need to save it
for highmem case?

MCEs... I see you have patch to disable them during suspend... That's
clearly wrong thing to do, right?

> > ..but that should be easy to kill. I'd be worried about this function:
> > 
> > static void free_suspend_pagedir_zone(struct zone *zone, unsigned long
> > pagedir)
> > {
> >         unsigned long zone_pfn, pagedir_end, pagedir_pfn,
> > pagedir_end_pfn;
> >         pagedir_end = pagedir + (PAGE_SIZE << pagedir_order);
> >         pagedir_pfn = __pa(pagedir) >> PAGE_SHIFT;
> >         pagedir_end_pfn = __pa(pagedir_end) >> PAGE_SHIFT;
> >         for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
> > {
> >                 struct page *page;
> >                 unsigned long pfn = zone_pfn + zone->zone_start_pfn;
> >                 if (!pfn_valid(pfn))
> >                         continue;
> >                 page = pfn_to_page(pfn);
> 
> Mmm. I should get around to using pfn_to_page. That's necessary for
> discontig support, right? Haven't looked at that yet. (Yes, swsusp has
> functionality suspend2 doesn't!) :>.
> 
> >                 if (!TestClearPageNosave(page))
> >                         continue;
> >                 else if (pfn >= pagedir_pfn && pfn < pagedir_end_pfn)
> >                         continue;
> >                 __free_page(page);
> >         }
> > }
> 
> Wow. This function is really hard to understand. Or maybe I'm really
> ignorant :>.

No, this function really is ugly and needs to die.

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: Suspend2 Merge: e820 table support.

[Nigel Cunningham]

Hi.

On Thu, 2004-09-16 at 21:37, Pavel Machek wrote:
> Hi!
> 
> > > Hmm, it also contains (saveable()):
> > > 
> > >         BUG_ON(PageReserved(page) && PageNosave(page));
> > 
> > How do you cover those HighMem pages that get marked Reserved and are
> > unusable? (That's what the e820 logic was for, iirc. Think it was done
> > about February!). Not handling them resulted in MCEs when trying to do
> > the atomic copy or when restoring (seemed random).
> 
> This function is not use for highmem, AFAICS. If page is marked
> reserved we do not touch it. Do you suggest that we need to save it
> for highmem case?

I love trying to remember things from six months ago. Will have to look
at the email from around then and get back to you.

> MCEs... I see you have patch to disable them during suspend... That's
> clearly wrong thing to do, right?

Yes, we shouldn't need to disable them if we have above right. I'll test
removing that with some of the people who had MCE issues.

Regards,

Nigel
-- 
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Many today claim to be tolerant. True tolerance, however, can cope with others
being intolerant.