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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <1246593909.27828.193.camel@falcon>
Date:	Fri, 03 Jul 2009 12:05:09 +0800
From:	Wu Zhangjin <wuzhangjin@...il.com>
To:	Ralf Baechle <ralf@...ux-mips.org>
Cc:	LKML <linux-kernel@...r.kernel.org>, linux-mips@...ux-mips.org,
	Pavel Machek <pavel@....cz>, "Rafael J. Wysocki" <rjw@...k.pl>,
	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Subject: Re: [PATCH] [MIPS] Hibernation: only save pages in system ram

On Fri, 2009-07-03 at 11:21 +0800, Wu Zhangjin wrote:
> On Fri, 2009-07-03 at 00:22 +0100, Ralf Baechle wrote:
> > On Tue, Jun 30, 2009 at 10:52:50PM +0800, Wu Zhangjin wrote:
> > 
> > > From: Wu Zhangjin <wuzj@...ote.com>
> > > 
> > > 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.
> 
> Just test it with the latest master branch of linux-mips on fulong2e,
> Your patch works well, thanks!
> 
> (BTW: of course, the ide-relative bug is also there even with Bart's
> patch, so, currently, I just revert this part of the ide patch: "ide:
> improve handling of Power Management requests" to test your patch: 
> 
> diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
> index d5f3c77..8c4608c 100644
> --- a/drivers/ide/ide-io.c
> +++ b/drivers/ide/ide-io.c
> @@ -468,12 +468,12 @@ void do_ide_request(struct request_queue *q)
>                 ide_hwif_t *prev_port;
>  repeat:
>                 prev_port = hwif->host->cur_port;
> -
> +/*
>                 if (drive->dev_flags & IDE_DFLAG_BLOCKED)
>                         rq = hwif->rq;
>                 else
>                         WARN_ON_ONCE(hwif->rq);
> -
> +*/
>                 if (drive->dev_flags & IDE_DFLAG_SLEEPING &&
>                     time_after(drive->sleep, jiffies)) {
>                         ide_unlock_port(hwif);
> 
> So, I will try to test Bart's patch with linux-next, I'm cloning it
> currently! perhaps there is something missing in my git repo or the
> linux-mips git repo? because i can not apply bart's patch neither to 
> git repo nor to the master branch of linux-mips git repo directly, even
> if pulled linux-next and Davie's ide-2.6 in.

Sorry, I have missed something in Bart's patch, Can apply it now, so
please ignore the above complaint.

but unfortunately, Bart's patch also not fix the problem of STD, I have
reported my test result in another E-mail thread: "[Bug #13663] suspend
to ram regression (IDE related)", so, please ignore this thread, thanks!

Regards,
Wu Zhangjin

> )
> 
> Regards,
> Wu Zhangjin
> 
> > 
> >   Ralf
> > 
> > Signed-off-by: Ralf Baechle <ralf@...ux-mips.org>
> > 
> >  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/