[PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

Pages (Reserved/ACPI NVS/ACPI Data) below max_low_pfn will be
saved/restored by S4 currently. We should mark 'Reserved' pages
not saveable.
Pages (Reserved/ACPI NVS/ACPI Data) above max_low_pfn will not be
saved/restored by S4 currently. We should save the
'ACPI NVS/ACPI Data' pages.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---

 linux-2.6.17-rc1-root/arch/i386/kernel/setup.c |  106 +++++++++++++++++++++++++
 1 files changed, 106 insertions(+)

diff -puN arch/i386/kernel/setup.c~swsusp_i386_save_pages arch/i386/kernel/setup.c
--- linux-2.6.17-rc1/arch/i386/kernel/setup.c~swsusp_i386_save_pages	2006-04-11 08:04:23.000000000 +0800
+++ linux-2.6.17-rc1-root/arch/i386/kernel/setup.c	2006-04-11 08:08:12.000000000 +0800
@@ -48,6 +48,7 @@
 #include <linux/crash_dump.h>
 #include <linux/dmi.h>
 #include <linux/pfn.h>
+#include <linux/suspend.h>
 
 #include <video/edid.h>
 
@@ -1400,6 +1401,111 @@ static void set_mca_bus(int x)
 static void set_mca_bus(int x) { }
 #endif
 
+#ifdef CONFIG_SOFTWARE_SUSPEND
+static void __init mark_nosave_page_range(unsigned long start, unsigned long end)
+{
+	struct page *page;
+	while (start <= end) {
+		page = pfn_to_page(start);
+		SetPageNosave(page);
+		start++;
+	}
+}
+
+static void __init e820_nosave_reserved_pages(void)
+{
+	int i;
+	unsigned long r_start = 0, r_end = 0;
+
+	/* Assume e820 map is sorted */
+	for (i = 0; i < e820.nr_map; i++) {
+		struct e820entry *ei = &e820.map[i];
+		unsigned long start, end;
+
+		start = PFN_DOWN(ei->addr);
+		end = PFN_UP(ei->addr + ei->size);
+		if (start >= end)
+			continue;
+		if (ei->type == E820_RESERVED)
+			continue;
+		r_end = start;
+		/*
+		 * Highmem 'Reserved' pages are marked as reserved, swsusp
+		 * will not save/restore them, so we ignore these pages here.
+		 */
+		if (r_end > max_low_pfn)
+			r_end = max_low_pfn;
+		if (r_end > r_start)
+			mark_nosave_page_range(r_start, r_end-1);
+		if (r_end >= max_low_pfn)
+			break;
+		r_start = end;
+	}
+}
+
+static void __init e820_save_acpi_pages(void)
+{
+	int i;
+
+	/* Assume e820 map is sorted */
+	for (i = 0; i < e820.nr_map; i++) {
+		struct e820entry *ei = &e820.map[i];
+		unsigned long start, end;
+
+		start = PFN_DOWN(ei->addr);
+		end = PFN_UP(ei->addr + ei->size);
+		if (start >= end)
+			continue;
+		if (ei->type != E820_ACPI && ei->type != E820_NVS)
+			continue;
+		/*
+		 * If the region is below max_low_pfn, it will be
+		 * saved/restored by swsusp follow 'RAM' type.
+		 */
+		if (start < max_low_pfn)
+			start = max_low_pfn;
+		/*
+		 * Highmem pages (ACPI NVS/Data) are reserved, but swsusp
+		 * highmem save/restore will not save/restore them. We marked
+		 * them as arch saveable pages here
+		 */
+		if (end > start)
+			swsusp_add_arch_pages(start, end - 1);
+	}
+}
+
+extern char __start_rodata, __end_rodata;
+/*
+ * BIOS reserved region/hole - no save/restore
+ * ACPI NVS - save/restore
+ * ACPI Data - this is a little tricky, the mem could be used by OS after OS
+ * reads tables from the region, but anyway save/restore the memory hasn't any
+ * side effect and Linux runtime module load/unload might use it.
+ * kernel rodata - no save/restore (kernel rodata isn't changed)
+ */
+static int __init mark_nosave_pages(void)
+{
+	unsigned long pfn_start, pfn_end;
+
+	/* FIXME: provide a version for efi BIOS */
+	if (efi_enabled)
+		return 0;
+	/* BIOS reserved regions & holes */
+	e820_nosave_reserved_pages();
+
+	/* kernel rodata */
+	pfn_start = PFN_UP(virt_to_phys(&__start_rodata));
+	pfn_end = PFN_DOWN(virt_to_phys(&__end_rodata));
+	mark_nosave_page_range(pfn_start, pfn_end-1);
+
+	/* record ACPI Data/NVS as saveable */
+	e820_save_acpi_pages();
+
+	return 0;
+}
+core_initcall(mark_nosave_pages);
+#endif
+
 /*
  * Determine if we were loaded by an EFI loader.  If so, then we have also been
  * passed the efi memmap, systab, etc., so we should use these data structures
_

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Hi.

On Wednesday 12 April 2006 12:38, Shaohua Li wrote:
> @@ -1400,6 +1401,111 @@ static void set_mca_bus(int x)
>  static void set_mca_bus(int x) { }
>  #endif
>
> +#ifdef CONFIG_SOFTWARE_SUSPEND
> +static void __init mark_nosave_page_range(unsigned long start, unsigned
> long end) +{
> +	struct page *page;
> +	while (start <= end) {

Should this be start < end? (End is usually the first byte of the next zone 
IIUC).

> +		page = pfn_to_page(start);
> +		SetPageNosave(page);
> +		start++;
> +	}
> +}
> +
> +static void __init e820_nosave_reserved_pages(void)
> +{
> +	int i;

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

On Wed, 2006-04-19 at 08:38 +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Wednesday 12 April 2006 12:38, Shaohua Li wrote:
> > @@ -1400,6 +1401,111 @@ static void set_mca_bus(int x)
> >  static void set_mca_bus(int x) { }
> >  #endif
> >
> > +#ifdef CONFIG_SOFTWARE_SUSPEND
> > +static void __init mark_nosave_page_range(unsigned long start, unsigned
> > long end) +{
> > +	struct page *page;
> > +	while (start <= end) {
> 
> Should this be start < end? (End is usually the first byte of the next zone 
> IIUC).
Thanks for looking at it. Yes you are right. Before calling this
routine, I already decrement 1 for 'end', so the routine will have the
last page pfn.

Thanks,
Shaohua

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

Hi.

On Wednesday 19 April 2006 11:22, Shaohua Li wrote:
> On Wed, 2006-04-19 at 08:38 +1000, Nigel Cunningham wrote:
> > Hi.
> >
> > On Wednesday 12 April 2006 12:38, Shaohua Li wrote:
> > > @@ -1400,6 +1401,111 @@ static void set_mca_bus(int x)
> > >  static void set_mca_bus(int x) { }
> > >  #endif
> > >
> > > +#ifdef CONFIG_SOFTWARE_SUSPEND
> > > +static void __init mark_nosave_page_range(unsigned long start,
> > > unsigned long end) +{
> > > +	struct page *page;
> > > +	while (start <= end) {
> >
> > Should this be start < end? (End is usually the first byte of the next
> > zone IIUC).
>
> Thanks for looking at it. Yes you are right. Before calling this
> routine, I already decrement 1 for 'end', so the routine will have the
> last page pfn.

Ah. I see. In the call itself. Sneaky :)

Would you consider modifying that bit so it doesn't confuse others in the 
future?

Oh, and while we're on the topic, if only part of a page is NVS, what's the 
right behaviour? My e820 table has:

BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Wednesday 19 April 2006 11:22, Shaohua Li wrote:
> > On Wed, 2006-04-19 at 08:38 +1000, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Wednesday 12 April 2006 12:38, Shaohua Li wrote:
> > > > @@ -1400,6 +1401,111 @@ static void set_mca_bus(int x)
> > > >  static void set_mca_bus(int x) { }
> > > >  #endif
> > > >
> > > > +#ifdef CONFIG_SOFTWARE_SUSPEND
> > > > +static void __init mark_nosave_page_range(unsigned long start,
> > > > unsigned long end) +{
> > > > +	struct page *page;
> > > > +	while (start <= end) {
> > >
> > > Should this be start < end? (End is usually the first byte of the next
> > > zone IIUC).
> >
> > Thanks for looking at it. Yes you are right. Before calling this
> > routine, I already decrement 1 for 'end', so the routine will have the
> > last page pfn.
> 
> Ah. I see. In the call itself. Sneaky :)
> 
> Would you consider modifying that bit so it doesn't confuse others in the 
> future?
I'm a little lazy :), but it isn't really confusing anyway.

> Oh, and while we're on the topic, if only part of a page is NVS, what's the 
> right behaviour? My e820 table has:
> 
> BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
If only part of a page is NVS, my patch will save the whole page. Any
other idea?

Thanks,
Shaohua

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 681 bytes --]

Hi.

On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > Oh, and while we're on the topic, if only part of a page is NVS, what's
> > the right behaviour? My e820 table has:
> >
> > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
>
> If only part of a page is NVS, my patch will save the whole page. Any
> other idea?

A device model driver that handles saving just the part of the page, using 
preallocated buffers to avoid the potential allocation problems? (The whole 
page could then safely be Nosave).

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

On Wed, 2006-04-19 at 12:08 +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> > On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > > Oh, and while we're on the topic, if only part of a page is NVS, what's
> > > the right behaviour? My e820 table has:
> > >
> > > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
> >
> > If only part of a page is NVS, my patch will save the whole page. Any
> > other idea?
> 
> A device model driver that handles saving just the part of the page, using 
> preallocated buffers to avoid the potential allocation problems? (The whole 
> page could then safely be Nosave).
The allocation might not be a problem, this just needs one or two extra
pages. A problem is if just part of the page is NVS, could we touch
other part (save/restore) the page.

Thanks,
Shaohua

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1505 bytes --]

Hi.

On Wednesday 19 April 2006 12:53, Shaohua Li wrote:
> On Wed, 2006-04-19 at 12:08 +1000, Nigel Cunningham wrote:
> > Hi.
> >
> > On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> > > On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > > > Oh, and while we're on the topic, if only part of a page is NVS,
> > > > what's the right behaviour? My e820 table has:
> > > >
> > > > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > > > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
> > >
> > > If only part of a page is NVS, my patch will save the whole page. Any
> > > other idea?
> >
> > A device model driver that handles saving just the part of the page,
> > using preallocated buffers to avoid the potential allocation problems?
> > (The whole page could then safely be Nosave).
>
> The allocation might not be a problem, this just needs one or two extra
> pages. A problem is if just part of the page is NVS, could we touch
> other part (save/restore) the page.

Yes, so I was thinking of treating it with a pseudo driver that could save and 
restore just that portion of the page.

Regarding the allocation, I was originally thinking of that other ACPI 
allocation while atomic issue, and trying to avoid another one. I guess this 
is simpler though because we know ahead of time how much is needed (am I 
right in thinking that in the other case, the amount of memory needed isn't 
known ahead of time?).

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

On Wed, 2006-04-19 at 12:59 +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Wednesday 19 April 2006 12:53, Shaohua Li wrote:
> > On Wed, 2006-04-19 at 12:08 +1000, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> > > > On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > > > > Oh, and while we're on the topic, if only part of a page is NVS,
> > > > > what's the right behaviour? My e820 table has:
> > > > >
> > > > > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > > > > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
> > > >
> > > > If only part of a page is NVS, my patch will save the whole page. Any
> > > > other idea?
> > >
> > > A device model driver that handles saving just the part of the page,
> > > using preallocated buffers to avoid the potential allocation problems?
> > > (The whole page could then safely be Nosave).
> >
> > The allocation might not be a problem, this just needs one or two extra
> > pages. A problem is if just part of the page is NVS, could we touch
> > other part (save/restore) the page.
> 
> Yes, so I was thinking of treating it with a pseudo driver that could save and 
> restore just that portion of the page.
Sounds like a good idea. If NVS is already aligned to page size, do you
still use the pseudo driver to save/restore the pages? In my system, the
NVS memory is 512k. 
In the other way, we could let the 'swsusp_add_arch_pages' accept
address instead of a pfn and let snapshot.c handle the partial page
issue.

> Regarding the allocation, I was originally thinking of that other ACPI 
> allocation while atomic issue, and trying to avoid another one. I guess this 
> is simpler though because we know ahead of time how much is needed (am I 
> right in thinking that in the other case, the amount of memory needed isn't 
> known ahead of time?).
Yes, we can get the amount of memory needed ahead per the e820 map.

Thanks,
Shaohua

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Hi.

On Wednesday 19 April 2006 13:28, Shaohua Li wrote:
> On Wed, 2006-04-19 at 12:59 +1000, Nigel Cunningham wrote:
> > Hi.
> >
> > On Wednesday 19 April 2006 12:53, Shaohua Li wrote:
> > > On Wed, 2006-04-19 at 12:08 +1000, Nigel Cunningham wrote:
> > > > Hi.
> > > >
> > > > On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> > > > > On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > > > > > Oh, and while we're on the topic, if only part of a page is NVS,
> > > > > > what's the right behaviour? My e820 table has:
> > > > > >
> > > > > > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > > > > > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
> > > > >
> > > > > If only part of a page is NVS, my patch will save the whole page.
> > > > > Any other idea?
> > > >
> > > > A device model driver that handles saving just the part of the page,
> > > > using preallocated buffers to avoid the potential allocation
> > > > problems? (The whole page could then safely be Nosave).
> > >
> > > The allocation might not be a problem, this just needs one or two extra
> > > pages. A problem is if just part of the page is NVS, could we touch
> > > other part (save/restore) the page.
> >
> > Yes, so I was thinking of treating it with a pseudo driver that could
> > save and restore just that portion of the page.
>
> Sounds like a good idea. If NVS is already aligned to page size, do you
> still use the pseudo driver to save/restore the pages? In my system, the
> NVS memory is 512k.
> In the other way, we could let the 'swsusp_add_arch_pages' accept
> address instead of a pfn and let snapshot.c handle the partial page
> issue.

I guess the cleanest solution would be to use the same routine in either case. 
If that was a pseudo driver, it would mean double the memory usage (the pages 
allocated would also be atomically copied), so perhaps using 
swsusp_add_arch_pages is the way to go.

I wonder too whether Mel Gorman e820 table patches could be leveraged to make 
finding the NVS data really nice and simple?

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages

[Shaohua Li]

On Wed, 2006-04-19 at 16:33 +1000, Nigel Cunningham wrote:
> Hi.
> 
> On Wednesday 19 April 2006 13:28, Shaohua Li wrote:
> > On Wed, 2006-04-19 at 12:59 +1000, Nigel Cunningham wrote:
> > > Hi.
> > >
> > > On Wednesday 19 April 2006 12:53, Shaohua Li wrote:
> > > > On Wed, 2006-04-19 at 12:08 +1000, Nigel Cunningham wrote:
> > > > > Hi.
> > > > >
> > > > > On Wednesday 19 April 2006 11:51, Shaohua Li wrote:
> > > > > > On Wed, 2006-04-19 at 11:41 +1000, Nigel Cunningham wrote:
> > > > > > > Oh, and while we're on the topic, if only part of a page is NVS,
> > > > > > > what's the right behaviour? My e820 table has:
> > > > > > >
> > > > > > > BIOS-e820: 000000003dff0000 - 000000003dffffc0 (ACPI data)
> > > > > > > BIOS-e820: 000000003dffffc0 - 000000003e000000 (ACPI NVS)
> > > > > >
> > > > > > If only part of a page is NVS, my patch will save the whole page.
> > > > > > Any other idea?
> > > > >
> > > > > A device model driver that handles saving just the part of the page,
> > > > > using preallocated buffers to avoid the potential allocation
> > > > > problems? (The whole page could then safely be Nosave).
> > > >
> > > > The allocation might not be a problem, this just needs one or two extra
> > > > pages. A problem is if just part of the page is NVS, could we touch
> > > > other part (save/restore) the page.
> > >
> > > Yes, so I was thinking of treating it with a pseudo driver that could
> > > save and restore just that portion of the page.
> >
> > Sounds like a good idea. If NVS is already aligned to page size, do you
> > still use the pseudo driver to save/restore the pages? In my system, the
> > NVS memory is 512k.
> > In the other way, we could let the 'swsusp_add_arch_pages' accept
> > address instead of a pfn and let snapshot.c handle the partial page
> > issue.
> 
> I guess the cleanest solution would be to use the same routine in either case. 
> If that was a pseudo driver, it would mean double the memory usage (the pages 
> allocated would also be atomically copied), so perhaps using 
> swsusp_add_arch_pages is the way to go.
Ok, fixed by below patch.
> 
> I wonder too whether Mel Gorman e820 table patches could be leveraged to make 
> finding the NVS data really nice and simple?
Sounds it can't benefit us to me.

patch is on top of the swsusp-add-architecture-special-saveable-pages
patches of 2.6.17-rc1-mm3. Andrew, could you please also queue this
patch in -mm for test? Thanks!

architecture special saveable memory might not be aligned to PAGE_SIZE.
We should just save part of a page in the unaligned case. We changed
swsusp_add_arch_pages to accept an 'address' instead of a 'pfn'.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---

 linux-2.6.17-rc1-mm3-root/arch/i386/kernel/setup.c   |   10 ++---
 linux-2.6.17-rc1-mm3-root/arch/x86_64/kernel/setup.c |   10 ++---
 linux-2.6.17-rc1-mm3-root/kernel/power/snapshot.c    |   34 ++++++++++++-------
 3 files changed, 32 insertions(+), 22 deletions(-)

diff -puN kernel/power/snapshot.c~swsusp-add-architecture-special-saveable-pages-fix kernel/power/snapshot.c
--- linux-2.6.17-rc1-mm3/kernel/power/snapshot.c~swsusp-add-architecture-special-saveable-pages-fix	2006-04-18 15:21:45.000000000 +0800
+++ linux-2.6.17-rc1-mm3-root/kernel/power/snapshot.c	2006-04-18 15:27:30.000000000 +0800
@@ -40,8 +40,9 @@ static unsigned int nr_meta_pages;
 static unsigned long *buffer;
 
 struct arch_saveable_page {
-	unsigned long pfn;
-	void *data;
+	unsigned long start;
+	unsigned long end;
+	char *data;
 	struct arch_saveable_page *next;
 };
 static struct arch_saveable_page *arch_pages;
@@ -50,13 +51,16 @@ int swsusp_add_arch_pages(unsigned long 
 {
 	struct arch_saveable_page *tmp;
 
-	while (start <= end) {
+	while (start < end) {
 		tmp = kzalloc(sizeof(struct arch_saveable_page), GFP_KERNEL);
 		if (!tmp)
 			return -ENOMEM;
-		tmp->pfn = start;
+		tmp->start = start;
+		tmp->end = ((start >> PAGE_SHIFT) + 1) << PAGE_SHIFT;
+		if (tmp->end > end)
+			tmp->end = end;
 		tmp->next = arch_pages;
-		start++;
+		start = tmp->end;
 		arch_pages = tmp;
 	}
 	return 0;
@@ -75,17 +79,20 @@ static unsigned int count_arch_pages(voi
 
 static int save_arch_mem(void)
 {
-	void *kaddr;
+	char *kaddr;
 	struct arch_saveable_page *tmp = arch_pages;
+	int offset;
 
 	pr_debug("swsusp: Saving arch specific memory");
 	while (tmp) {
-		tmp->data = (void *)__get_free_page(GFP_ATOMIC);
+		tmp->data = (char *)__get_free_page(GFP_ATOMIC);
 		if (!tmp->data)
 			return -ENOMEM;
+		offset = tmp->start - (tmp->start & PAGE_MASK);
 		/* arch pages might haven't a 'struct page' */
-		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
-		memcpy(tmp->data, kaddr, PAGE_SIZE);
+		kaddr = kmap_atomic_pfn(tmp->start >> PAGE_SHIFT, KM_USER0);
+		memcpy(tmp->data + offset, kaddr + offset,
+			tmp->end - tmp->start);
 		kunmap_atomic(kaddr, KM_USER0);
 
 		tmp = tmp->next;
@@ -95,14 +102,17 @@ static int save_arch_mem(void)
 
 static int restore_arch_mem(void)
 {
-	void *kaddr;
+	char *kaddr;
 	struct arch_saveable_page *tmp = arch_pages;
+	int offset;
 
 	while (tmp) {
 		if (!tmp->data)
 			continue;
-		kaddr = kmap_atomic_pfn(tmp->pfn, KM_USER0);
-		memcpy(kaddr, tmp->data, PAGE_SIZE);
+		offset = tmp->start - (tmp->start & PAGE_MASK);
+		kaddr = kmap_atomic_pfn(tmp->start >> PAGE_SHIFT, KM_USER0);
+		memcpy(kaddr + offset, tmp->data + offset,
+			tmp->end - tmp->start);
 		kunmap_atomic(kaddr, KM_USER0);
 		free_page((long)tmp->data);
 		tmp->data = NULL;
diff -puN arch/i386/kernel/setup.c~swsusp-add-architecture-special-saveable-pages-fix arch/i386/kernel/setup.c
--- linux-2.6.17-rc1-mm3/arch/i386/kernel/setup.c~swsusp-add-architecture-special-saveable-pages-fix	2006-04-18 15:28:00.000000000 +0800
+++ linux-2.6.17-rc1-mm3-root/arch/i386/kernel/setup.c	2006-04-18 15:29:26.000000000 +0800
@@ -1500,8 +1500,8 @@ static void __init e820_save_acpi_pages(
 		struct e820entry *ei = &e820.map[i];
 		unsigned long start, end;
 
-		start = PFN_DOWN(ei->addr);
-		end = PFN_UP(ei->addr + ei->size);
+		start = ei->addr;
+		end = ei->addr + ei->size;
 		if (start >= end)
 			continue;
 		if (ei->type != E820_ACPI && ei->type != E820_NVS)
@@ -1510,15 +1510,15 @@ static void __init e820_save_acpi_pages(
 		 * If the region is below max_low_pfn, it will be
 		 * saved/restored by swsusp follow 'RAM' type.
 		 */
-		if (start < max_low_pfn)
-			start = max_low_pfn;
+		if (start < (max_low_pfn << PAGE_SHIFT))
+			start = max_low_pfn << PAGE_SHIFT;
 		/*
 		 * Highmem pages (ACPI NVS/Data) are reserved, but swsusp
 		 * highmem save/restore will not save/restore them. We marked
 		 * them as arch saveable pages here
 		 */
 		if (end > start)
-			swsusp_add_arch_pages(start, end - 1);
+			swsusp_add_arch_pages(start, end);
 	}
 }
 
diff -puN arch/x86_64/kernel/setup.c~swsusp-add-architecture-special-saveable-pages-fix arch/x86_64/kernel/setup.c
--- linux-2.6.17-rc1-mm3/arch/x86_64/kernel/setup.c~swsusp-add-architecture-special-saveable-pages-fix	2006-04-18 15:28:07.000000000 +0800
+++ linux-2.6.17-rc1-mm3-root/arch/x86_64/kernel/setup.c	2006-04-18 15:31:31.000000000 +0800
@@ -564,8 +564,8 @@ static void __init e820_save_acpi_pages(
 		struct e820entry *ei = &e820.map[i];
 		unsigned long start, end;
 
-		start = round_down(ei->addr, PAGE_SIZE) >> PAGE_SHIFT;
-		end = round_up(ei->addr + ei->size, PAGE_SIZE) >> PAGE_SHIFT;
+		start = ei->addr, PAGE_SIZE;
+		end = ei->addr + ei->size;
 		if (start >= end)
 			continue;
 		if (ei->type != E820_ACPI && ei->type != E820_NVS)
@@ -574,10 +574,10 @@ static void __init e820_save_acpi_pages(
 		 * If the region is below end_pfn, it will be
 		 * saved/restored by swsusp follow 'RAM' type.
 		 */
-		if (start < end_pfn)
-			start = end_pfn;
+		if (start < (end_pfn << PAGE_SHIFT))
+			start = end_pfn << PAGE_SHIFT;
 		if (end > start)
-			swsusp_add_arch_pages(start, end - 1);
+			swsusp_add_arch_pages(start, end);
 	}
 }
 
_