Re: [PATCH -mm] swsusp: Improve handling of highmem

["Rafael J. Wysocki" <rjw@sisk.pl>]

Hi,

On Monday, 23 October 2006 17:04, Pavel Machek wrote:
> HI!
> 
> > Currently swsusp saves the contents of highmem pages by copying them to the
> > normal zone which is quite inefficient  (eg. it requires two normal pages to be
> > used for saving one highmem page).  This may be improved by using highmem
> > for saving the contents of saveable highmem pages.
> ...
> >  include/linux/suspend.h |    9 
> >  kernel/power/power.h    |    2 
> >  kernel/power/snapshot.c |  841 ++++++++++++++++++++++++++++++++++++------------
> >  kernel/power/swap.c     |    2 
> >  kernel/power/swsusp.c   |   53 +--
> >  kernel/power/user.c     |    2 
> >  mm/vmscan.c             |    3 
> 
> Well, I just hoped that highmem would quietly die out...
> 
> ...
> > +static struct page *alloc_image_page(gfp_t gfp_mask) {
> > +	struct page *page;
> 
> { should go on new line.

Ah, yes, thanks.  I'll fix this later if you don't mind.

> >  	memory_bm_position_reset(orig_bm);
> >  	memory_bm_position_reset(copy_bm);
> >  	do {
> >  		pfn = memory_bm_next_pfn(orig_bm);
> > -		if (likely(pfn != BM_END_OF_MAP)) {
> > -			struct page *page;
> > -			void *src;
> > -
> > -			page = pfn_to_page(pfn);
> > -			src = page_address(page);
> > -			page = pfn_to_page(memory_bm_next_pfn(copy_bm));
> > -			copy_data_page(page_address(page), src);
> > -		}
> > +		if (likely(pfn != BM_END_OF_MAP))
> > +			copy_data_page(memory_bm_next_pfn(copy_bm), pfn);
> >  	} while (pfn != BM_END_OF_MAP);
> >  }
> 
> While(1) and "if (pfn != BM_END_OF_MAP) { ...break; } ? Why do you
> need to test pfn != BM_END_OF_MAP *three* times?

Why?  It's two times, and I don't like while(1) loops, really.

> > Index: linux-2.6.18-mm3/mm/vmscan.c
> > ===================================================================
> > --- linux-2.6.18-mm3.orig/mm/vmscan.c
> > +++ linux-2.6.18-mm3/mm/vmscan.c
> > @@ -1233,6 +1233,9 @@ out:
> >  	}
> >  	if (!all_zones_ok) {
> >  		cond_resched();
> > +
> > +		try_to_freeze();
> > +
> >  		goto loop_again;
> >  	}
> 
> What is this?

:-)

This is needed because during the resume there likely are no free pages in the
highmem zone which makes kswapd spin forever, but we have to freeze it before
the image is restored.

Greetings,
Rafael


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/