Skip to contentSkip to conversation

Suspend and hibernation code cleanups

PATCH In review Rafael J. Wysocki 2 participants 27 messages Nov 26, 2007 – Nov 27, 2007

Subsystems (2)
Suspend to RAM Hibernation
lore
Share & tools
lore

Rafael J. Wysocki

author
#0 lore raw

cover Suspend and hibernation code cleanups

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

Signature · 1 line
"Premature optimization is the root of all evil." - Donald Knuth
author
#1 lore raw

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

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 <[email protected]>
---
 kernel/power/main.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/power/main.c +2 -3
Index: 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)
#2 lore raw

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

Quoted 7 lines · 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 <[email protected]>

ACK.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw

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

In reply to Rafael J. Wysocki
Quoted 7 lines · 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 <[email protected]>

ACK.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 kernel/power/main.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
kernel/power/main.c +2 -3
Index: 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)
author
#5 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>
---
 kernel/power/disk.c   |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/power/power.h  |    1 -
 kernel/power/swsusp.c |   35 -----------------------------------
 3 files changed, 46 insertions(+), 37 deletions(-)
kernel/power/disk.c +46 -1
Index: 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);
kernel/power/swsusp.c +0 -35
Index: 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;
-}
kernel/power/power.h +0 -1
Index: 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);
#6 lore raw

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

Hi!

Quoted 9 lines

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

Move looks ok to me.

Quoted 8 lines

/**
+ * 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.
+ */

kernel/power/disk.c +1 -0
+

kernel/power/disk.c int hibernation_snapshot(int platform_mo · line 294

Quoted 10 lines
kernel/power/disk.c +10 -0
+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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#7 lore raw

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

Quoted 27 lines · Pavel Machek

Hi!

Quoted 9 lines

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

Move looks ok to me.

Quoted 12 lines

/**
+ * 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.
+ */

kernel/power/disk.c +5 -0
+
+static int resume_target_kernel(void)
+{
+	int error;
+
Quoted 8 lines

kernel/power/disk.c int hibernation_snapshot(int platform_mo · line 294

Quoted 6 lines
kernel/power/disk.c +6 -0
+	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.

Quoted 5 lines

. 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

#8 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 9 lines

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

Move looks ok to me.

Quoted 8 lines

/**
+ * 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.
+ */

kernel/power/disk.c +1 -0
+

kernel/power/disk.c int hibernation_snapshot(int platform_mo · line 294

Quoted 10 lines
kernel/power/disk.c +10 -0
+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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#9 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>
---
 kernel/power/disk.c   |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/power/power.h  |    1 -
 kernel/power/swsusp.c |   35 -----------------------------------
 3 files changed, 46 insertions(+), 37 deletions(-)
kernel/power/disk.c +46 -1
Index: 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);
kernel/power/swsusp.c +0 -35
Index: 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;
-}
kernel/power/power.h +0 -1
Index: 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);
author
#10 lore raw

PATCH 3/6 Suspend: Use common prefix in messages

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/main.c  |   19 ++++++++++---------
 kernel/power/power.h |    2 ++
 2 files changed, 12 insertions(+), 9 deletions(-)
kernel/power/main.c +10 -9
Index: 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);
kernel/power/power.h +2 -0
Index: 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;
#11 lore raw

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

Hi!

Quoted 5 lines

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>

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

