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

[Pavel Machek <pavel@ucw.cz>]

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.

Of course, userspace would have to use separate data structure. [Hash table?]

> 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.

Well... for resume. If userspace does the allocation, it is:

userspace reads image
userspace relocates it
sys_atomic_restore(image)
if something goes wrong, userspace is clearly responsible for freeing
it.

How would you propose kernel<->user interface?

userspace reads pagedir
sys_these_pages_are_forbidden(pagedir)
userspace reads rest
sys_atomic_restore(image)
if something goes wrong, userspace must dealocate pages _and_ clear
forbidden flags?

> > 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. ;-)

Until someone will want to get page flag bits back ;-), ok.

								Pavel
-- 
Thanks, Sharp!