Re: [PATCH 2/3] swsusp: move snapshot-handling functions to snapshot.c

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

Hi,

On Sunday, 30 of October 2005 20:52, Pavel Machek wrote:
> Hi!
> 
> > This patch moves the snapshot-handling functions remaining in swsusp.c
> > to snapshot.c (ie. it moves the code without changing the functionality).
> >
> I'm sorry, but I acked this one too quickly. I'd prefer to keep "relocate" code where
> it is, and define "must not collide" as a part of interface.

That's doable, but frankly I don't like the idea.

> That will keep snapshot.c smaller/simpler, and I plan to
> eventually put responsibility for relocation to userspace.

Please note that the relocating code uses the page flags to mark the allocated
pages as well as to avoid the pages that should not be used.  In my opinion
no userspace process should be allowed to fiddle with the page flags.

Moreover, get_safe_page() is called directly by the arch code on x86-64,
so it has to stay in the kernel and hence it should be in snapshot.c.
OTOH the relocating code is nothing more than "if the page is not safe,
use get_safe_page() to allocate one" kind of thing, so I don't see a point
in taking it out of the kernel (in the future) too.

> That should simplify error handling at least: data structures
> needed for relocation can be kept in userspace memory,

Well, after the patches that are already in -mm we don't use any additional
data structures for this purpose, so that's not a problem, I think. ;-)

> and therefore we do not risk memory leak in case something goes wrong.

We don't.  All memory allocated with either get_image_page() or
get_safe_page() will eventually be released by swsusp_free(), no matter
what happens.

Greetings,
Rafael