Re: [PATCH 2/3] swsusp i386 mark special saveable/unsaveable pages
From: Shaohua Li
Date: Wed Apr 19 2006 - 23:09:50 EST
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@xxxxxxxxx>
---
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);
}
}
_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/