From f581b63aa1049ac030d6eb6c24e1be1ce2072ae7 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 19 Jan 2012 23:22:38 +0100 Subject: [PATCH 1/5] PM / Documentation: Fix spelling mistake in basic-pm-debugging.txt Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- Documentation/power/basic-pm-debugging.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/power/basic-pm-debugging.txt b/Documentation/power/basic-pm-debugging.txt index 40a4c65f380a..262acf56fa79 100644 --- a/Documentation/power/basic-pm-debugging.txt +++ b/Documentation/power/basic-pm-debugging.txt @@ -15,7 +15,7 @@ test at least a couple of times in a row for confidence. [This is necessary, because some problems only show up on a second attempt at suspending and resuming the system.] Moreover, hibernating in the "reboot" and "shutdown" modes causes the PM core to skip some platform-related callbacks which on ACPI -systems might be necessary to make hibernation work. Thus, if you machine fails +systems might be necessary to make hibernation work. Thus, if your machine fails to hibernate or resume in the "reboot" mode, you should try the "platform" mode: # echo platform > /sys/power/disk From 5eb6f9ad96967be4e0da55521a253e11b534bd3f Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 19 Jan 2012 23:22:49 +0100 Subject: [PATCH 2/5] PM / Documentation: Fix minor issue in freezing_of_tasks.txt In a paragraph, "kernel thread" is mistakenly written as "kernel". Fix this by adding thread after word "kernel". Changes are shown in multiple lines, as they are realigned to 80 col width. Signed-off-by: Viresh Kumar Acked-by: Pavel Machek Signed-off-by: Rafael J. Wysocki --- Documentation/power/freezing-of-tasks.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/power/freezing-of-tasks.txt b/Documentation/power/freezing-of-tasks.txt index 6ccb68f68da6..ebd7490ef1df 100644 --- a/Documentation/power/freezing-of-tasks.txt +++ b/Documentation/power/freezing-of-tasks.txt @@ -120,10 +120,10 @@ So in practice, the 'at all' may become a 'why freeze kernel threads?' and freezing user threads I don't find really objectionable." Still, there are kernel threads that may want to be freezable. For example, if -a kernel that belongs to a device driver accesses the device directly, it in -principle needs to know when the device is suspended, so that it doesn't try to -access it at that time. However, if the kernel thread is freezable, it will be -frozen before the driver's .suspend() callback is executed and it will be +a kernel thread that belongs to a device driver accesses the device directly, it +in principle needs to know when the device is suspended, so that it doesn't try +to access it at that time. However, if the kernel thread is freezable, it will +be frozen before the driver's .suspend() callback is executed and it will be thawed after the driver's .resume() callback has run, so it won't be accessing the device while it's suspended. From 160cb5a97daef0cb894685d84c9d4700bb7cccb4 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 19 Jan 2012 23:23:10 +0100 Subject: [PATCH 3/5] PM / Hibernate: Correct additional pages number calculation The struct bm_block is allocated by chain_alloc(), so it'd better counting it in LINKED_PAGE_DATA_SIZE. Signed-off-by: Namhyung Kim Signed-off-by: Rafael J. Wysocki --- kernel/power/snapshot.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 1cf88900ec4f..6a768e537001 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -812,7 +812,8 @@ unsigned int snapshot_additional_pages(struct zone *zone) unsigned int res; res = DIV_ROUND_UP(zone->spanned_pages, BM_BITS_PER_BLOCK); - res += DIV_ROUND_UP(res * sizeof(struct bm_block), PAGE_SIZE); + res += DIV_ROUND_UP(res * sizeof(struct bm_block), + LINKED_PAGE_DATA_SIZE); return 2 * res; } From 72081624d5ad3cf56deb6e727b78c4e7a55e4eec Mon Sep 17 00:00:00 2001 From: "Srivatsa S. Bhat" Date: Thu, 19 Jan 2012 23:25:33 +0100 Subject: [PATCH 4/5] PM / Hibernate: Rewrite unlock_system_sleep() to fix s2disk regression Commit 33e638b, "PM / Sleep: Use the freezer_count() functions in [un]lock_system_sleep() APIs" introduced an undesirable change in the behaviour of unlock_system_sleep() since freezer_count() internally calls try_to_freeze() - which we don't need in unlock_system_sleep(). And commit bcda53f, "PM / Sleep: Replace mutex_[un]lock(&pm_mutex) with [un]lock_system_sleep()" made these APIs wide-spread. This caused a regression in suspend-to-disk where snapshot_read() and snapshot_write() were getting frozen due to the try_to_freeze embedded in unlock_system_sleep(), since these functions were invoked when the freezing condition was still in effect. Fix this by rewriting unlock_system_sleep() by open-coding freezer_count() and dropping the try_to_freeze() part. Not only will this fix the regression but this will also ensure that the API only does what it is intended to do, and nothing more, under the hood. While at it, make the code more correct and robust by ensuring that the PF_FREEZER_SKIP flag gets cleared with pm_mutex held, to avoid a race with the freezer. Also, to be on the safer side, open-code freezer_do_not_count() as well (inside lock_system_sleep()), to ensure that any unrelated modification to freezer[_do_not]_count() does not break things again! Reported-and-tested-by: Rafael J. Wysocki Signed-off-by: Srivatsa S. Bhat Acked-by: Tejun Heo Signed-off-by: Rafael J. Wysocki --- include/linux/suspend.h | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 95040cc33107..91784a4f8608 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -357,14 +357,29 @@ extern bool pm_save_wakeup_count(unsigned int count); static inline void lock_system_sleep(void) { - freezer_do_not_count(); + current->flags |= PF_FREEZER_SKIP; mutex_lock(&pm_mutex); } static inline void unlock_system_sleep(void) { + /* + * Don't use freezer_count() because we don't want the call to + * try_to_freeze() here. + * + * Reason: + * Fundamentally, we just don't need it, because freezing condition + * doesn't come into effect until we release the pm_mutex lock, + * since the freezer always works with pm_mutex held. + * + * More importantly, in the case of hibernation, + * unlock_system_sleep() gets called in snapshot_read() and + * snapshot_write() when the freezing condition is still in effect. + * Which means, if we use try_to_freeze() here, it would make them + * enter the refrigerator, thus causing hibernation to lockup. + */ + current->flags &= ~PF_FREEZER_SKIP; mutex_unlock(&pm_mutex); - freezer_count(); } #else /* !CONFIG_PM_SLEEP */ From e4c89a508f4385a0cd8681c2749a2cd2fa476e40 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 23 Jan 2012 21:59:08 +0100 Subject: [PATCH 5/5] PM / Sleep: Fix read_unlock_usermodehelper() call. Commit b298d289 "PM / Sleep: Fix freezer failures due to racy usermodehelper_is_disabled()" added read_unlock_usermodehelper() but read_unlock_usermodehelper() is called without read_lock_usermodehelper() when kmalloc() failed. Signed-off-by: Tetsuo Handa Acked-by: Srivatsa S. Bhat Signed-off-by: Rafael J. Wysocki --- drivers/base/firmware_class.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 26ab358dac62..6c9387d646ec 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -525,8 +525,7 @@ static int _request_firmware(const struct firmware **firmware_p, if (!firmware) { dev_err(device, "%s: kmalloc(struct firmware) failed\n", __func__); - retval = -ENOMEM; - goto out; + return -ENOMEM; } if (fw_get_builtin_firmware(firmware, name)) {