public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] swsusp cleanups
@ 2006-08-02 16:42 Rafael J. Wysocki
  2006-08-02 16:48 ` [PATCH 1/3] swsusp: Fix mark_free_pages Rafael J. Wysocki
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 16:42 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Pavel Machek

Hi,

The following patches contain some small clean up some functions
in kernel/power/snapshot.c and mm/page_alloc.c.

Greetings,
Rafael


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] swsusp: Fix mark_free_pages
  2006-08-02 16:42 [PATCH 0/3] swsusp cleanups Rafael J. Wysocki
@ 2006-08-02 16:48 ` Rafael J. Wysocki
  2006-08-02 19:51   ` Dave Hansen
  2006-08-02 16:53 ` [PATCH 2/3] swsusp: Reorder memory-allocating functions Rafael J. Wysocki
  2006-08-02 16:57 ` [PATCH 3/3] swsusp: Fix alloc_pagedir Rafael J. Wysocki
  2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 16:48 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Pavel Machek

Clean up mm/page_alloc.c#mark_free_pages() and make it avoid clearing
PageNosaveFree for PageNosave pages.  This allows us to get rid of an ugly
hack in kernel/power/snapshot.c#copy_data_pages().

Additionally, the page-copying loop in copy_data_pages() is moved to an
inline function.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/snapshot.c |   27 +++++++++++++--------------
 mm/page_alloc.c         |   22 ++++++++++++++--------
 2 files changed, 27 insertions(+), 22 deletions(-)

Index: linux-2.6.18-rc2-mm1/mm/page_alloc.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/mm/page_alloc.c	2006-07-31 20:42:29.000000000 +0200
+++ linux-2.6.18-rc2-mm1/mm/page_alloc.c	2006-07-31 20:52:11.000000000 +0200
@@ -703,7 +703,8 @@ static void __drain_pages(unsigned int c
 
 void mark_free_pages(struct zone *zone)
 {
-	unsigned long zone_pfn, flags;
+	unsigned long pfn, max_zone_pfn;
+	unsigned long flags;
 	int order;
 	struct list_head *curr;
 
@@ -711,18 +712,23 @@ void mark_free_pages(struct zone *zone)
 		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));
 
+	max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
+	for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
+		struct page *page = pfn_to_page(pfn);
+
+		if (!PageNosave(page))
+			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));
+			pfn = page_to_pfn(list_entry(curr, struct page, lru));
+			for (i = 0; i < (1UL << order); i++)
+				SetPageNosaveFree(pfn_to_page(pfn + i));
+		}
 
-			for (i=0; i < (1<<order); i++)
-				SetPageNosaveFree(pfn_to_page(start_pfn+i));
-	}
 	spin_unlock_irqrestore(&zone->lock, flags);
 }
 
Index: linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/kernel/power/snapshot.c	2006-07-31 20:47:26.000000000 +0200
+++ linux-2.6.18-rc2-mm1/kernel/power/snapshot.c	2006-07-31 21:01:22.000000000 +0200
@@ -208,37 +208,36 @@ unsigned int count_data_pages(void)
 	return n;
 }
 
+static inline void copy_data_page(long *dst, long *src)
+{
+	int n;
+
+	/* copy_page and memcpy are not usable for copying task structs. */
+	for (n = PAGE_SIZE / sizeof(long); n; n--)
+		*dst++ = *src++;
+}
+
 static void copy_data_pages(struct pbe *pblist)
 {
 	struct zone *zone;
 	unsigned long pfn, max_zone_pfn;
-	struct pbe *pbe, *p;
+	struct pbe *pbe;
 
 	pbe = pblist;
 	for_each_zone (zone) {
 		if (is_highmem(zone))
 			continue;
 		mark_free_pages(zone);
-		/* This is necessary for swsusp_free() */
-		for_each_pb_page (p, pblist)
-			SetPageNosaveFree(virt_to_page(p));
-		for_each_pbe (p, pblist)
-			SetPageNosaveFree(virt_to_page(p->address));
 		max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
 		for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
 			struct page *page = saveable_page(pfn);
 
 			if (page) {
-				long *src, *dst;
-				int n;
+				void *ptr = page_address(page);;
 
 				BUG_ON(!pbe);
-				pbe->orig_address = (unsigned long)page_address(page);
-				/* copy_page and memcpy are not usable for copying task structs. */
-				dst = (long *)pbe->address;
-				src = (long *)pbe->orig_address;
-				for (n = PAGE_SIZE / sizeof(long); n; n--)
-					*dst++ = *src++;
+				copy_data_page((void *)pbe->address, ptr);
+				pbe->orig_address = (unsigned long)ptr;
 				pbe = pbe->next;
 			}
 		}


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] swsusp: Reorder memory-allocating functions
  2006-08-02 16:42 [PATCH 0/3] swsusp cleanups Rafael J. Wysocki
  2006-08-02 16:48 ` [PATCH 1/3] swsusp: Fix mark_free_pages Rafael J. Wysocki
@ 2006-08-02 16:53 ` Rafael J. Wysocki
  2006-08-02 20:19   ` Pavel Machek
  2006-08-02 16:57 ` [PATCH 3/3] swsusp: Fix alloc_pagedir Rafael J. Wysocki
  2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 16:53 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Pavel Machek

Move some functions in kernel/power/snapshot.c to a better place
(in the same file) and introduce free_image_page() (will be necessary in the
future).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/snapshot.c |   93 +++++++++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 40 deletions(-)

Index: linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/kernel/power/snapshot.c	2006-08-01 08:58:45.000000000 +0200
+++ linux-2.6.18-rc2-mm1/kernel/power/snapshot.c	2006-08-01 22:15:41.000000000 +0200
@@ -156,6 +156,58 @@ static inline int save_highmem(void) {re
 static inline int restore_highmem(void) {return 0;}
 #endif
 
+/**
+ *	@safe_needed - on resume, for storing the PBE list and the image,
+ *	we can only use memory pages that do not conflict with the pages
+ *	used before suspend.
+ *
+ *	The unsafe pages are marked with the PG_nosave_free flag
+ *	and we count them using unsafe_pages
+ */
+
+static unsigned int unsafe_pages;
+
+static void *alloc_image_page(gfp_t gfp_mask, int safe_needed)
+{
+	void *res;
+
+	res = (void *)get_zeroed_page(gfp_mask);
+	if (safe_needed)
+		while (res && PageNosaveFree(virt_to_page(res))) {
+			/* The page is unsafe, mark it for swsusp_free() */
+			SetPageNosave(virt_to_page(res));
+			unsafe_pages++;
+			res = (void *)get_zeroed_page(gfp_mask);
+		}
+	if (res) {
+		SetPageNosave(virt_to_page(res));
+		SetPageNosaveFree(virt_to_page(res));
+	}
+	return res;
+}
+
+unsigned long get_safe_page(gfp_t gfp_mask)
+{
+	return (unsigned long)alloc_image_page(gfp_mask, 1);
+}
+
+/**
+ *	free_image_page - free page represented by @addr, allocated with
+ *	alloc_image_page (page flags set by it must be cleared)
+ */
+
+static inline void free_image_page(void *addr, int clear_nosave_free)
+{
+	ClearPageNosave(virt_to_page(addr));
+	if (clear_nosave_free)
+		ClearPageNosaveFree(virt_to_page(addr));
+	free_page((unsigned long)addr);
+}
+
+/**
+ *	pfn_is_nosave - check if given pfn is in the 'nosave' section
+ */
+
 static inline int pfn_is_nosave(unsigned long pfn)
 {
 	unsigned long nosave_begin_pfn = __pa(&__nosave_begin) >> PAGE_SHIFT;
@@ -245,7 +297,6 @@ static void copy_data_pages(struct pbe *
 	BUG_ON(pbe);
 }
 
-
 /**
  *	free_pagedir - free pages allocated with alloc_pagedir()
  */
@@ -256,10 +307,7 @@ static void free_pagedir(struct pbe *pbl
 
 	while (pblist) {
 		pbe = (pblist + PB_PAGE_SKIP)->next;
-		ClearPageNosave(virt_to_page(pblist));
-		if (clear_nosave_free)
-			ClearPageNosaveFree(virt_to_page(pblist));
-		free_page((unsigned long)pblist);
+		free_image_page(pblist, clear_nosave_free);
 		pblist = pbe;
 	}
 }
@@ -303,41 +351,6 @@ static inline void create_pbe_list(struc
 	}
 }
 
-static unsigned int unsafe_pages;
-
-/**
- *	@safe_needed - on resume, for storing the PBE list and the image,
- *	we can only use memory pages that do not conflict with the pages
- *	used before suspend.
- *
- *	The unsafe pages are marked with the PG_nosave_free flag
- *	and we count them using unsafe_pages
- */
-
-static void *alloc_image_page(gfp_t gfp_mask, int safe_needed)
-{
-	void *res;
-
-	res = (void *)get_zeroed_page(gfp_mask);
-	if (safe_needed)
-		while (res && PageNosaveFree(virt_to_page(res))) {
-			/* The page is unsafe, mark it for swsusp_free() */
-			SetPageNosave(virt_to_page(res));
-			unsafe_pages++;
-			res = (void *)get_zeroed_page(gfp_mask);
-		}
-	if (res) {
-		SetPageNosave(virt_to_page(res));
-		SetPageNosaveFree(virt_to_page(res));
-	}
-	return res;
-}
-
-unsigned long get_safe_page(gfp_t gfp_mask)
-{
-	return (unsigned long)alloc_image_page(gfp_mask, 1);
-}
-
 /**
  *	alloc_pagedir - Allocate the page directory.
  *

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] swsusp: Fix alloc_pagedir
  2006-08-02 16:42 [PATCH 0/3] swsusp cleanups Rafael J. Wysocki
  2006-08-02 16:48 ` [PATCH 1/3] swsusp: Fix mark_free_pages Rafael J. Wysocki
  2006-08-02 16:53 ` [PATCH 2/3] swsusp: Reorder memory-allocating functions Rafael J. Wysocki
@ 2006-08-02 16:57 ` Rafael J. Wysocki
  2006-08-02 20:22   ` Pavel Machek
  2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 16:57 UTC (permalink / raw)
  To: Andrew Morton; +Cc: LKML, Pavel Machek

Get rid of the FIXME in kernel/power/snapshot.c#alloc_pagedir() and
simplify the functions called by it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/snapshot.c |   32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

Index: linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/kernel/power/snapshot.c	2006-08-01 22:15:41.000000000 +0200
+++ linux-2.6.18-rc2-mm1/kernel/power/snapshot.c	2006-08-01 22:24:57.000000000 +0200
@@ -316,12 +316,12 @@ static void free_pagedir(struct pbe *pbl
  *	fill_pb_page - Create a list of PBEs on a given memory page
  */
 
-static inline void fill_pb_page(struct pbe *pbpage)
+static inline void fill_pb_page(struct pbe *pbpage, unsigned int n)
 {
 	struct pbe *p;
 
 	p = pbpage;
-	pbpage += PB_PAGE_SKIP;
+	pbpage += n - 1;
 	do
 		p->next = p + 1;
 	while (++p < pbpage);
@@ -330,24 +330,26 @@ static inline void fill_pb_page(struct p
 /**
  *	create_pbe_list - Create a list of PBEs on top of a given chain
  *	of memory pages allocated with alloc_pagedir()
+ *
+ *	This function assumes that pages allocated by alloc_image_page() will
+ *	always be zeroed.
  */
 
 static inline void create_pbe_list(struct pbe *pblist, unsigned int nr_pages)
 {
-	struct pbe *pbpage, *p;
+	struct pbe *pbpage;
 	unsigned int num = PBES_PER_PAGE;
 
 	for_each_pb_page (pbpage, pblist) {
 		if (num >= nr_pages)
 			break;
 
-		fill_pb_page(pbpage);
+		fill_pb_page(pbpage, PBES_PER_PAGE);
 		num += PBES_PER_PAGE;
 	}
 	if (pbpage) {
-		for (num -= PBES_PER_PAGE - 1, p = pbpage; num < nr_pages; p++, num++)
-			p->next = p + 1;
-		p->next = NULL;
+		num -= PBES_PER_PAGE;
+		fill_pb_page(pbpage, nr_pages - num);
 	}
 }
 
@@ -374,17 +376,17 @@ static struct pbe *alloc_pagedir(unsigne
 		return NULL;
 
 	pblist = alloc_image_page(gfp_mask, safe_needed);
-	/* FIXME: rewrite this ugly loop */
-	for (pbe = pblist, num = PBES_PER_PAGE; pbe && num < nr_pages;
-        		pbe = pbe->next, num += PBES_PER_PAGE) {
+	pbe = pblist;
+	for (num = PBES_PER_PAGE; num < nr_pages; num += PBES_PER_PAGE) {
+		if (!pbe) {
+			free_pagedir(pblist, 1);
+			return NULL;
+		}
 		pbe += PB_PAGE_SKIP;
 		pbe->next = alloc_image_page(gfp_mask, safe_needed);
+		pbe = pbe->next;
 	}
-	if (!pbe) { /* get_zeroed_page() failed */
-		free_pagedir(pblist, 1);
-		pblist = NULL;
-        } else
-		create_pbe_list(pblist, nr_pages);
+	create_pbe_list(pblist, nr_pages);
 	return pblist;
 }
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] swsusp: Fix mark_free_pages
  2006-08-02 16:48 ` [PATCH 1/3] swsusp: Fix mark_free_pages Rafael J. Wysocki
