PageReserved removal from swsusp

PageReserved removal from swsusp

[Nick Piggin]

Hi,

kernel/power/swsusp.c is the last remaining real user of PageReserved
with my current PageReserved removal patchset. This is actually not a
problem for my purposes, and swsusp is quite able to continue using
PageReserved... however that may be a bit ugly, and means swsusp will
be the sole user of 3(!) page-flags.

The PageReserved test in save_highmem_zone() is the hard one. rvmalloc
is no problem, but it seems to be important to prevent saving regions
that aren't RAM.

I seem to be able to work around this on i386 by testing page_is_ram()
instead of PageReserved, however that looks nonportable, and at least
on i386 we rather want something like page_is_usable_ram() which is
(page_is_ram && !(bad_ppro && page_kills_ppro))

Otherwise we could perhaps have a PageUsableRAM() which returns
page->flags != 0xffffffff or some other unlikely combination of flags.

They're my two ideas. Anyone else?

-- 
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com 
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: PageReserved removal from swsusp

[Nigel Cunningham]

Hi.

On Mon, 2005-07-25 at 11:38, Nick Piggin wrote:
> Hi,
> 
> kernel/power/swsusp.c is the last remaining real user of PageReserved
> with my current PageReserved removal patchset. This is actually not a
> problem for my purposes, and swsusp is quite able to continue using
> PageReserved... however that may be a bit ugly, and means swsusp will
> be the sole user of 3(!) page-flags.
> 
> The PageReserved test in save_highmem_zone() is the hard one. rvmalloc
> is no problem, but it seems to be important to prevent saving regions
> that aren't RAM.
> 
> I seem to be able to work around this on i386 by testing page_is_ram()
> instead of PageReserved, however that looks nonportable, and at least
> on i386 we rather want something like page_is_usable_ram() which is
> (page_is_ram && !(bad_ppro && page_kills_ppro))
> 
> Otherwise we could perhaps have a PageUsableRAM() which returns
> page->flags != 0xffffffff or some other unlikely combination of flags.
> 
> They're my two ideas. Anyone else?

I have a few differences in Suspend2 that let me only use one real
pageflag (Nosave).

The changes are:

1) Use a set of order zero allocations, tied together via a kmalloc'd
list of pointers as a means of dynamically creating and destroying
pseudo page-flags (such as are only needed while suspending). The
bitmaps don't support sparsemem yet, but this support could be added
pretty easily.

2) This bitmap could be used straight off for swsusp's PageNosave flag
since it is only used in kernel/power/swsusp.c (2.6.13-rc3 checked).

3) Set & Clear PageNosaveFree are also used in mm/page_alloc.c, in
mark_free_pages, which is only called from swsusp.c, so a dynamically
allocated bitmap could be used there too.

4) That leaves PageReserved. Pavel and I both rely on a page flag being
set in the arch specific code (based on e820 tables), so as to know what
pages are untouchable. As I look more closely though, I wonder if we
could do without that if we instead directly do the 

(page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))

and

(addr < (void *)&__nosave_begin || addr >= (void *)&__nosave_end)

tests when preparing the image. Assuming, of course, that page_is_ram,
bad_ppro and page_kills_ppro can be made usable by us.

Regards,

Nigel
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: PageReserved removal from swsusp

[Nick Piggin]

Nigel Cunningham wrote:
> Hi.
> 
>>
>>They're my two ideas. Anyone else?
> 
> 
> I have a few differences in Suspend2 that let me only use one real
> pageflag (Nosave).
> 

Thanks for the reply.

> The changes are:
> 
> 1) Use a set of order zero allocations, tied together via a kmalloc'd
> list of pointers as a means of dynamically creating and destroying
> pseudo page-flags (such as are only needed while suspending). The
> bitmaps don't support sparsemem yet, but this support could be added
> pretty easily.
> 
> 2) This bitmap could be used straight off for swsusp's PageNosave flag
> since it is only used in kernel/power/swsusp.c (2.6.13-rc3 checked).
> 
> 3) Set & Clear PageNosaveFree are also used in mm/page_alloc.c, in
> mark_free_pages, which is only called from swsusp.c, so a dynamically
> allocated bitmap could be used there too.
> 

