Re: [RFC][PATCH] swsusp: support creating bigger images

Rafael J. Wysocki
2006-04-25T21:12:55Z

Hi,

On Tuesday 25 April 2006 22:32, Pavel Machek wrote:
> > > > -unsigned int count_data_pages(void)
> > > > +/**
> > > > + * need_to_copy - determine if a page needs to be copied before saving.
> > > > + * Returns false if the page can be saved without copying.
> > > > + */
> > > > +
> > > > +static int need_to_copy(struct page *page)
> > > > +{
> > > > + if (!PageLRU(page) || PageCompound(page))
> > > > + return 1;
> > > > + if (page_mapped(page))
> > > > + return page_mapped_by_current(page);
> > > > +
> > > > + return 1;
> > > > +}
> > >
> > > I'd much rather VM internal type stuff get moved *out* of kernel/power :(
> >
> > Well, I kind of agree, but I don't know where to place it under mm/.
> >
> > > It needs more comments too. Also, how important is it for the page to be
> > > off the LRU?
> >
> > Hm, I'm not sure if that's what you're asking about, but the pages off the LRU
> > are handled in a usual way, ie. copied when snapshotting the system. The
> > pages _on_ the LRU may be included in the snapshot image without
> > copying, but I require them additionally to be (a) mapped by someone and
> > (b) not mapped by the current task.
>
> Why do you _want_ them mapped by someone?

Because this means they belong to a task that is frozen and won't touch them
(of course unless it's us). The kernel has no reason to access them either
(even after we resume devices) except for reclaiming, but that's handled
explicitly. Thus it's safe to include them in the image without copying.

As I said before, I think the page cache pages may be treated this way too.
It probably applies to all of the LRU pages, but there may be some corner
cases. The mapped pages are just easy to single out.

> > > b) Why are you clearing PageLRU outside the spinlock?
> >
> > Well, good question. ;-) Moreover it seems I don't need to acquire the
> > spinlock at all, because this is done on one CPU with IRQs disabled and the
> > other tasks frozen.
>
> Well, it is probably better to still take the spinlock. That way, if
> something goes wrong we get deadlock, not anything worse.

OK, so I'll just clear/set the LRU flag under the spinlock.

Greetings,
Rafael