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 23:02, 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 will keep snapshot.c
> > > smaller/simpler,
> > 
> > Speaking of simplifications and having seen your code I hope you will agree with
> > the appended patch against vanilla 2.6.14-git3 (it reduces the duplication of code,
> > and replaces swsusp_pagedir_relocate with a simpler mechanism).
> 
> ...and also moves stuff around in a way
> 
> a) I don't like
> 
> and
> 
> b) is almost impossible to review

OK, I'll try to split it into two patches to make it cleaner.

> :-). Can you keep "relocate" code in swsusp.c, just making it simpler?

If you mean mark_unsafe_pages() and copy_backup_list(), no problem.
The rest is still there.

> 
> > @@ -997,20 +870,22 @@
> >  	int error = 0;
> >  	struct pbe *p;
> >  
> > -	if (!(p = alloc_pagedir(nr_copy_pages)))
> > +	if (!(p = alloc_pagedir(nr_copy_pages, 0)))
> >  		return -ENOMEM;
> >  
> >  	if ((error = read_pagedir(p)))
> >  		return error;
> > -
> >  	create_pbe_list(p, nr_copy_pages);
> > -
> > -	if (!(pagedir_nosave = swsusp_pagedir_relocate(p)))
> > +	mark_unsafe_pages(p);
> > +	if (!(pagedir_nosave = alloc_pagedir(nr_copy_pages, 1)))
> >  		return -ENOMEM;
> 
> Okay, this is probably better approach than copying pagedir around...

It's nice you agree here. :-)

Greetings,
Rafael