@ 2006-08-02 19:51   ` Dave Hansen
  2006-08-02 20:12     ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Dave Hansen @ 2006-08-02 19:51 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Andrew Morton, LKML, Pavel Machek

On Wed, 2006-08-02 at 18:48 +0200, Rafael J. Wysocki wrote:
> +       max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
> +       for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
> +               struct page *page = pfn_to_page(pfn);
> +
> +               if (!PageNosave(page))
> +                       ClearPageNosaveFree(page);
> +       } 

This is certainly not the only place in swsusp where there is a bug like
this, but it is not correct to assume that each page inside of a zone is
valid.  With sparsemem, you need to do pfn_valid() on each pfn before
calling pfn_to_page().

Something like:

       max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
       for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
		struct page *page;
		if (!pfn_valid(pfn))
			continue;

		page = pfn_to_page(pfn);
		if (!PageNosave(page))
                       ClearPageNosaveFree(page);
       } 


-- Dave


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] swsusp: Fix mark_free_pages
  2006-08-02 19:51   ` Dave Hansen
@ 2006-08-02 20:12     ` Rafael J. Wysocki
  2006-08-02 20:30       ` Pavel Machek
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 20:12 UTC (permalink / raw)
  To: Dave Hansen; +Cc: Andrew Morton, LKML, Pavel Machek