kernel/power/main.c static int enter_state(suspend_state_t s · line 349

Quoted 9 lines
kernel/power/main.c +2 -2
@@ -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#12 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 5 lines

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>

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

kernel/power/main.c static int enter_state(suspend_state_t s · line 349

Quoted 9 lines
kernel/power/main.c +2 -2
@@ -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#13 lore raw

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

Quoted 9 lines · Pavel Machek

Hi!

Quoted 5 lines

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>

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: ".

Quoted 4 lines
Quoted 4 lines
kernel/power/main.c +0 -0
@@ -344,11 +344,12 @@ static int enter_state(suspend_state_t s
 	if (!mutex_trylock(&pm_mutex))
 		return -EBUSY;
Quoted 8 lines

kernel/power/main.c static int enter_state(suspend_state_t s · line 349

Quoted 5 lines
kernel/power/main.c +2 -2
-	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

author
#14 lore raw

PATCH 3/6 Suspend: Use common prefix in messages

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/main.c  |   19 ++++++++++---------
 kernel/power/power.h |    2 ++
 2 files changed, 12 insertions(+), 9 deletions(-)
kernel/power/main.c +10 -9
Index: 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);
kernel/power/power.h +2 -0
Index: 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;
author
#15 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>
---
 kernel/power/main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/power/main.c +2 -2
Index: 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)
#16 lore raw

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

Quoted 6 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

ACK.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#17 lore raw

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

In reply to Rafael J. Wysocki
Quoted 6 lines · Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>

ACK.

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#18 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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 <[email protected]>
---
 kernel/power/main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/power/main.c +2 -2
Index: 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)
author
#19 lore raw

PATCH 5/6 Hibernation: Use common prefix in messages

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 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(-)
kernel/power/disk.c +20 -18
Index: 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;
kernel/power/snapshot.c +13 -11
Index: 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;
kernel/power/swap.c +23 -20

146 lines not on this page · raw message

kernel/power/swsusp.c +5 -5
Index: 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;
#20 lore raw

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

Hi!

Quoted 13 lines
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 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
===================================================================

kernel/power/disk.c int create_image(int platform_mode) · line 195

Quoted 6 lines
kernel/power/disk.c +1 -1
@@ -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

kernel/power/disk.c static int resume_target_kernel(void) · line 296

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int hibernate(void) · line 488

Quoted 9 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c static int save_image(struct swap_map_ha · line 348

Quoted 9 lines
kernel/power/disk.c +1 -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);

This one is wrong.

kernel/power/disk.c static int save_image(struct swap_map_ha · line 357

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int swsusp_write(unsigned int flags) · line 395

Quoted 9 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c int swsusp_write(unsigned int flags) · line 427

Quoted 12 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c static int load_image(struct swap_map_ha · line 535

Quoted 9 lines
kernel/power/disk.c +1 -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);

And here.

kernel/power/disk.c static int load_image(struct swap_map_ha · line 543

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int swsusp_shrink_memory(void) · line 260

Quoted 11 lines
kernel/power/disk.c +2 -2
@@ -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#21 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 13 lines
Signed-off-by Rafael J. Wysocki <[email protected]>
---
 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
===================================================================

kernel/power/disk.c int create_image(int platform_mode) · line 195

Quoted 6 lines
kernel/power/disk.c +1 -1
@@ -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

kernel/power/disk.c static int resume_target_kernel(void) · line 296

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int hibernate(void) · line 488

Quoted 9 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c static int save_image(struct swap_map_ha · line 348

Quoted 9 lines
kernel/power/disk.c +1 -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);

This one is wrong.

kernel/power/disk.c static int save_image(struct swap_map_ha · line 357

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int swsusp_write(unsigned int flags) · line 395

Quoted 9 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c int swsusp_write(unsigned int flags) · line 427

Quoted 12 lines
kernel/power/disk.c +2 -2
@@ -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.

kernel/power/disk.c static int load_image(struct swap_map_ha · line 535

Quoted 9 lines
kernel/power/disk.c +1 -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);

And here.

kernel/power/disk.c static int load_image(struct swap_map_ha · line 543

Quoted 9 lines
kernel/power/disk.c +1 -1
@@ -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.

kernel/power/disk.c int swsusp_shrink_memory(void) · line 260

Quoted 11 lines
kernel/power/disk.c +2 -2
@@ -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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#22 lore raw

PATCH 5/6 Hibernation: Use common prefix in messages

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 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(-)
kernel/power/disk.c +20 -18
Index: 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;
kernel/power/snapshot.c +13 -11
Index: 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;
kernel/power/swap.c +23 -20

146 lines not on this page · raw message

kernel/power/swsusp.c +5 -5
Index: 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;
author
#23 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/power.h |    2 ++
 kernel/power/swap.c  |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
kernel/power/power.h +2 -0
Index: 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[];
kernel/power/swap.c +0 -2
Index: 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 {
#24 lore raw

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

Hi!

Quoted 6 lines

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#25 lore raw

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

In reply to Rafael J. Wysocki

Hi!

Quoted 6 lines

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#26 lore raw

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

In reply to Rafael J. Wysocki

From: Rafael J. Wysocki <[email protected]>

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

Signed-off-by Rafael J. Wysocki <[email protected]>
---
 kernel/power/power.h |    2 ++
 kernel/power/swap.c  |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
kernel/power/power.h +2 -0
Index: 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[];
kernel/power/swap.c +0 -2
Index: 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 {