Message-ID: <20111117083042.11419.19871.stgit@srivatsabhat.in.ibm.com>
Date:	Thu, 17 Nov 2011 14:00:50 +0530
From:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To:	rjw@...k.pl
Cc:	pavel@....cz, lenb@...nel.org, ak@...ux.intel.com, tj@...nel.org,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-mm@...ck.org
Subject: [PATCH v3] PM/Memory-hotplug: Avoid task freezing failures

The lock_system_sleep() function is used in the memory hotplug code at
several places in order to implement mutual exclusion with hibernation.
However, this function tries to acquire the 'pm_mutex' lock using
mutex_lock() and hence blocks in TASK_UNINTERRUPTIBLE state if it doesn't
get the lock. This would lead to task freezing failures and hence
hibernation failure as a consequence, even though the hibernation call path
successfully acquired the lock.

This patch fixes this issue by modifying lock_system_sleep() to use
mutex_trylock() in a loop until the lock is acquired, instead of using
mutex_lock(), in order to avoid going to uninterruptible sleep.
Also, we use msleep() to avoid busy looping and breaking expectations
that we go to sleep when we fail to acquire the lock.
And we also call try_to_freeze() in order to cooperate with the freezer,
without which we would end up in almost the same situation as mutex_lock(),
due to uninterruptible sleep caused by msleep().

v3: Tejun suggested avoiding busy-looping by adding an msleep() since
    it is not guaranteed that we will get frozen immediately.

v2: Tejun pointed problems with using mutex_lock_interruptible() in a
    while loop, when signals not related to freezing are involved.
    So, replaced it with mutex_trylock().

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@...ux.vnet.ibm.com>
---

 include/linux/suspend.h |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 57a6924..0af3048 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -5,6 +5,8 @@
 #include <linux/notifier.h>
 #include <linux/init.h>
 #include <linux/pm.h>
+#include <linux/freezer.h>
+#include <linux/delay.h>
 #include <linux/mm.h>
 #include <asm/errno.h>
 
@@ -380,7 +382,40 @@ static inline void unlock_system_sleep(void) {}
 
 static inline void lock_system_sleep(void)
 {
-	mutex_lock(&pm_mutex);
+	/*
+	 * "To sleep, or not to sleep, that is the question!"
+	 *
+	 * We should not use mutex_lock() here because, in case we fail to
+	 * acquire the lock, it would put us to sleep in TASK_UNINTERRUPTIBLE
+	 * state, which would lead to task freezing failures. As a
+	 * consequence, hibernation would fail (even though it had acquired
+	 * the 'pm_mutex' lock).
+	 * Using mutex_lock_interruptible() in a loop is not a good idea,
+	 * because we could end up treating non-freezing signals badly.
+	 * So we use mutex_trylock() in a loop instead.
+	 *
+	 * Also, we add try_to_freeze() to the loop, to co-operate with the
+	 * freezer, to avoid task freezing failures due to busy-looping.
+	 *
+	 * But then, since it is not guaranteed that we will get frozen
+	 * rightaway, we could keep spinning for some time, breaking the
+	 * expectation that we go to sleep when we fail to acquire the lock.
+	 * So we add an msleep() to the loop, to dampen the spin (but we are
+	 * careful enough not to sleep for too long at a stretch, lest the
+	 * freezer whine and give up again!).
+	 *
+	 * Now that we no longer busy-loop, try_to_freeze() becomes all the
+	 * more important, due to a subtle reason: if we don't cooperate with
+	 * the freezer at this point, we could end up in a situation very
+	 * similar to mutex_lock() due to the usage of msleep() (which sleeps
+	 * uninterruptibly).
+	 *
+	 * Phew! What a delicate balance!
+	 */
+	while (!mutex_trylock(&pm_mutex)) {
+		try_to_freeze();
+		msleep(10);
+	}
 }
 
 static inline void unlock_system_sleep(void)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists