Re: [PATCH] [MIPS] Hibernation: only save pages in system ram

From: Ralf Baechle
Date: Thu Jul 02 2009 - 19:22:46 EST


On Tue, Jun 30, 2009 at 10:52:50PM +0800, Wu Zhangjin wrote:

> From: Wu Zhangjin <wuzj@xxxxxxxxxx>
>
> when using hibernation(STD) with CONFIG_FLATMEM in linux-mips-64bit, it
> fails for the current mips-specific hibernation implementation save the
> pages in all of the memory space(except the nosave section) and make
> there will be not enough memory left to the STD task itself, and then
> fail. in reality, we only need to save the pages in system rams.
>
> here is the reason why it fail:
>
> kernel/power/snapshot.c:
>
> static void mark_nosave_pages(struct memory_bitmap *bm)
> {
> ...
> if (pfn_valid(pfn)) {
> ...
> }
> }
>
> arch/mips/include/asm/page.h:
>
> ...
> #ifdef CONFIG_FLATMEM
>
> #define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>
> #elif defined(CONFIG_SPARSEMEM)
>
> /* pfn_valid is defined in linux/mmzone.h */
> ...
>
> we can rewrite pfn_valid(pfn) to fix this problem, but I really do not
> want to touch such a widely-used macro, so, I used another solution:
>
> static struct page *saveable_page(struct zone *zone, unsigned long pfn)
> {
> ...
> if ( .... pfn_is_nosave(pfn)
> return NULL;
> ...
> }
>
> and pfn_is_nosave is implemented in arch/mips/power/cpu.c, so, hacking
> this one is better.

No - pfn_valid() is broken, so it should be fixed. Commit
752fbeb2e3555c0d236e992f1195fd7ce30e728d introduced the breakage. It
seemed to assume that the valid range for PFNs doesn't start at 0 but
some higher number but got that entirely wrong..

#define ARCH_PFN_OFFSET PFN_UP(PHYS_OFFSET)
#define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)

works nicely when PHYS_OFFSET is 0 - as for most MIPS systems and goes
horribly wrong otherwise. So I suggest below patch.

Ralf

Signed-off-by: Ralf Baechle <ralf@xxxxxxxxxxxxxx>

arch/mips/include/asm/page.h | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index dc0eaa7..96a14a4 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -165,7 +165,14 @@ typedef struct { unsigned long pgprot; } pgprot_t;

#ifdef CONFIG_FLATMEM

-#define pfn_valid(pfn) ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
+#define pfn_valid(pfn) \
+({ \
+ unsigned long __pfn = (pfn); \
+ /* avoid <linux/bootmem.h> include hell */ \
+ extern unsigned long min_low_pfn; \
+ \
+ __pfn >= min_low_pfn && __pfn < max_mapnr; \
+})

#elif defined(CONFIG_SPARSEMEM)

--
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/