On Wednesday 02 August 2006 21:51, Dave Hansen wrote:
> On Wed, 2006-08-02 at 18:48 +0200, Rafael J. Wysocki wrote:
> > +       max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
> > +       for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
> > +               struct page *page = pfn_to_page(pfn);
> > +
> > +               if (!PageNosave(page))
> > +                       ClearPageNosaveFree(page);
> > +       } 
> 
> This is certainly not the only place in swsusp where there is a bug like
> this,

Well, I don't think so.  They have been hunted for and dealt whith, except
for this one. ;-)

> but it is not correct to assume that each page inside of a zone is 
> valid.

Obviously, I just forgot about it.

---
Clean up mm/page_alloc.c#mark_free_pages() and make it avoid clearing
PageNosaveFree for PageNosave pages.  This allows us to get rid of an ugly
hack in kernel/power/snapshot.c#copy_data_pages().

Additionally, the page-copying loop in copy_data_pages() is moved to an
inline function.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/snapshot.c |   27 +++++++++++++--------------
 mm/page_alloc.c         |   24 ++++++++++++++++--------
 2 files changed, 29 insertions(+), 22 deletions(-)

Index: linux-2.6.18-rc2-mm1/mm/page_alloc.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/mm/page_alloc.c
+++ linux-2.6.18-rc2-mm1/mm/page_alloc.c
@@ -703,7 +703,8 @@ static void __drain_pages(unsigned int c
 
 void mark_free_pages(struct zone *zone)
 {
-	unsigned long zone_pfn, flags;
+	unsigned long pfn, max_zone_pfn;
+	unsigned long flags;
 	int order;
 	struct list_head *curr;
 
@@ -711,18 +712,25 @@ void mark_free_pages(struct zone *zone)
 		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));
+
+	max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
+	for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++)
+		if (pfn_valid(pfn)) {
+			struct page *page = pfn_to_page(pfn);
+
+			if (!PageNosave(page))
+				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));
+			pfn = page_to_pfn(list_entry(curr, struct page, lru));
+			for (i = 0; i < (1UL << order); i++)
+				SetPageNosaveFree(pfn_to_page(pfn + i));
+		}
 