1,2,3 all sound good. I guess if swsusp becomes any more of a page flag
hog then the page flag bitmap sounds like a good idea. Though at present
it will only allow us to save 1 flag (PageNosaveFree).

I guess I'm mostly interested in how to remove PageReserved usage. That is
one that isn't able to be nicely handled with your pseudo flags bitmap.

> 4) That leaves PageReserved. Pavel and I both rely on a page flag being
> set in the arch specific code (based on e820 tables), so as to know what
> pages are untouchable. As I look more closely though, I wonder if we
> could do without that if we instead directly do the 
> 
> (page_is_ram(pfn) && !(bad_ppro && page_kills_ppro(pfn))
> 
> and
> 
> (addr < (void *)&__nosave_begin || addr >= (void *)&__nosave_end)
> 
> tests when preparing the image. Assuming, of course, that page_is_ram,
> bad_ppro and page_kills_ppro can be made usable by us.
> 

Well that doesn't sound too unreasonable. Of course you don't want to
use all that directly, but have it put in a nice arch defined wrapper
for you (eg. page_is_usable_ram).

I'm currently playing around with trying to reuse an existing flag
to get this information (instead of PageReserved). But it doesn't seem
like a big problem if we have to fall back to the above.

Thanks,
Nick

-- 
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com 
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: PageReserved removal from swsusp

[Nick Piggin]

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

Nick Piggin wrote:

> I'm currently playing around with trying to reuse an existing flag
> to get this information (instead of PageReserved). But it doesn't seem
> like a big problem if we have to fall back to the above.
> 

OK, with the attached patch (on top of the PageReserved removal patches)
things work nicely. However I'm not sure that I really like the use of
flags == 0xffffffff to indicate the page is unusable. For one thing it
may confuse things that walk physical pages, and for another it can
easily break if someone clears a flag of an 'unusable' page.

-- 
SUSE Labs, Novell Inc.


[-- Attachment #2: mm-PageUnusable.patch --]
[-- Type: text/plain, Size: 3482 bytes --]

Index: linux-2.6/arch/i386/mm/init.c
===================================================================
--- linux-2.6.orig/arch/i386/mm/init.c
+++ linux-2.6/arch/i386/mm/init.c
@@ -273,7 +273,7 @@ void __init one_highpage_init(struct pag
 		__free_page(page);
 		totalhigh_pages++;
 	} else
-		SetPageReserved(page);
+		SetPageUnusable(page);
 }
 
 #ifdef CONFIG_NUMA
@@ -573,7 +573,7 @@ void __init mem_init(void)
 		/*
 		 * Only count reserved RAM pages
 		 */
-		if (page_is_ram(tmp) && PageReserved(pfn_to_page(tmp)))
+		if (!PageUnusable(pfn_to_page(tmp)) && PageReserved(pfn_to_page(tmp)))
 			reservedpages++;
 
 	set_highmem_pages_init(bad_ppro);
Index: linux-2.6/include/linux/page-flags.h
===================================================================
--- linux-2.6.orig/include/linux/page-flags.h
+++ linux-2.6/include/linux/page-flags.h
@@ -306,6 +306,9 @@ extern void __mod_page_state(unsigned lo
 #define SetPageUncached(page)	set_bit(PG_uncached, &(page)->flags)
 #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
 
+#define PageUnusable(page)	((page)->flags == 0xffffffff)
+#define SetPageUnusable(page)	((page)->flags = 0xffffffff)
+
 struct page;	/* forward declaration */
 
 int test_clear_page_dirty(struct page *page);
Index: linux-2.6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.orig/kernel/power/swsusp.c
+++ linux-2.6/kernel/power/swsusp.c
@@ -433,16 +433,8 @@ static int save_highmem_zone(struct zone
 		if (!pfn_valid(pfn))
 			continue;
 		page = pfn_to_page(pfn);
-		/*
-		 * This condition results from rvmalloc() sans vmalloc_32()
-		 * and architectural memory reservations. This should be
-		 * corrected eventually when the cases giving rise to this
-		 * are better understood.
-		 */
-		if (PageReserved(page)) {
-			printk("highmem reserved page?!\n");
+		if (PageUnusable(page))
 			continue;
-		}
 		BUG_ON(PageNosave(page));
 		if (PageNosaveFree(page))
 			continue;
@@ -528,6 +520,8 @@ static int saveable(struct zone * zone, 
 		return 0;
 
 	page = pfn_to_page(pfn);
+	if (PageUnusable(page))
+		return 0;
 	if (PageNosave(page))
 		return 0;
 	if (pfn_is_nosave(pfn)) {
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c
+++ linux-2.6/mm/page_alloc.c
@@ -576,22 +576,29 @@ void mark_free_pages(struct zone *zone)
 	unsigned long zone_pfn, flags;
 	int order;
 	struct list_head *curr;
+	struct page *page;
 
 	if (!zone->spanned_pages)
 		return;
 
 	spin_lock_irqsave(&zone->lock, flags);
-	for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
-		ClearPageNosaveFree(pfn_to_page(zone_pfn + zone->zone_start_pfn));
+	for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
+		page = pfn_to_page(zone_pfn + zone->zone_start_pfn);
+		if (PageUnusable(page))
+			continue;
+		ClearPageNosaveFree(page);
+	}
 
 	for (order = MAX_ORDER - 1; order >= 0; --order)
 		list_for_each(curr, &zone->free_area[order].free_list) {
-			unsigned long start_pfn, i;
+			unsigned long i;
 
-			start_pfn = page_to_pfn(list_entry(curr, struct page, lru));
-
-			for (i=0; i < (1<<order); i++)
-				SetPageNosaveFree(pfn_to_page(start_pfn+i));
+			page = list_entry(curr, struct page, lru);
+			for (i=0; i < (1<<order); i++) {
+				if (PageUnusable(page+i))
+					continue;
+				SetPageNosaveFree(page+i);
+			}
 	}
 	spin_unlock_irqrestore(&zone->lock, flags);
 }

Re: PageReserved removal from swsusp

[Pavel Machek]

Hi!

> >I'm currently playing around with trying to reuse an existing flag
> >to get this information (instead of PageReserved). But it doesn't seem
> >like a big problem if we have to fall back to the above.
> >
> 
> OK, with the attached patch (on top of the PageReserved removal patches)
> things work nicely. However I'm not sure that I really like the use of
> flags == 0xffffffff to indicate the page is unusable. For one thing it
> may confuse things that walk physical pages, and for another it can
> easily break if someone clears a flag of an 'unusable' page.

No compains from my part....

Another solution may be to use PageReserved as kind of "shift".

I.e. PageNosave == PageReserved | PageLocked, PageNosaveFree ==
PageReserved | PageFree or something like that....
								Pavel
-- 
teflon -- maybe it is a trademark, but it should not be.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: PageReserved removal from swsusp

[Hugh Dickins]

On Mon, 25 Jul 2005, Nick Piggin wrote:
> 
> However I'm not sure that I really like the use of
> flags == 0xffffffff to indicate the page is unusable. For one thing it
> may confuse things that walk physical pages, and for another it can
> easily break if someone clears a flag of an 'unusable' page.

I don't like that either.  Setting all the flags seems to maximize the
chance of error somewhere.  Perhaps a magic number in one of the other
struct page fields would work more safely.  Or perhaps just leave it
as its own flag bit for now, and later go on a separate free-up-some-
flags exercise.

Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: PageReserved removal from swsusp

[Nick Piggin]

Hugh Dickins wrote:
> On Mon, 25 Jul 2005, Nick Piggin wrote:
> 
>>However I'm not sure that I really like the use of
>>flags == 0xffffffff to indicate the page is unusable. For one thing it
>>may confuse things that walk physical pages, and for another it can
>>easily break if someone clears a flag of an 'unusable' page.
> 
> 
> I don't like that either.  Setting all the flags seems to maximize the
> chance of error somewhere.  Perhaps a magic number in one of the other
> struct page fields would work more safely.  Or perhaps just leave it
> as its own flag bit for now, and later go on a separate free-up-some-
> flags exercise.
> 

OK I'll leave it using PageReserved, and we can rethink it later.

-- 
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com 
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>