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

[Pavel Machek <pavel@ucw.cz>]

Hi!

> > Can you elaborate? resume is certainly going to get list of pbes...
> 
> OK
> On x86-64 we have to allocate a few safe pages to put the temporary page
> tables on them.  In principle I can imagine the following code for this:
> 
> do {
> 	get a page;
> 	walk the list of pbes to verify that the page is safe;
> 	if (the page is not safe)
> 		keep track of it;
> } while (the page is not safe)
> 
> but I'd rather not like to propose Andi to merge it. ;-)  Currently the x86-64 arch
> code uses the same method of marking non-safe pages that is used by
> the rest of swsusp for efficiency and I think it should stay this
> way.

Ok, I see.

> > > - sys_create_pagedir
> > 
> > Ugly...
> 
> Oh, it can be done on-the-fly in
> sys_put_this_stuff_where_appropriate(image data) (at the expense of one
> redundant check per call).

Yes, but it is still ugly, as you keep some context across the
syscalls.

> > > Cleanup: /* certainly something's gone wrong */
> > > - sys_destroy_pagedir /* that's it */
> > > - sys_resume_devices
> > 
> > You should not need to do this one. resuming devices is going to be
> > integrated in atomic_restore, because suspending devices is there, too.
> 
> Yes, but I need to thaw processes anyway, so I can release memory as well.
> OTOH, if sys_atomic_restore fails because of the lack of memory, the memory
> should be freed _before_ resuming devices, since otherwise subsequent
> failures are almost certain to appear (I've seen what happens in that case).
> Now, if the memory is allocated by the kernel, I can easily put an
> emergency memory-freeing call in sys_atomic_restore (in that case
> sys_destroy_pagedir will be redundant, but so what?).

Ugh, I'd say "don't care about this one too much". If resume is
failing, we have bad problems anyway.

> > Here's how it looks... additionaly, I have ioctl for getting one
> > usable page. It is true that I did not solve error paths, yet; I'll
> > certainly need some way to free memory, too.
> 
> IMHO, these are important issues.

Yes, but I do not expect any problems while actually coding that...

								Pavel
-- 
Thanks, Sharp!