* [PATCH -mm 1/2] swsusp: clean up browsing of pfns
2006-07-10 20:40 [PATCH -mm 0/2] swsusp cleanups Rafael J. Wysocki
@ 2006-07-10 20:51 ` Rafael J. Wysocki
2006-07-11 8:34 ` Pavel Machek
2006-07-10 21:16 ` [PATCH -mm 2/2] swsusp: struct snapshot_handle cleanup Rafael J. Wysocki
1 sibling, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2006-07-10 20:51 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, Pavel Machek
Clean up some loops over pfns for each zone in snapshot.c: reduce the number
of additions to perform, rework detection of saveable pages and make the code
a bit less difficult to understand, hopefully.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/snapshot.c | 62 ++++++++++++++++++++++++------------------------
1 files changed, 32 insertions(+), 30 deletions(-)
Index: linux-2.6.18-rc1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc1.orig/kernel/power/snapshot.c 2006-07-09 19:48:05.000000000 +0200
+++ linux-2.6.18-rc1/kernel/power/snapshot.c 2006-07-09 20:26:19.000000000 +0200
@@ -156,7 +156,7 @@ static inline int save_highmem(void) {re
static inline int restore_highmem(void) {return 0;}
#endif
-static int pfn_is_nosave(unsigned long pfn)
+static inline int pfn_is_nosave(unsigned long pfn)
{
unsigned long nosave_begin_pfn = __pa(&__nosave_begin) >> PAGE_SHIFT;
unsigned long nosave_end_pfn = PAGE_ALIGN(__pa(&__nosave_end)) >> PAGE_SHIFT;
@@ -167,43 +167,43 @@ static int pfn_is_nosave(unsigned long p
* saveable - Determine whether a page should be cloned or not.
* @pfn: The page PFN
*
- * We save a page if it's Reserved, and not in the range of pages
- * statically defined as 'unsaveable', or if it isn't reserved, and
- * isn't part of a free chunk of pages.
+ * We save a page if it isn't Nosave, and is not in the range of pages
+ * statically defined as 'unsaveable', and it
+ * isn't a part of a free chunk of pages.
*/
-static int saveable(struct zone *zone, unsigned long *zone_pfn)
+static struct page *saveable_page(unsigned long pfn)
{
- unsigned long pfn = *zone_pfn + zone->zone_start_pfn;
struct page *page;
if (!pfn_valid(pfn))
- return 0;
+ return NULL;
page = pfn_to_page(pfn);
- BUG_ON(PageReserved(page) && PageNosave(page));
+
if (PageNosave(page))
- return 0;
+ return NULL;
if (PageReserved(page) && pfn_is_nosave(pfn))
- return 0;
+ return NULL;
if (PageNosaveFree(page))
- return 0;
+ return NULL;
- return 1;
+ return page;
}
unsigned int count_data_pages(void)
{
struct zone *zone;
- unsigned long zone_pfn;
+ unsigned long pfn, max_zone_pfn;
unsigned int n = 0;
for_each_zone (zone) {
if (is_highmem(zone))
continue;
mark_free_pages(zone);
- for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
- n += saveable(zone, &zone_pfn);
+ max_zone_pfn = zone->zone_start_pfn + zone->spanned_pages;
+ for (pfn = zone->zone_start_pfn; pfn < max_zone_pfn; pfn++)
+ n += !!saveable_page(pfn);
}
return n;
}
@@ -211,7 +211,7 @@ unsigned int count_data_pages(void)
static void copy_data_pages(struct pbe *pblist)
{
struct zone *zone;
- unsigned long zone_pfn;
+ unsigned long pfn, max_zone_pfn;
struct pbe *pbe, *p;
pbe = pblist;
@@ -224,13 +224,14 @@ static void copy_data_pages(struct pbe *
SetPageNosaveFree(virt_to_page(p));
for_each_pbe (p, pblist)
SetPageNosaveFree(virt_to_page(p->address));
- for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
- if (saveable(zone, &zone_pfn)) {
- struct page *page;
+ 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;
- page = pfn_to_page(zone_pfn + zone->zone_start_pfn);
BUG_ON(!pbe);
pbe->orig_address = (unsigned long)page_address(page);
/* copy_page and memcpy are not usable for copying task structs. */
@@ -383,13 +384,14 @@ static struct pbe *alloc_pagedir(unsigne
void swsusp_free(void)
{
struct zone *zone;
- unsigned long zone_pfn;
+ unsigned long pfn, max_zone_pfn;
for_each_zone(zone) {
- for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
- if (pfn_valid(zone_pfn + zone->zone_start_pfn)) {
- struct page *page;
- page = 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) && PageNosaveFree(page)) {
ClearPageNosave(page);
ClearPageNosaveFree(page);
@@ -598,7 +600,7 @@ int snapshot_read_next(struct snapshot_h
static int mark_unsafe_pages(struct pbe *pblist)
{
struct zone *zone;
- unsigned long zone_pfn;
+ unsigned long pfn, max_zone_pfn;
struct pbe *p;
if (!pblist) /* a sanity check */
@@ -606,10 +608,10 @@ static int mark_unsafe_pages(struct pbe
/* Clear page flags */
for_each_zone (zone) {
- for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
- if (pfn_valid(zone_pfn + zone->zone_start_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))
+ ClearPageNosaveFree(pfn_to_page(pfn));
}
/* Mark orig addresses */
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH -mm 2/2] swsusp: struct snapshot_handle cleanup
2006-07-10 20:40 [PATCH -mm 0/2] swsusp cleanups Rafael J. Wysocki
2006-07-10 20:51 ` [PATCH -mm 1/2] swsusp: clean up browsing of pfns Rafael J. Wysocki
@ 2006-07-10 21:16 ` Rafael J. Wysocki
2006-07-11 8:34 ` Pavel Machek
1 sibling, 1 reply; 8+ messages in thread
From: Rafael J. Wysocki @ 2006-07-10 21:16 UTC (permalink / raw)
To: Andrew Morton; +Cc: LKML, Pavel Machek
Add comments describing struct snapshot_handle and its members,
change the confusing name of its member 'page' to 'cur'.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/power.h | 64 ++++++++++++++++++++++++++++++++++++++++++------
kernel/power/snapshot.c | 40 +++++++++++++++---------------
2 files changed, 76 insertions(+), 28 deletions(-)
Index: linux-2.6.18-rc1-mm1/kernel/power/power.h
===================================================================
--- linux-2.6.18-rc1-mm1.orig/kernel/power/power.h
+++ linux-2.6.18-rc1-mm1/kernel/power/power.h
@@ -50,17 +50,65 @@ extern asmlinkage int swsusp_arch_resume
extern unsigned int count_data_pages(void);
+/**
+ * Auxiliary structure used for reading the snapshot image data and
+ * metadata from and writing them to the list of page backup entries
+ * (PBEs) which is the main data structure of swsusp.
+ *
+ * Using struct snapshot_handle we can transfer the image, including its
+ * metadata, as a continuous sequence of bytes with the help of
+ * snapshot_read_next() and snapshot_write_next().
+ *
+ * The code that writes the image to a storage or transfers it to
+ * the user land is required to use snapshot_read_next() for this
+ * purpose and it should not make any assumptions regarding the internal
+ * structure of the image. Similarly, the code that reads the image from
+ * a storage or transfers it from the user land is required to use
+ * snapshot_write_next().
+ *
+ * This may allow us to change the internal structure of the image
+ * in the future with considerably less effort.
+ */
+
struct snapshot_handle {
- loff_t offset;
- unsigned int page;
- unsigned int page_offset;
- unsigned int prev;
- struct pbe *pbe, *last_pbe;
- void *buffer;
- unsigned int buf_offset;
- int sync_read;
+ loff_t offset; /* number of the last byte ready for reading
+ * or writing in the sequence
+ */
+ unsigned int cur; /* number of the block of PAGE_SIZE bytes the
+ * next operation will refer to (ie. current)
+ */
+ unsigned int cur_offset; /* offset with respect to the current
+ * block (for the next operation)
+ */
+ unsigned int prev; /* number of the block of PAGE_SIZE bytes that
+ * was the current one previously
+ */
+ struct pbe *pbe; /* PBE that corresponds to 'buffer' */
+ struct pbe *last_pbe; /* When the image is restored (eg. read
+ * from disk) we can store some image
+ * data directly in the page frames
+ * in which they were before suspend.
+ * In such a case the PBEs that
+ * correspond to them will be unused.
+ * This is the last PBE, so far, that
+ * does not correspond to such data.
+ */
+ void *buffer; /* address of the block to read from
+ * or write to
+ */
+ unsigned int buf_offset; /* location to read from or write to,
+ * given as a displacement from 'buffer'
+ */
+ int sync_read; /* Set to one to notify the caller of
+ * snapshot_write_next() that it may
+ * need to call wait_on_bio_chain()
+ */
};
+/* This macro returns the address from/to which the caller of
+ * snapshot_read_next()/snapshot_write_next() is allowed to
+ * read/write data after the function returns
+ */
#define data_of(handle) ((handle).buffer + (handle).buf_offset)
extern int snapshot_read_next(struct snapshot_handle *handle, size_t count);
Index: linux-2.6.18-rc1-mm1/kernel/power/snapshot.c
===================================================================
--- linux-2.6.18-rc1-mm1.orig/kernel/power/snapshot.c
+++ linux-2.6.18-rc1-mm1/kernel/power/snapshot.c
@@ -555,7 +555,7 @@ static inline struct pbe *pack_orig_addr
int snapshot_read_next(struct snapshot_handle *handle, size_t count)
{
- if (handle->page > nr_meta_pages + nr_copy_pages)
+ if (handle->cur > nr_meta_pages + nr_copy_pages)
return 0;
if (!buffer) {
/* This makes the buffer be freed by swsusp_free() */
@@ -568,8 +568,8 @@ int snapshot_read_next(struct snapshot_h
handle->buffer = buffer;
handle->pbe = pagedir_nosave;
}
- if (handle->prev < handle->page) {
- if (handle->page <= nr_meta_pages) {
+ if (handle->prev < handle->cur) {
+ if (handle->cur <= nr_meta_pages) {
handle->pbe = pack_orig_addresses(buffer, handle->pbe);
if (!handle->pbe)
handle->pbe = pagedir_nosave;
@@ -577,15 +577,15 @@ int snapshot_read_next(struct snapshot_h
handle->buffer = (void *)handle->pbe->address;
handle->pbe = handle->pbe->next;
}
- handle->prev = handle->page;
+ handle->prev = handle->cur;
}
- handle->buf_offset = handle->page_offset;
- if (handle->page_offset + count >= PAGE_SIZE) {
- count = PAGE_SIZE - handle->page_offset;
- handle->page_offset = 0;
- handle->page++;
+ handle->buf_offset = handle->cur_offset;
+ if (handle->cur_offset + count >= PAGE_SIZE) {
+ count = PAGE_SIZE - handle->cur_offset;
+ handle->cur_offset = 0;
+ handle->cur++;
} else {
- handle->page_offset += count;
+ handle->cur_offset += count;
}
handle->offset += count;
return count;
@@ -820,7 +820,7 @@ int snapshot_write_next(struct snapshot_
{
int error = 0;
- if (handle->prev && handle->page > nr_meta_pages + nr_copy_pages)
+ if (handle->prev && handle->cur > nr_meta_pages + nr_copy_pages)
return 0;
if (!buffer) {
/* This makes the buffer be freed by swsusp_free() */
@@ -831,7 +831,7 @@ int snapshot_write_next(struct snapshot_
if (!handle->offset)
handle->buffer = buffer;
handle->sync_read = 1;
- if (handle->prev < handle->page) {
+ if (handle->prev < handle->cur) {
if (!handle->prev) {
error = load_header(handle,
(struct swsusp_info *)buffer);
@@ -854,15 +854,15 @@ int snapshot_write_next(struct snapshot_
handle->buffer = get_buffer(handle);
handle->sync_read = 0;
}
- handle->prev = handle->page;
+ handle->prev = handle->cur;
}
- handle->buf_offset = handle->page_offset;
- if (handle->page_offset + count >= PAGE_SIZE) {
- count = PAGE_SIZE - handle->page_offset;
- handle->page_offset = 0;
- handle->page++;
+ handle->buf_offset = handle->cur_offset;
+ if (handle->cur_offset + count >= PAGE_SIZE) {
+ count = PAGE_SIZE - handle->cur_offset;
+ handle->cur_offset = 0;
+ handle->cur++;
} else {
- handle->page_offset += count;
+ handle->cur_offset += count;
}
handle->offset += count;
return count;
@@ -871,5 +871,5 @@ int snapshot_write_next(struct snapshot_
int snapshot_image_loaded(struct snapshot_handle *handle)
{
return !(!handle->pbe || handle->pbe->next || !nr_copy_pages ||
- handle->page <= nr_meta_pages + nr_copy_pages);
+ handle->cur <= nr_meta_pages + nr_copy_pages);
}
^ permalink raw reply [flat|nested] 8+ messages in thread