Re: [PATCH] Free swap suspend from dependency on PageReserved

From: Rafael J. Wysocki
Date: Thu Oct 06 2005 - 03:27:26 EST


Hi Nigel,

On Thursday, 6 of October 2005 00:24, Nigel Cunningham wrote:
> Hi Dave.
>
> On Thu, 2005-10-06 at 08:03, Dave Hansen wrote:
> > On Thu, 2005-10-06 at 07:04 +1000, Nigel Cunningham wrote:
> > >
> > > + for (tmp = 0; tmp < max_low_pfn; tmp++, addr += PAGE_SIZE) {
> > > + if (page_is_ram(tmp)) {
> > > + /*
> > > + * Only count reserved RAM pages
> > > + */
> > > + if (PageReserved(mem_map+tmp))
> > > + reservedpages++;
> >
> > Please don't reference mem_map[] directly outside of #ifdef
> > CONFIG_FLATMEM areas. It is not defined for all config cases. Please
> > use pfn_to_page(), instead. It should work in all cases where the page
> > is valid.
> >
> > Also, instead of keeping addr defined like you do, and comparing it
> > during each run of the loop, why not just use pfn_is_nosave(), which is
> > already defined? Then, you won't even need the variable.
>
> Thanks for the comments. Will revise and resend.

There's one more reference to PageReserved() in swsusp.c, which is
in save_highmem_zone(). You seem to omit this one (or I have missed
it in your patch in which case please ignore this message). Is that
intentional?

Rafael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/