-			for (i=0; i < (1<<order); i++)
-				SetPageNosaveFree(pfn_to_page(start_pfn+i));
-	}
 	spin_unlock_irqrestore(&zone->lock, flags);
 }
 
Index: linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/kernel/power/snapshot.c
+++ linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
@@ -208,37 +208,36 @@ unsigned int count_data_pages(void)
 	return n;
 }
 
+static inline void copy_data_page(long *dst, long *src)
+{
+	int n;
+
+	/* copy_page and memcpy are not usable for copying task structs. */
+	for (n = PAGE_SIZE / sizeof(long); n; n--)
+		*dst++ = *src++;
+}
+
 static void copy_data_pages(struct pbe *pblist)
 {
 	struct zone *zone;
 	unsigned long pfn, max_zone_pfn;
-	struct pbe *pbe, *p;
+	struct pbe *pbe;
 
 	pbe = pblist;
 	for_each_zone (zone) {
 		if (is_highmem(zone))
 			continue;
 		mark_free_pages(zone);
-		/* This is necessary for swsusp_free() */
-		for_each_pb_page (p, pblist)
-			SetPageNosaveFree(virt_to_page(p));
-		for_each_pbe (p, pblist)
-			SetPageNosaveFree(virt_to_page(p->address));
 		max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
 		for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
 			struct page *page = saveable_page(pfn);
 
 			if (page) {
-				long *src, *dst;
-				int n;
+				void *ptr = page_address(page);;
 
 				BUG_ON(!pbe);
-				pbe->orig_address = (unsigned long)page_address(page);
-				/* copy_page and memcpy are not usable for copying task structs. */
-				dst = (long *)pbe->address;
-				src = (long *)pbe->orig_address;
-				for (n = PAGE_SIZE / sizeof(long); n; n--)
-					*dst++ = *src++;
+				copy_data_page((void *)pbe->address, ptr);
+				pbe->orig_address = (unsigned long)ptr;
 				pbe = pbe->next;
 			}
 		}

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] swsusp: Reorder memory-allocating functions
  2006-08-02 16:53 ` [PATCH 2/3] swsusp: Reorder memory-allocating functions Rafael J. Wysocki
@ 2006-08-02 20:19   ` Pavel Machek
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Machek @ 2006-08-02 20:19 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Andrew Morton, LKML

On Wed 2006-08-02 18:53:46, Rafael J. Wysocki wrote:
> Move some functions in kernel/power/snapshot.c to a better place
> (in the same file) and introduce free_image_page() (will be necessary in the
> future).

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] swsusp: Fix alloc_pagedir
  2006-08-02 16:57 ` [PATCH 3/3] swsusp: Fix alloc_pagedir Rafael J. Wysocki
@ 2006-08-02 20:22   ` Pavel Machek
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Machek @ 2006-08-02 20:22 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Andrew Morton, LKML

Hi!

> Get rid of the FIXME in kernel/power/snapshot.c#alloc_pagedir() and
> simplify the functions called by it.

ACK.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] swsusp: Fix mark_free_pages
  2006-08-02 20:12     ` Rafael J. Wysocki
@ 2006-08-02 20:30       ` Pavel Machek
  2006-08-02 20:48         ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Pavel Machek @ 2006-08-02 20:30 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Dave Hansen, Andrew Morton, LKML

Hi!

Looks good to me (ACK).

