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

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

Hi,

On Monday, 31 of October 2005 22:59, Pavel Machek wrote:
}-- snip --{
> 
> > > > - 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.

That depends on how you implement the interface.  If you insist on using
ioctls then yes, it's ugly.  However, if it is a file in sysfs, for example,
then you have well-defined open(), close(), read() and write() operations
and it is assumed you will keep some context accross eg. write()s.
 
> > > > 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.

We loose the saved system state, but the kernel that has just booted is
supposed to continue, so we should make it possible.  Alternatively,
we can do something like a panic and force the user to reboot,
in which case we can forget about the error paths, freeing memory
etc. altogether.

Greetings,
Rafael