[PATCH 0/6] Suspend and hibernation code cleanups

[PATCH 0/6] Suspend and hibernation code cleanups

[Rafael J. Wysocki]

Hi,

The following series of patches contains some suspend and hibernation code
cleanups that I'd like to go into 2.6.25.

The patches are on top of the suspend branch of the linux-acpi-2.6 tree.

Please review.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH 1/6] Suspend: Fix compilation warning for CONFIG_SUSPEND unset

[Rafael J. Wysocki]

Suspend: Make debug facility depend on CONFIG_SUSPEND

Make the new suspend debug facility code depend on CONFIG_SUSPEND,
as appropriate, to remove the compiler warning printed when CONFIG_PM is set
and CONFIG_SUSPEND is not set.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/main.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -52,6 +52,8 @@ int pm_notifier_call_chain(unsigned long
 
 #endif /* CONFIG_PM_SLEEP */
 
+#ifdef CONFIG_SUSPEND
+
 #ifdef CONFIG_PM_DEBUG
 int pm_test_level = TEST_NONE;
 
@@ -125,9 +127,6 @@ power_attr(pm_test);
 static inline int suspend_test(int level) { return 0; }
 #endif /* !CONFIG_PM_DEBUG */
 
-
-#ifdef CONFIG_SUSPEND
-
 /* This is just an arbitrary number */
 #define FREE_PAGE_NUMBER (100)
 

Re: [PATCH 1/6] Suspend: Fix compilation warning for CONFIG_SUSPEND unset

[Pavel Machek]

On Tue 2007-11-27 00:27:23, Rafael J. Wysocki wrote:
> Suspend: Make debug facility depend on CONFIG_SUSPEND
> 
> Make the new suspend debug facility code depend on CONFIG_SUSPEND,
> as appropriate, to remove the compiler warning printed when CONFIG_PM is set
> and CONFIG_SUSPEND is not set.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 2/6] Hibernation: Move low level resume to disk.c

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Move the low level restore code to kernel/power/disk.c , since the
corresponding low level hibernation code is already there.

Make restore fail if device_power_down(PMSG_PRETHAW) returns an
error.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/disk.c   |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/power/power.h  |    1 -
 kernel/power/swsusp.c |   35 -----------------------------------
 3 files changed, 46 insertions(+), 37 deletions(-)

Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -275,6 +275,51 @@ int hibernation_snapshot(int platform_mo
 }
 
 /**
+ *	resume_target_kernel - prepare devices that need to be suspended with
+ *	interrupts off, restore the contents of highmem that have not been
+ *	restored yet from the image and run the low level code that will restore
+ *	the remaining contents of memory and switch to the just restored target
+ *	kernel.
+ */
+
+static int resume_target_kernel(void)
+{
+	int error;
+
+	local_irq_disable();
+	error = device_power_down(PMSG_PRETHAW);
+	if (error) {
+		printk(KERN_ERR "Some devices failed to power down, "
+			KERN_ERR "aborting resume\n");
+		goto Enable_irqs;
+	}
+	/* We'll ignore saved state, but this gets preempt count (etc) right */
+	save_processor_state();
+	error = restore_highmem();
+	if (!error) {
+		error = swsusp_arch_resume();
+		/* The code below is only ever reached in case of a failure.
+		 * Otherwise execution continues at place where
+		 * swsusp_arch_suspend() was called
+        	 */
+		BUG_ON(!error);
+		/* This call to restore_highmem() undos the previous one */
+		restore_highmem();
+	}
+	/* The only reason why swsusp_arch_resume() can fail is memory being
+	 * very tight, so we have to free it as soon as we can to avoid
+	 * subsequent failures
+	 */
+	swsusp_free();
+	restore_processor_state();
+	touch_softlockup_watchdog();
+	device_power_up();
+ Enable_irqs:
+	local_irq_enable();
+	return error;
+}
+
+/**
  *	hibernation_restore - quiesce devices and restore the hibernation
  *	snapshot image.  If successful, control returns in hibernation_snaphot()
  *	@platform_mode - if set, use the platform driver, if available, to
@@ -297,7 +342,7 @@ int hibernation_restore(int platform_mod
 	if (!error) {
 		error = disable_nonboot_cpus();
 		if (!error)
-			error = swsusp_resume();
+			error = resume_target_kernel();
 		enable_nonboot_cpus();
 	}
 	platform_restore_cleanup(platform_mode);
Index: linux-2.6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.orig/kernel/power/swsusp.c
+++ linux-2.6/kernel/power/swsusp.c
@@ -261,38 +261,3 @@ int swsusp_shrink_memory(void)
 
 	return 0;
 }
-
-int swsusp_resume(void)
-{
-	int error;
-
-	local_irq_disable();
-	/* NOTE:  device_power_down() is just a suspend() with irqs off;
-	 * it has no special "power things down" semantics
-	 */
-	if (device_power_down(PMSG_PRETHAW))
-		printk(KERN_ERR "Some devices failed to power down, very bad\n");
-	/* We'll ignore saved state, but this gets preempt count (etc) right */
-	save_processor_state();
-	error = restore_highmem();
-	if (!error) {
-		error = swsusp_arch_resume();
-		/* The code below is only ever reached in case of a failure.
-		 * Otherwise execution continues at place where
-		 * swsusp_arch_suspend() was called
-        	 */
-		BUG_ON(!error);
-		/* This call to restore_highmem() undos the previous one */
-		restore_highmem();
-	}
-	/* The only reason why swsusp_arch_resume() can fail is memory being
-	 * very tight, so we have to free it as soon as we can to avoid
-	 * subsequent failures
-	 */
-	swsusp_free();
-	restore_processor_state();
-	touch_softlockup_watchdog();
-	device_power_up();
-	local_irq_enable();
-	return error;
-}
Index: linux-2.6/kernel/power/power.h
===================================================================
--- linux-2.6.orig/kernel/power/power.h
+++ linux-2.6/kernel/power/power.h
@@ -154,7 +154,6 @@ extern int swsusp_swap_in_use(void);
 extern int swsusp_check(void);
 extern int swsusp_shrink_memory(void);
 extern void swsusp_free(void);
