Re: [pm] fix oops after saving image

Pavel Machek
2003-10-03T22:34:17Z

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?]