Re: [pm] fix oops after saving image

[Pavel Machek]

Hi!

> > I do not want to waste 4K, does this look better?
> > 									Pavel
> > 
> > --- tmp/linux/kernel/power/swsusp.c	2003-10-02 22:29:06.000000000 +0200
> > +++ linux/kernel/power/swsusp.c	2003-10-02 22:27:07.000000000 +0200
> > @@ -283,6 +283,9 @@
> >  	unsigned long address;
> >  	struct page *page;
> >  
> > +	if (!buffer)
> > +		return -ENOMEM;
> > +
> >  	printk( "Writing data to swap (%d pages): ", nr_copy_pages );
> >  	for (i=0; i<nr_copy_pages; i++) {
> >  		if (!(i%100))
> 
> Argh! This bit was in the previous patch I applied. Please get it 
> straight, or just keep adding to one patch and resending it (with an 
> itemized list of changes). 

I do not want to do that (other people would have problems
reviewing). I'll try to be more carefull.

One thing would help me: could you reply with "Applied" when you apply
some patch? Just now it seems to be silence==applied and reply==some
problem, but that makes it "interesting" to guess what your tree looks
like.

> > @@ -345,7 +348,7 @@
> >  	printk( "|\n" );
> >  
> >  	MDELAY(1000);
> > -	free_page((unsigned long) buffer);
> > +	/* No need to free anything, system is going down, anyway. */
> >  	return 0;
> >  }
> 
> It's technically still incorrect, since you'd still be leaking memory if
> suspend failed. And, the comment is still in an unfortunate place.  
> Something like this, before the function helps to provide understanding of 
> the entire operation:

At this point, suspend may no longer fail: we have ready-to-run image
in swap, and rollback would happen on next reboot -- corrupting
data. Yep better docs is needed.

How about this one?
								Pavel

--- tmp/linux/kernel/power/swsusp.c	2003-10-04 00:21:55.000000000 +0200
+++ linux/kernel/power/swsusp.c	2003-10-04 00:17:19.000000000 +0200
@@ -274,6 +274,17 @@
 	swap_list_unlock();
 }
 
+/**
+ *    write_suspend_image - Write entire image to disk.
+ *
+ *    After writing suspend signature to the disk, suspend may no
+ *    longer fail: we have ready-to-run image in swap, and rollback
+ *    would happen on next reboot -- corrupting data.
+ *
+ *    Note: The buffer we allocate to use to write the suspend header is
+ *    not freed; its not needed since system is going down anyway
+ *    (plus it causes oops and I'm lazy^H^H^H^Htoo busy).
+ */
 static int write_suspend_image(void)
 {
 	int i;
@@ -345,7 +359,6 @@
 	printk( "|\n" );
 
 	MDELAY(1000);
-	free_page((unsigned long) buffer);
 	return 0;
 }
 

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/