PageReserved removal from swsusp

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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi.

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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Thanks for the reply.

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.

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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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.

Index: 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
===================================================================
@@ -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
===================================================================
@@ -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
===================================================================
@@ -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);
 }

Hi!

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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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 [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>