Re: [pm] fix oops after saving image

Pavel Machek
2003-10-02T20:39:20Z

Hi!

> > --- tmp/linux/kernel/power/swsusp.c 2003-10-02 00:04:35.000000000 +0200
> > +++ linux/kernel/power/swsusp.c 2003-10-01 23:56:49.000000000 +0200
> > @@ -345,7 +348,7 @@
> > printk( "|\n" );
> >
> > MDELAY(1000);
> > - free_page((unsigned long) buffer);
> > + /* Trying to free_page((unsigned long) buffer) here is bad idea, not sure why */
> > return 0;
> > }
>
> Patches like this really do a disservice to anyone trying to read the code
> and figure out what is going on. I've spent a considerable amount of time
> deciphering and santizing the swsusp code, which is why pmdisk exists.
>
> The patch is simply a band-aid, and completely meaningless without the
> context of the email. If I applied this, one would be able to ascertain
> the reason for the patch, if they manipulated the BK tools correctly.
> However, seeing that line solely in the context on the rest of the source
> makes one cock their head, squint their eyes and pray that they never have
> to look at that file again.
>
> If you're seeing an Oops, please search more for the cause and submit a
> real fix for it.
>
> Or, simply change the semantics of the code enough to eliminate the
> possibility of a problem. In the pmdisk code, I've statically declared the
> header, so we don't need that alloc/free. Please see that file for an
> example.

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))
@@ -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;
}

--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]