Re: [RFC][PATCH 2/3] swsusp: Do not use page flags

Rafael J. Wysocki
2007-03-13T09:16:11Z

On Tuesday, 13 March 2007 05:47, Nick Piggin wrote:
> Rafael J. Wysocki wrote:
>
> > }
> >
> > /**
> > + * This structure represents a range of page frames the contents of which
> > + * should not be saved during the suspend.
> > + */
> > +
> > +struct nosave_region {
> > + struct list_head list;
> > + unsigned long start_pfn;
> > + unsigned long end_pfn;
> > +};
> > +
> > +static LIST_HEAD(nosave_regions);
> > +
> > +/**
> > + * register_nosave_region - register a range of page frames the contents
> > + * of which should not be saved during the suspend (to be used in the early
> > + * initializatoion code)
> > + */
> > +
> > +void __init
> > +register_nosave_region(unsigned long start_pfn, unsigned long end_pfn)
> > +{
> > + struct nosave_region *region;
> > +
> > + if (start_pfn >= end_pfn)
> > + return;
> > +
> > + if (!list_empty(&nosave_regions)) {
> > + /* Try to extend the previous region (they should be sorted) */
> > + region = list_entry(nosave_regions.prev,
> > + struct nosave_region, list);
> > + if (region->end_pfn == start_pfn) {
> > + region->end_pfn = end_pfn;
> > + goto Report;
> > + }
> > + }
> > + /* This allocation cannot fail */
> > + region = alloc_bootmem_low(sizeof(struct nosave_region));
> > + region->start_pfn = start_pfn;
> > + region->end_pfn = end_pfn;
> > + list_add_tail(&region->list, &nosave_regions);
> > + Report:
> > + printk("swsusp: Registered nosave memory region: %016lx - %016lx\n",
> > + start_pfn << PAGE_SHIFT, end_pfn << PAGE_SHIFT);
> > +}
>
>
> I wonder why you reimplemented this and put it in snapshot.c, rather than
> use my version which was nicely in its own file, had appropriate locking,
> etc.?

Well, the locking is not necessary and we only need a list for that. Also,
mark_nosave_pages() refers to a function that's invisible outside snapshot.c
and I didn't think it was a good idea to separate mark_nosave_pages()
from register_nosave_region().

Greetings,
Rafael