Re: [PATCH 2/4] swsusp: clean up resume error path

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

Hi,

On Tuesday, 18 of October 2005 01:47, Pavel Machek wrote:
> Hi!
> 
> > The following patch removes an incorrect call to restore_highmem() from
> > the resume error path (there's no saved highmem in that case) and makes
> > swsusp touch the softlockup watchdog if there's no error (currently it only
> > touches the watchdog if an error occurs).
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Index: linux-2.6.14-rc4-mm1/kernel/power/swsusp.c
> > ===================================================================
> > --- linux-2.6.14-rc4-mm1.orig/kernel/power/swsusp.c	2005-10-17 23:28:34.000000000 +0200
> > +++ linux-2.6.14-rc4-mm1/kernel/power/swsusp.c	2005-10-17 23:28:47.000000000 +0200
> > @@ -628,7 +629,6 @@
> >  	 */
> >  	swsusp_free();
> >  	restore_processor_state();
> > -	restore_highmem();
> >  	touch_softlockup_watchdog();
> >  	device_power_up();
> >  	local_irq_enable();
> 
> I don't like this one. restore_highmem() does freeing of allocated
> pages. If swsusp_arch_suspend() fails in specific way, I suspect it
> could leak highmem.

The pages to be freed are only allocated in suspend_prepare_image()
(now swsusp_save()), which is on suspend, and this is the resume
error path.

The boot kernel that performs the resume does not save highmem,
so it need not and IMO it should not call restore_highmem() in the
error path (if nothing more, it's misleading).  OTOH if the resume
succeeds, restore_highmem() will be called from swsusp_suspend().

Gretings,
Rafael