-extern int swsusp_resume(void);
 extern int swsusp_read(unsigned int *flags_p);
 extern int swsusp_write(unsigned int flags);
 extern void swsusp_close(void);

Re: [PATCH 2/6] Hibernation: Move low level resume to disk.c

[Pavel Machek]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move the low level restore code to kernel/power/disk.c , since the
> corresponding low level hibernation code is already there.
> 
> Make restore fail if device_power_down(PMSG_PRETHAW) returns an
> error.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Move looks ok to me.


>  /**
> + *	resume_target_kernel - prepare devices that need to be suspended with
> + *	interrupts off, restore the contents of highmem that have not been
> + *	restored yet from the image and run the low level code that will restore
> + *	the remaining contents of memory and switch to the just restored target
> + *	kernel.
> + */
> +
> +static int resume_target_kernel(void)
> +{
> +	int error;
> +
> +	local_irq_disable();
> +	error = device_power_down(PMSG_PRETHAW);
> +	if (error) {
> +		printk(KERN_ERR "Some devices failed to power down, "
> +			KERN_ERR "aborting resume\n");
> +		goto Enable_irqs;

Too many KERN_ERR's ;-). Plus... are you sure this does the right
thing? ... .. ... yes, it does. If one device fails,
device_power_down() powers up the rest using dpm_power_up(). Good.

ACK... modulo double KERN_ERR.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 2/6] Hibernation: Move low level resume to disk.c

[Rafael J. Wysocki]

On Tuesday, 27 of November 2007, Pavel Machek wrote:
> Hi!
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Move the low level restore code to kernel/power/disk.c , since the
> > corresponding low level hibernation code is already there.
> > 
> > Make restore fail if device_power_down(PMSG_PRETHAW) returns an
> > error.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move looks ok to me.
> 
> 
> >  /**
> > + *	resume_target_kernel - prepare devices that need to be suspended with
> > + *	interrupts off, restore the contents of highmem that have not been
> > + *	restored yet from the image and run the low level code that will restore
> > + *	the remaining contents of memory and switch to the just restored target
> > + *	kernel.
> > + */
> > +
> > +static int resume_target_kernel(void)
> > +{
> > +	int error;
> > +
> > +	local_irq_disable();
> > +	error = device_power_down(PMSG_PRETHAW);
> > +	if (error) {
> > +		printk(KERN_ERR "Some devices failed to power down, "
> > +			KERN_ERR "aborting resume\n");
> > +		goto Enable_irqs;
> 
> Too many KERN_ERR's ;-)

Yeah, will fix.

> . Plus... are you sure this does the right 
> thing? ... .. ... yes, it does. If one device fails,
> device_power_down() powers up the rest using dpm_power_up(). Good.
> 
> ACK... modulo double KERN_ERR.

OK, thanks!

Rafael

[PATCH 3/6] Suspend: Use common prefix in messages

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Make suspend messages start with one common prefix "PM: ".

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/main.c  |   19 ++++++++++---------
 kernel/power/power.h |    2 ++
 2 files changed, 12 insertions(+), 9 deletions(-)

Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -183,11 +183,11 @@ static int suspend_prepare(void)
 
 	free_pages = global_page_state(NR_FREE_PAGES);
 	if (free_pages < FREE_PAGE_NUMBER) {
-		pr_debug("PM: free some memory\n");
+		pr_debug(PREFIX "Free some memory\n");
 		shrink_all_memory(FREE_PAGE_NUMBER - free_pages);
 		if (nr_free_pages() < FREE_PAGE_NUMBER) {
 			error = -ENOMEM;
-			printk(KERN_ERR "PM: No enough memory\n");
+			printk(KERN_ERR PREFIX "No enough memory\n");
 		}
 	}
 	if (!error)
@@ -227,7 +227,7 @@ static int suspend_enter(suspend_state_t
 	BUG_ON(!irqs_disabled());
 
 	if ((error = device_power_down(PMSG_SUSPEND))) {
-		printk(KERN_ERR "Some devices failed to power down\n");
+		printk(KERN_ERR PREFIX "Some devices failed to power down\n");
 		goto Done;
 	}
 
@@ -261,7 +261,7 @@ int suspend_devices_and_enter(suspend_st
 	suspend_console();
 	error = device_suspend(PMSG_SUSPEND);
 	if (error) {
-		printk(KERN_ERR "Some devices failed to suspend\n");
+		printk(KERN_ERR PREFIX "Some devices failed to suspend\n");
 		goto Resume_console;
 	}
 
@@ -344,11 +344,12 @@ static int enter_state(suspend_state_t s
 	if (!mutex_trylock(&pm_mutex))
 		return -EBUSY;
 
-	printk("Syncing filesystems ... ");
+	printk(KERN_INFO PREFIX "Syncing filesystems ... ");
 	sys_sync();
-	printk("done.\n");
+	printk(KERN_INFO "done.\n");
 
-	pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]);
+	pr_debug(PREFIX "Preparing system to enter %s sleep state\n",
+			pm_states[state]);
 	error = suspend_prepare();
 	if (error)
 		goto Unlock;
@@ -356,11 +357,11 @@ static int enter_state(suspend_state_t s
 	if (suspend_test(TEST_FREEZER))
 		goto Finish;
 
-	pr_debug("PM: Entering %s sleep\n", pm_states[state]);
+	pr_debug(PREFIX "Entering %s sleep state\n", pm_states[state]);
 	error = suspend_devices_and_enter(state);
 
  Finish:
-	pr_debug("PM: Finishing wakeup.\n");
+	pr_debug(PREFIX "Finishing wakeup.\n");
 	suspend_finish();
  Unlock:
 	mutex_unlock(&pm_mutex);
Index: linux-2.6/kernel/power/power.h
===================================================================
--- linux-2.6.orig/kernel/power/power.h
+++ linux-2.6/kernel/power/power.h
@@ -2,6 +2,8 @@
 #include <linux/suspend_ioctls.h>
 #include <linux/utsname.h>
 
+#define PREFIX	"PM: "
+
 struct swsusp_info {
 	struct new_utsname	uts;
 	u32			version_code;

Re: [PATCH 3/6] Suspend: Use common prefix in messages

[Pavel Machek]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Make suspend messages start with one common prefix "PM: ".
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

I do not think I like the PREFIX idea (see other message).


> @@ -344,11 +344,12 @@ static int enter_state(suspend_state_t s
>  	if (!mutex_trylock(&pm_mutex))
>  		return -EBUSY;
>  
> -	printk("Syncing filesystems ... ");
> +	printk(KERN_INFO PREFIX "Syncing filesystems ... ");
>  	sys_sync();
> -	printk("done.\n");
> +	printk(KERN_INFO "done.\n");


Plus this is wrong.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 3/6] Suspend: Use common prefix in messages

[Rafael J. Wysocki]

On Tuesday, 27 of November 2007, Pavel Machek wrote:
> Hi!
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Make suspend messages start with one common prefix "PM: ".
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> I do not think I like the PREFIX idea (see other message).

The problem is that we use three different types of messages:
- with no prefix at all
- with "PM: "
- with "swsusp: "
and I think that we should use just one, preferably "PM: ".

> > @@ -344,11 +344,12 @@ static int enter_state(suspend_state_t s
> >  	if (!mutex_trylock(&pm_mutex))
> >  		return -EBUSY;
> >  
> > -	printk("Syncing filesystems ... ");
> > +	printk(KERN_INFO PREFIX "Syncing filesystems ... ");
> >  	sys_sync();
> > -	printk("done.\n");
> > +	printk(KERN_INFO "done.\n");
> 
> 
> Plus this is wrong.

Yes, I've had a temporary blackout regarding prinkt(), not even making much
sense ...

I'll resend the entire series in a while.

Greetings,
Rafael

[PATCH 4/6] Suspend: Fix comment in main.c

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Fix a comment in kernel/power/main.c so that it doesn't contain lines
longer that 80 characters.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/power/main.c
===================================================================
--- linux-2.6.orig/kernel/power/main.c
+++ linux-2.6/kernel/power/main.c
@@ -242,8 +242,8 @@ static int suspend_enter(suspend_state_t
 }
 
 /**
- *	suspend_devices_and_enter - suspend devices and enter the desired system sleep
- *			  state.
+ *	suspend_devices_and_enter - suspend devices and enter the desired system
+ *				    sleep state.
  *	@state:		  state to enter
  */
 int suspend_devices_and_enter(suspend_state_t state)

Re: [PATCH 4/6] Suspend: Fix comment in main.c

[Pavel Machek]

On Tue 2007-11-27 00:31:21, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Fix a comment in kernel/power/main.c so that it doesn't contain lines
> longer that 80 characters.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ACK.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 5/6] Hibernation: Use common prefix in messages

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Make hibernation messages start with one common prefix "PM: ".

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/disk.c     |   38 ++++++++++++++++++++------------------
 kernel/power/snapshot.c |   24 +++++++++++++-----------
 kernel/power/swap.c     |   43 +++++++++++++++++++++++--------------------
 kernel/power/swsusp.c   |   10 +++++-----
 4 files changed, 61 insertions(+), 54 deletions(-)

Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -191,7 +191,7 @@ int create_image(int platform_mode)
 	 */
 	error = device_power_down(PMSG_FREEZE);
 	if (error) {
-		printk(KERN_ERR "Some devices failed to power down, "
+		printk(KERN_ERR PREFIX "Some devices failed to power down, "
 			KERN_ERR "aborting suspend\n");
 		goto Enable_irqs;
 	}
@@ -203,7 +203,8 @@ int create_image(int platform_mode)
 	save_processor_state();
 	error = swsusp_arch_suspend();
 	if (error)
-		printk(KERN_ERR "Error %d while creating the image\n", error);
+		printk(KERN_ERR PREFIX "Error %d while creating the image\n",
+			error);
 	/* Restore control flow magically appears here */
 	restore_processor_state();
 	if (!in_suspend)
@@ -289,7 +290,7 @@ static int resume_target_kernel(void)
 	local_irq_disable();
 	error = device_power_down(PMSG_PRETHAW);
 	if (error) {
-		printk(KERN_ERR "Some devices failed to power down, "
+		printk(KERN_ERR PREFIX "Some devices failed to power down, "
 			KERN_ERR "aborting resume\n");
 		goto Enable_irqs;
 	}
@@ -436,7 +437,7 @@ static void power_down(void)
 	 * Valid image is on the disk, if we continue we risk serious data
 	 * corruption after resume.
 	 */
-	printk(KERN_CRIT "Please power me down manually\n");
+	printk(KERN_CRIT PREFIX "Please power down manually\n");
 	while(1);
 }
 
@@ -482,9 +483,9 @@ int hibernate(void)
 	if (error)
 		goto Exit;
 
-	printk("Syncing filesystems ... ");
+	printk(KERN_INFO PREFIX "Syncing filesystems ... ");
 	sys_sync();
-	printk("done.\n");
+	printk(KERN_INFO "done.\n");
 
 	error = prepare_processes();
 	if (error)
@@ -502,13 +503,13 @@ int hibernate(void)
 
 		if (hibernation_mode == HIBERNATION_PLATFORM)
 			flags |= SF_PLATFORM_MODE;
-		pr_debug("PM: writing image.\n");
+		pr_debug(PREFIX "Writing image.\n");
 		error = swsusp_write(flags);
 		swsusp_free();
 		if (!error)
 			power_down();
 	} else {
-		pr_debug("PM: Image restored successfully.\n");
+		pr_debug(PREFIX "Image restored successfully.\n");
 		swsusp_free();
 	}
  Thaw:
@@ -558,10 +559,11 @@ static int software_resume(void)
 			return -ENOENT;
 		}
 		swsusp_resume_device = name_to_dev_t(resume_file);
-		pr_debug("swsusp: Resume From Partition %s\n", resume_file);
+		pr_debug(PREFIX "Resume from fartition %s\n", resume_file);
 	} else {
-		pr_debug("swsusp: Resume From Partition %d:%d\n",
-			 MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
+		pr_debug(PREFIX "Resume from partition %d:%d\n",
+			 MAJOR(swsusp_resume_device),
+			 MINOR(swsusp_resume_device));
 	}
 
 	if (noresume) {
@@ -573,7 +575,7 @@ static int software_resume(void)
 		return 0;
 	}
 
-	pr_debug("PM: Checking swsusp image.\n");
+	pr_debug(PREFIX "Checking hibernation image.\n");
 	error = swsusp_check();
 	if (error)
 		goto Unlock;
@@ -592,20 +594,20 @@ static int software_resume(void)
 	if (error)
 		goto Finish;
 
-	pr_debug("PM: Preparing processes for restore.\n");
+	pr_debug(PREFIX "Preparing processes for restore.\n");
 	error = prepare_processes();
 	if (error) {
 		swsusp_close();
 		goto Done;
 	}
 
-	pr_debug("PM: Reading swsusp image.\n");
+	pr_debug(PREFIX "Reading hibernation image.\n");
 
 	error = swsusp_read(&flags);
 	if (!error)
 		hibernation_restore(flags & SF_PLATFORM_MODE);
 
-	printk(KERN_ERR "PM: Restore failed, recovering.\n");
+	printk(KERN_ERR PREFIX "Restore failed, recovering.\n");
 	swsusp_free();
 	unprepare_processes();
  Done:
@@ -616,7 +618,7 @@ static int software_resume(void)
 	/* For success case, the suspend path will release the lock */
  Unlock:
 	mutex_unlock(&pm_mutex);
-	pr_debug("PM: Resume from disk failed.\n");
+	pr_debug(PREFIX "Resume from disk failed.\n");
 	return error;
 }
 
@@ -724,7 +726,7 @@ static ssize_t disk_store(struct kset *k
 		error = -EINVAL;
 
 	if (!error)
-		pr_debug("PM: suspend-to-disk mode set to '%s'\n",
+		pr_debug(PREFIX "Hibernation mode set to '%s'\n",
 			 hibernation_modes[mode]);
 	mutex_unlock(&pm_mutex);
 	return error ? error : n;
@@ -754,7 +756,7 @@ static ssize_t resume_store(struct kset 
 	mutex_lock(&pm_mutex);
 	swsusp_resume_device = res;
 	mutex_unlock(&pm_mutex);
-	printk("Attempting manual resume\n");
+	printk(KERN_INFO PREFIX "Starting manual resume from disk\n");
 	noresume = 0;
 	software_resume();
 	ret = n;
Index: linux-2.6/kernel/power/snapshot.c
===================================================================
--- linux-2.6.orig/kernel/power/snapshot.c
+++ linux-2.6/kernel/power/snapshot.c
@@ -635,7 +635,7 @@ __register_nosave_region(unsigned long s
 	region->end_pfn = end_pfn;
 	list_add_tail(&region->list, &nosave_regions);
  Report:
-	printk("swsusp: Registered nosave memory region: %016lx - %016lx\n",
+	printk(PREFIX "Registered nosave memory region: %016lx - %016lx\n",
 		start_pfn << PAGE_SHIFT, end_pfn << PAGE_SHIFT);
 }
 
@@ -704,7 +704,7 @@ static void mark_nosave_pages(struct mem
 	list_for_each_entry(region, &nosave_regions, list) {
 		unsigned long pfn;
 
-		printk("swsusp: Marking nosave pages: %016lx - %016lx\n",
+		printk(PREFIX "Marking nosave pages: %016lx - %016lx\n",
 				region->start_pfn << PAGE_SHIFT,
 				region->end_pfn << PAGE_SHIFT);
 
@@ -749,7 +749,7 @@ int create_basic_memory_bitmaps(void)
 	free_pages_map = bm2;
 	mark_nosave_pages(forbidden_pages_map);
 
-	printk("swsusp: Basic memory bitmaps created\n");
+	printk(PREFIX "Basic memory bitmaps created\n");
 
 	return 0;
 
@@ -784,7 +784,7 @@ void free_basic_memory_bitmaps(void)
 	memory_bm_free(bm2, PG_UNSAFE_CLEAR);
 	kfree(bm2);
 
-	printk("swsusp: Basic memory bitmaps freed\n");
+	printk(PREFIX "Basic memory bitmaps freed\n");
 }
 
 /**
@@ -1088,7 +1088,8 @@ static int enough_free_mem(unsigned int 
 	}
 
 	nr_pages += count_pages_for_highmem(nr_highmem);
-	pr_debug("swsusp: Normal pages needed: %u + %u + %u, available pages: %u\n",
+	pr_debug(PREFIX "Normal pages needed: %u + %u + %u, "
+		"available pages: %u\n",
 		nr_pages, PAGES_FOR_IO, meta, free);
 
 	return free > nr_pages + PAGES_FOR_IO + meta;
@@ -1201,20 +1202,20 @@ asmlinkage int swsusp_save(void)
 {
 	unsigned int nr_pages, nr_highmem;
 
-	printk("swsusp: critical section: \n");
+	printk(PREFIX "Critical section: \n");
 
 	drain_local_pages();
 	nr_pages = count_data_pages();
 	nr_highmem = count_highmem_pages();
-	printk("swsusp: Need to copy %u pages\n", nr_pages + nr_highmem);
+	printk(PREFIX "Need to copy %u pages\n", nr_pages + nr_highmem);
 
 	if (!enough_free_mem(nr_pages, nr_highmem)) {
-		printk(KERN_ERR "swsusp: Not enough free memory\n");
+		printk(KERN_ERR PREFIX "Not enough free memory\n");
 		return -ENOMEM;
 	}
 
 	if (swsusp_alloc(&orig_bm, &copy_bm, nr_pages, nr_highmem)) {
-		printk(KERN_ERR "swsusp: Memory allocation failed\n");
+		printk(KERN_ERR PREFIX "Memory allocation failed\n");
 		return -ENOMEM;
 	}
 
@@ -1234,7 +1235,8 @@ asmlinkage int swsusp_save(void)
 	nr_copy_pages = nr_pages;
 	nr_meta_pages = DIV_ROUND_UP(nr_pages * sizeof(long), PAGE_SIZE);
 
-	printk("swsusp: critical section: done (%d pages copied)\n", nr_pages);
+	printk(KERN_INFO PREFIX "Critical section: done (%d pages copied)\n",
+		nr_pages);
 
 	return 0;
 }
@@ -1433,7 +1435,7 @@ static int check_header(struct swsusp_in
 	if (!reason && info->num_physpages != num_physpages)
 		reason = "memory size";
 	if (reason) {
-		printk(KERN_ERR "swsusp: Resume mismatch: %s\n", reason);
+		printk(KERN_ERR PREFIX "Resume mismatch: %s\n", reason);
 		return -EPERM;
 	}
 	return 0;
Index: linux-2.6/kernel/power/swap.c
===================================================================
--- linux-2.6.orig/kernel/power/swap.c
+++ linux-2.6/kernel/power/swap.c
@@ -73,7 +73,8 @@ static int submit(int rw, pgoff_t page_o
 	bio->bi_end_io = end_swap_bio_read;
 
 	if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
-		printk("swsusp: ERROR: adding page to bio at %ld\n", page_off);
+		printk(KERN_ERR PREFIX "Error adding page to bio at %ld\n",
+			page_off);
 		bio_put(bio);
 		return -EFAULT;
 	}
@@ -153,7 +154,7 @@ static int mark_swapfiles(sector_t start
 		error = bio_write_page(swsusp_resume_block,
 					swsusp_header, NULL);
 	} else {
-		printk(KERN_ERR "swsusp: Swap header not found!\n");
+		printk(KERN_ERR PREFIX "Swap header not found!\n");
 		error = -ENODEV;
 	}
 	return error;
@@ -325,7 +326,8 @@ static int save_image(struct swap_map_ha
 	struct timeval start;
 	struct timeval stop;
 
-	printk("Saving image data pages (%u pages) ...     ", nr_to_write);
+	printk(KERN_INFO PREFIX "Saving image data pages (%u pages) ...     ",
+		nr_to_write);
 	m = nr_to_write / 100;
 	if (!m)
 		m = 1;
@@ -340,7 +342,7 @@ static int save_image(struct swap_map_ha
 			if (error)
 				break;
 			if (!(nr_pages % m))
-				printk("\b\b\b\b%3d%%", nr_pages / m);
+				printk(KERN_INFO "\b\b\b\b%3d%%", nr_pages / m);
 			nr_pages++;
 		}
 	} while (ret > 0);
@@ -349,7 +351,7 @@ static int save_image(struct swap_map_ha
 	if (!error)
 		error = err2;
 	if (!error)
-		printk("\b\b\b\bdone\n");
+		printk(KERN_INFO "\b\b\b\bdone\n");
 	swsusp_show_speed(&start, &stop, nr_to_write, "Wrote");
 	return error;
 }
@@ -365,7 +367,7 @@ static int enough_swap(unsigned int nr_p
 {
 	unsigned int free_swap = count_swap_pages(root_swap, 1);
 
-	pr_debug("swsusp: free swap pages: %u\n", free_swap);
+	pr_debug(PREFIX "Free swap pages: %u\n", free_swap);
 	return free_swap > nr_pages + PAGES_FOR_IO;
 }
 
@@ -388,8 +390,8 @@ int swsusp_write(unsigned int flags)
 
 	error = swsusp_swap_check();
 	if (error) {
-		printk(KERN_ERR "swsusp: Cannot find swap device, try "
-				"swapon -a.\n");
+		printk(KERN_ERR PREFIX "Cannot find swap device, try "
+			KERN_ERR "swapon -a.\n");
 		return error;
 	}
 	memset(&snapshot, 0, sizeof(struct snapshot_handle));
@@ -402,7 +404,7 @@ int swsusp_write(unsigned int flags)
 	}
 	header = (struct swsusp_info *)data_of(snapshot);
 	if (!enough_swap(header->pages)) {
-		printk(KERN_ERR "swsusp: Not enough free swap\n");
+		printk(KERN_ERR PREFIX "Not enough free swap\n");
 		error = -ENOSPC;
 		goto out;
 	}
@@ -417,9 +419,9 @@ int swsusp_write(unsigned int flags)
 
 		if (!error) {
 			flush_swap_writer(&handle);
-			printk("S");
+			printk(KERN_INFO PREFIX "S");
 			error = mark_swapfiles(start, flags);
-			printk("|\n");
+			printk(KERN_INFO "|\n");
 		}
 	}
 	if (error)
@@ -507,7 +509,8 @@ static int load_image(struct swap_map_ha
 	int err2;
 	unsigned nr_pages;
 
-	printk("Loading image data pages (%u pages) ...     ", nr_to_read);
+	printk(KERN_INFO PREFIX "Loading image data pages (%u pages) ...     ",
+		nr_to_read);
 	m = nr_to_read / 100;
 	if (!m)
 		m = 1;
@@ -526,7 +529,7 @@ static int load_image(struct swap_map_ha
 		if (error)
 			break;
 		if (!(nr_pages % m))
-			printk("\b\b\b\b%3d%%", nr_pages / m);
+			printk(KERN_INFO "\b\b\b\b%3d%%", nr_pages / m);
 		nr_pages++;
 	}
 	err2 = wait_on_bio_chain(&bio);
@@ -534,7 +537,7 @@ static int load_image(struct swap_map_ha
 	if (!error)
 		error = err2;
 	if (!error) {
-		printk("\b\b\b\bdone\n");
+		printk(KERN_INFO "\b\b\b\bdone\n");
 		snapshot_write_finalize(snapshot);
 		if (!snapshot_image_loaded(snapshot))
 			error = -ENODATA;
@@ -558,7 +561,7 @@ int swsusp_read(unsigned int *flags_p)
 
 	*flags_p = swsusp_header->flags;
 	if (IS_ERR(resume_bdev)) {
-		pr_debug("swsusp: block device not initialised\n");
+		pr_debug(PREFIX "Block device not initialised\n");
 		return PTR_ERR(resume_bdev);
 	}
 
@@ -577,9 +580,9 @@ int swsusp_read(unsigned int *flags_p)
 	blkdev_put(resume_bdev);
 
 	if (!error)
-		pr_debug("swsusp: Reading resume file was successful\n");
+		pr_debug(PREFIX "Reading resume file was successful\n");
 	else
-		pr_debug("swsusp: Error %d resuming\n", error);
+		pr_debug(PREFIX "Error %d resuming\n", error);
 	return error;
 }
 
@@ -611,13 +614,13 @@ int swsusp_check(void)
 		if (error)
 			blkdev_put(resume_bdev);
 		else
-			pr_debug("swsusp: Signature found, resuming\n");
+			pr_debug(PREFIX "Signature found, resuming\n");
 	} else {
 		error = PTR_ERR(resume_bdev);
 	}
 
 	if (error)
-		pr_debug("swsusp: Error %d check for resume file\n", error);
+		pr_debug(PREFIX "Error %d check for resume file\n", error);
 
 	return error;
 }
@@ -629,7 +632,7 @@ int swsusp_check(void)
 void swsusp_close(void)
 {
 	if (IS_ERR(resume_bdev)) {
-		pr_debug("swsusp: block device not initialised\n");
+		pr_debug(PREFIX "Block device not initialised\n");
 		return;
 	}
 
Index: linux-2.6/kernel/power/swsusp.c
===================================================================
--- linux-2.6.orig/kernel/power/swsusp.c
+++ linux-2.6/kernel/power/swsusp.c
@@ -188,8 +188,8 @@ void swsusp_show_speed(struct timeval *s
 		centisecs = 1;	/* avoid div-by-zero */
 	k = nr_pages * (PAGE_SIZE / 1024);
 	kps = (k * 100) / centisecs;
-	printk("%s %d kbytes in %d.%02d seconds (%d.%02d MB/s)\n", msg, k,
-			centisecs / 100, centisecs % 100,
+	printk(KERN_INFO "%s %d kbytes in %d.%02d seconds (%d.%02d MB/s)\n",
+			msg, k, centisecs / 100, centisecs % 100,
 			kps / 1000, (kps % 1000) / 10);
 }
 
@@ -219,7 +219,7 @@ int swsusp_shrink_memory(void)
 	char *p = "-\\|/";
 	struct timeval start, stop;
 
-	printk("Shrinking memory...  ");
+	printk(KERN_INFO PREFIX "Shrinking memory...  ");
 	do_gettimeofday(&start);
 	do {
 		long size, highmem_size;
@@ -253,10 +253,10 @@ int swsusp_shrink_memory(void)
 			tmp = __shrink_memory(size - (image_size / PAGE_SIZE));
 			pages += tmp;
 		}
-		printk("\b%c", p[i++%4]);
+		printk(KERN_INFO "\b%c", p[i++%4]);
 	} while (tmp > 0);
 	do_gettimeofday(&stop);
-	printk("\bdone (%lu pages freed)\n", pages);
+	printk(KERN_INFO "\bdone (%lu pages freed)\n", pages);
 	swsusp_show_speed(&start, &stop, pages, "Freed");
 
 	return 0;

Re: [PATCH 5/6] Hibernation: Use common prefix in messages

[Pavel Machek]

Hi!

> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  kernel/power/disk.c     |   38 ++++++++++++++++++++------------------
>  kernel/power/snapshot.c |   24 +++++++++++++-----------
>  kernel/power/swap.c     |   43 +++++++++++++++++++++++--------------------
>  kernel/power/swsusp.c   |   10 +++++-----
>  4 files changed, 61 insertions(+), 54 deletions(-)
> 
> Index: linux-2.6/kernel/power/disk.c
> ===================================================================
> --- linux-2.6.orig/kernel/power/disk.c
> +++ linux-2.6/kernel/power/disk.c
> @@ -191,7 +191,7 @@ int create_image(int platform_mode)
>  	 */
>  	error = device_power_down(PMSG_FREEZE);
>  	if (error) {
> -		printk(KERN_ERR "Some devices failed to power down, "
> +		printk(KERN_ERR PREFIX "Some devices failed to power down, "
>  			KERN_ERR "aborting suspend\n");

This one is actually wrong. (But you did not introduce it). KERN_ERR
will be in the middle of line

> @@ -289,7 +290,7 @@ static int resume_target_kernel(void)
>  	local_irq_disable();
>  	error = device_power_down(PMSG_PRETHAW);
>  	if (error) {
> -		printk(KERN_ERR "Some devices failed to power down, "
> +		printk(KERN_ERR PREFIX "Some devices failed to power down, "
>  			KERN_ERR "aborting resume\n");
>  		goto Enable_irqs;
>  	}

Same here.

> @@ -482,9 +483,9 @@ int hibernate(void)
>  	if (error)
>  		goto Exit;
>  
> -	printk("Syncing filesystems ... ");
> +	printk(KERN_INFO PREFIX "Syncing filesystems ... ");
>  	sys_sync();
> -	printk("done.\n");
> +	printk(KERN_INFO "done.\n");

No. KERN_INFO would end up in the middle of the line.

> @@ -340,7 +342,7 @@ static int save_image(struct swap_map_ha
>  			if (error)
>  				break;
>  			if (!(nr_pages % m))
> -				printk("\b\b\b\b%3d%%", nr_pages / m);
> +				printk(KERN_INFO "\b\b\b\b%3d%%", nr_pages / m);
>  			nr_pages++;
>  		}
>  	} while (ret > 0);

This one is wrong.

> @@ -349,7 +351,7 @@ static int save_image(struct swap_map_ha
>  	if (!error)
>  		error = err2;
>  	if (!error)
> -		printk("\b\b\b\bdone\n");
> +		printk(KERN_INFO "\b\b\b\bdone\n");
>  	swsusp_show_speed(&start, &stop, nr_to_write, "Wrote");
>  	return error;
>  }

As is this. We do some pretty printing with backspaces, and <9> will
break it.

> @@ -388,8 +390,8 @@ int swsusp_write(unsigned int flags)
>  
>  	error = swsusp_swap_check();
>  	if (error) {
> -		printk(KERN_ERR "swsusp: Cannot find swap device, try "
> -				"swapon -a.\n");
> +		printk(KERN_ERR PREFIX "Cannot find swap device, try "
> +			KERN_ERR "swapon -a.\n");
>  		return error;

Same here.

> @@ -417,9 +419,9 @@ int swsusp_write(unsigned int flags)
>  
>  		if (!error) {
>  			flush_swap_writer(&handle);
> -			printk("S");
> +			printk(KERN_INFO PREFIX "S");
>  			error = mark_swapfiles(start, flags);
> -			printk("|\n");
> +			printk(KERN_INFO "|\n");
>  		}
>  	}
>  	if (error)

And here.

> @@ -526,7 +529,7 @@ static int load_image(struct swap_map_ha
>  		if (error)
>  			break;
>  		if (!(nr_pages % m))
> -			printk("\b\b\b\b%3d%%", nr_pages / m);
> +			printk(KERN_INFO "\b\b\b\b%3d%%", nr_pages / m);
>  		nr_pages++;
>  	}
>  	err2 = wait_on_bio_chain(&bio);

And here.

> @@ -534,7 +537,7 @@ static int load_image(struct swap_map_ha
>  	if (!error)
>  		error = err2;
>  	if (!error) {
> -		printk("\b\b\b\bdone\n");
> +		printk(KERN_INFO "\b\b\b\bdone\n");
>  		snapshot_write_finalize(snapshot);
>  		if (!snapshot_image_loaded(snapshot))
>  			error = -ENODATA;

...and here.

> @@ -253,10 +253,10 @@ int swsusp_shrink_memory(void)
>  			tmp = __shrink_memory(size - (image_size / PAGE_SIZE));
>  			pages += tmp;
>  		}
> -		printk("\b%c", p[i++%4]);
> +		printk(KERN_INFO "\b%c", p[i++%4]);
>  	} while (tmp > 0);
>  	do_gettimeofday(&stop);
> -	printk("\bdone (%lu pages freed)\n", pages);
> +	printk(KERN_INFO "\bdone (%lu pages freed)\n", pages);
>  	swsusp_show_speed(&start, &stop, pages, "Freed");

And here.

All in all, I do not think I like the "PREFIX" idea. 

printk(PREFIX "foo");

is longer than

printk("pm: foo");

... and the extra indirection will make greping slightly harder. (I
bet I would do 

grep "pm: foo" *.c

if I saw such message).

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 6/6] Hibernation: Move extern definition to header file

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Move the extern definition of resume_file[] from kernel/power/swap.c
to kernel/power/power.h .

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 kernel/power/power.h |    2 ++
 kernel/power/swap.c  |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/power/power.h
===================================================================
--- linux-2.6.orig/kernel/power/power.h
+++ linux-2.6/kernel/power/power.h
@@ -207,3 +207,5 @@ enum {
 #define TEST_MAX	(__TEST_AFTER_LAST - 1)
 
 extern int pm_test_level;
+
+extern char resume_file[];
Index: linux-2.6/kernel/power/swap.c
===================================================================
--- linux-2.6.orig/kernel/power/swap.c
+++ linux-2.6/kernel/power/swap.c
@@ -28,8 +28,6 @@
 
 #include "power.h"
 
-extern char resume_file[];
-
 #define SWSUSP_SIG	"S1SUSPEND"
 
 struct swsusp_header {

Re: [PATCH 6/6] Hibernation: Move extern definition to header file

[Pavel Machek]

Hi!

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Move the extern definition of resume_file[] from kernel/power/swap.c
> to kernel/power/power.h .
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

can we just get rid of extern? It does not seem to be used. (at least
in very quick grep over 2.6.24-rc3):

pavel@amd:/data/l/linux/kernel/power$ grep resume_file *.c
disk.c:char resume_file[256] = CONFIG_PM_STD_PARTITION;
disk.c:         if (!strlen(resume_file)) {
disk.c:         swsusp_resume_device = name_to_dev_t(resume_file);
disk.c:         pr_debug("swsusp: Resume From Partition %s\n",
resume_file);
disk.c: strncpy( resume_file, str, 255 );
swap.c:extern char resume_file[];
pavel@amd:/data/l/linux/kernel/power$

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html