swsusp cleanups
Subsystems (2)
cover swsusp cleanups
Hi,
I have two patches that clean up the swsusp code a little.
Greetings,
Rafael
PATCH 1/2 swsusp: clean up browsing of pfns
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.
--- kernel/power/snapshot.c | 62 ++++++++++++++++++++++++------------------------ 1 files changed, 32 insertions(+), 30 deletions(-)
kernel/power/snapshot.c +32 -30
@@ -156,7 +156,7 @@ static inline int save_highmem(void) {restatic 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(unsignevoid 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_hstatic 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 */
Re: [PATCH -mm 1/2] swsusp: clean up browsing of pfns
Hi!
Quoted 3 lines
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.
Also remove the BUG_ON() so that you can solve Andrew's monster machine problem. ACK.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH -mm 1/2] swsusp: clean up browsing of pfns
Quoted 8 lines · On Tue, 11 Jul 2006 10:34:15 +0200 Pavel Machek <[email protected]> wrote:
Hi!
Quoted 3 lines
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.Also remove the
BUG_ON()so that you can solve Andrew's monster machine problem.
I don't understand your comment. I assume you're adding an explanation for the removal of:
- BUG_ON(PageReserved(page) && PageNosave(page));
from saveable_page().
But my emt64 test box is oopsing when touching a hole in the memory-map; it isn't going BUG() (any more than usual ;))
Re: [PATCH -mm 1/2] swsusp: clean up browsing of pfns
Quoted 17 lines · Andrew Morton
Quoted 8 lines · On Tue, 11 Jul 2006 10:34:15 +0200 Pavel Machek <[email protected]> wrote:
Hi!
Quoted 3 lines
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.Also remove the
BUG_ON()so that you can solve Andrew's monster machine problem.I don't understand your comment. I assume you're adding an explanation for the removal of:
kernel/power/snapshot.c +0
-1-BUG_ON (PageReserved (page) &&PageNosave (page));
Quoted 1 line
from
saveable_page().
Yes.
Quoted 2 lines
But my emt64 test box is oopsing when touching a hole in the memory-map; it isn't going
BUG()(any more than usual ;))
Well, it would go BUG() with patch Rafael has somewhere on the disk. Next step is having pages both reserved and nosave, I believe.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH -mm 1/2] swsusp: clean up browsing of pfns
Quoted 22 lines · Pavel Machek
Quoted 17 lines · Andrew Morton
Quoted 8 lines · On Tue, 11 Jul 2006 10:34:15 +0200 Pavel Machek <[email protected]> wrote:
Hi!
Quoted 3 lines
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.Also remove the
BUG_ON()so that you can solve Andrew's monster machine problem.I don't understand your comment. I assume you're adding an explanation for the removal of:
kernel/power/snapshot.c +0
-1-BUG_ON (PageReserved (page) &&PageNosave (page));Quoted 1 line
from
saveable_page().Yes.
Quoted 5 lines
Quoted 2 lines
But my emt64 test box is oopsing when touching a hole in the memory-map; it isn't going
BUG()(any more than usual ;))Well, it would go
BUG()with patch Rafael has somewhere on the disk. Next step is having pages both reserved and nosave, I believe.
Yes.
Rafael
PATCH 2/2 swsusp: struct snapshot_handle cleanup
In reply to Rafael J. WysockiAdd comments describing struct snapshot_handle and its members, change the confusing name of its member 'page' to 'cur'.
--- kernel/power/power.h | 64 ++++++++++++++++++++++++++++++++++++++++++------ kernel/power/snapshot.c | 40 +++++++++++++++--------------- 2 files changed, 76 insertions(+), 28 deletions(-)
kernel/power/power.h +56 -8
@@ -50,17 +50,65 @@ extern asmlinkage int swsusp_arch_resumeextern 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);
kernel/power/snapshot.c +20 -20
@@ -555,7 +555,7 @@ static inline struct pbe *pack_orig_addrint 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); }
Re: [PATCH -mm 2/2] swsusp: struct snapshot_handle cleanup
Quoted 4 lines · Rafael J. Wysocki
Add comments describing struct
snapshot_handleand its members, change the confusing name of its member 'page' to 'cur'.Signed-off-by Rafael J. Wysocki <[email protected]>
ACK.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html