>  			if (page) {
> -				long *src, *dst;
> -				int n;
> +				void *ptr = page_address(page);;

You probably want to remove one of ";"s.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] swsusp: Fix mark_free_pages
  2006-08-02 20:30       ` Pavel Machek
@ 2006-08-02 20:48         ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2006-08-02 20:48 UTC (permalink / raw)
  To: Pavel Machek; +Cc: Dave Hansen, Andrew Morton, LKML

On Wednesday 02 August 2006 22:30, Pavel Machek wrote:
> Hi!
> 
> Looks good to me (ACK).
> 
> >  			if (page) {
> > -				long *src, *dst;
> > -				int n;
> > +				void *ptr = page_address(page);;
> 
> You probably want to remove one of ";"s.

Ouch.

I hope this is the final one.
---
Clean up mm/page_alloc.c#mark_free_pages() and make it avoid clearing
PageNosaveFree for PageNosave pages.  This allows us to get rid of an ugly
hack in kernel/power/snapshot.c#copy_data_pages().

Additionally, the page-copying loop in copy_data_pages() is moved to an
inline function.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/snapshot.c |   27 +++++++++++++--------------
 mm/page_alloc.c         |   24 ++++++++++++++++--------
 2 files changed, 29 insertions(+), 22 deletions(-)

Index: linux-2.6.18-rc2-mm1/mm/page_alloc.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/mm/page_alloc.c
+++ linux-2.6.18-rc2-mm1/mm/page_alloc.c
@@ -703,7 +703,8 @@ static void __drain_pages(unsigned int c
 
 void mark_free_pages(struct zone *zone)
 {
-	unsigned long zone_pfn, flags;
+	unsigned long pfn, max_zone_pfn;
+	unsigned long flags;
 	int order;
 	struct list_head *curr;
 
@@ -711,18 +712,25 @@ void mark_free_pages(struct zone *zone)
 		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));
+
+	max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
+	for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++)
+		if (pfn_valid(pfn)) {
+			struct page *page = pfn_to_page(pfn);
+
+			if (!PageNosave(page))
+				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));
+			pfn = page_to_pfn(list_entry(curr, struct page, lru));
+			for (i = 0; i < (1UL << order); i++)
+				SetPageNosaveFree(pfn_to_page(pfn + i));
+		}
 
-			for (i=0; i < (1<<order); i++)
-				SetPageNosaveFree(pfn_to_page(start_pfn+i));
-	}
 	spin_unlock_irqrestore(&zone->lock, flags);
 }
 
Index: linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc2-mm1.orig/kernel/power/snapshot.c
+++ linux-2.6.18-rc2-mm1/kernel/power/snapshot.c
@@ -208,37 +208,36 @@ unsigned int count_data_pages(void)
 	return n;
 }
 
+static inline void copy_data_page(long *dst, long *src)
+{
+	int n;
+
+	/* copy_page and memcpy are not usable for copying task structs. */
+	for (n = PAGE_SIZE / sizeof(long); n; n--)
+		*dst++ = *src++;
+}
+
 static void copy_data_pages(struct pbe *pblist)
 {
 	struct zone *zone;
 	unsigned long pfn, max_zone_pfn;
-	struct pbe *pbe, *p;
+	struct pbe *pbe;
 
 	pbe = pblist;
 	for_each_zone (zone) {
 		if (is_highmem(zone))
 			continue;
 		mark_free_pages(zone);
-		/* This is necessary for swsusp_free() */
-		for_each_pb_page (p, pblist)
-			SetPageNosaveFree(virt_to_page(p));
-		for_each_pbe (p, pblist)
-			SetPageNosaveFree(virt_to_page(p->address));
 		max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
 		for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++) {
 			struct page *page = saveable_page(pfn);
 
 			if (page) {
-				long *src, *dst;
-				int n;
+				void *ptr = page_address(page);
 
 				BUG_ON(!pbe);
-				pbe->orig_address = (unsigned long)page_address(page);
-				/* copy_page and memcpy are not usable for copying task structs. */
-				dst = (long *)pbe->address;
-				src = (long *)pbe->orig_address;
-				for (n = PAGE_SIZE / sizeof(long); n; n--)
-					*dst++ = *src++;
+				copy_data_page((void *)pbe->address, ptr);
+				pbe->orig_address = (unsigned long)ptr;
 				pbe = pbe->next;
 			}
 		}

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2006-08-02 20:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-02 16:42 [PATCH 0/3] swsusp cleanups Rafael J. Wysocki
2006-08-02 16:48 ` [PATCH 1/3] swsusp: Fix mark_free_pages Rafael J. Wysocki
2006-08-02 19:51   ` Dave Hansen
2006-08-02 20:12     ` Rafael J. Wysocki
2006-08-02 20:30       ` Pavel Machek
2006-08-02 20:48         ` Rafael J. Wysocki
2006-08-02 16:53 ` [PATCH 2/3] swsusp: Reorder memory-allocating functions Rafael J. Wysocki
2006-08-02 20:19   ` Pavel Machek
2006-08-02 16:57 ` [PATCH 3/3] swsusp: Fix alloc_pagedir Rafael J. Wysocki
2006-08-02 20:22   ` Pavel Machek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox