All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: rjw@sisk.pl, paul@paulmenage.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Cc: arnd@arndb.de, oleg@redhat.com, matthltc@us.ibm.com,
	"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Subject: [PATCH UPDATED 12/17] freezer: clean up freeze_processes() failure path
Date: Thu, 3 Nov 2011 15:25:38 -0700	[thread overview]
Message-ID: <20111103222538.GQ4417@google.com> (raw)
In-Reply-To: <1320087928-32307-13-git-send-email-tj@kernel.org>

>From 2981f8990826d8ab411c7d7bba59bfb7540a015c Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Thu, 3 Nov 2011 15:19:27 -0700

freeze_processes() failure path is rather messy.  Freezing is canceled
for workqueues and tasks which aren't frozen yet but frozen tasks are
left alone and should be thawed by the caller and of course some
callers (xen and kexec) didn't do it.

This patch updates __thaw_task() to handle cancelation correctly and
makes freeze_processes() and freeze_kernel_threads() call
thaw_processes() on failure instead so that the system is fully thawed
on failure.  Unnecessary [suspend_]thaw_processes() calls are removed
from kernel/power/hibernate.c, suspend.c and user.c.

While at it, restructure error checking if clause in suspend_prepare()
to be less weird.

-v2: Srivatsa spotted missing removal of suspend_thaw_processes() in
     suspend_prepare() and error in commit message.  Updated.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
Alright, updated suspend.c and the commit message and added Srivatsa's
Acked-by.  This update doesn't cause conflicts with other patches and
git branch is updated accordingly.

Thanks.

 include/linux/freezer.h  |    1 -
 kernel/freezer.c         |   25 +++++++++----------------
 kernel/power/hibernate.c |   15 ++-------------
 kernel/power/process.c   |   16 ++++++++--------
 kernel/power/suspend.c   |    8 +++-----
 kernel/power/user.c      |    4 +---
 6 files changed, 23 insertions(+), 46 deletions(-)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 253bcfe..e6657778 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -61,7 +61,6 @@ static inline bool try_to_freeze(void)
 }
 
 extern bool freeze_task(struct task_struct *p, bool sig_only);
-extern void cancel_freezing(struct task_struct *p);
 
 #ifdef CONFIG_CGROUP_FREEZER
 extern int cgroup_freezing_or_frozen(struct task_struct *task);
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 3be1e36..7a9f706 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -129,21 +129,6 @@ out_unlock:
 	return ret;
 }
 
-void cancel_freezing(struct task_struct *p)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&freezer_lock, flags);
-	if (freezing(p)) {
-		pr_debug("  clean up: %s\n", p->comm);
-		clear_freeze_flag(p);
-		spin_lock(&p->sighand->siglock);
-		recalc_sigpending_and_wake(p);
-		spin_unlock(&p->sighand->siglock);
-	}
-	spin_unlock_irqrestore(&freezer_lock, flags);
-}
-
 void __thaw_task(struct task_struct *p)
 {
 	unsigned long flags;
@@ -153,10 +138,18 @@ void __thaw_task(struct task_struct *p)
 	 * be visible to @p as waking up implies wmb.  Waking up inside
 	 * freezer_lock also prevents wakeups from leaking outside
 	 * refrigerator.
+	 *
+	 * If !FROZEN, @p hasn't reached refrigerator, recalc sigpending to
+	 * avoid leaving dangling TIF_SIGPENDING behind.
 	 */
 	spin_lock_irqsave(&freezer_lock, flags);
 	clear_freeze_flag(p);
-	if (frozen(p))
+	if (frozen(p)) {
 		wake_up_process(p);
+	} else {
+		spin_lock(&p->sighand->siglock);
+		recalc_sigpending_and_wake(p);
+		spin_unlock(&p->sighand->siglock);
+	}
 	spin_unlock_irqrestore(&freezer_lock, flags);
 }
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 1c53f7f..def6406 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -593,17 +593,6 @@ static void power_down(void)
 	while(1);
 }
 
-static int prepare_processes(void)
-{
-	int error = 0;
-
-	if (freeze_processes()) {
-		error = -EBUSY;
-		thaw_processes();
-	}
-	return error;
-}
-
 /**
  * hibernate - Carry out system hibernation, including saving the image.
  */
@@ -636,7 +625,7 @@ int hibernate(void)
 	sys_sync();
 	printk("done.\n");
 
-	error = prepare_processes();
+	error = freeze_processes();
 	if (error)
 		goto Finish;
 
@@ -799,7 +788,7 @@ static int software_resume(void)
 		goto close_finish;
 
 	pr_debug("PM: Preparing processes for restore.\n");
-	error = prepare_processes();
+	error = freeze_processes();
 	if (error) {
 		swsusp_close(FMODE_READ);
 		goto Done;
diff --git a/kernel/power/process.c b/kernel/power/process.c
index e59676f..ce64383 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -91,11 +91,6 @@ static int try_to_freeze_tasks(bool sig_only)
 	elapsed_csecs = elapsed_csecs64;
 
 	if (todo) {
-		/* This does not unfreeze processes that are already frozen
-		 * (we have slightly ugly calling convention in that respect,
-		 * and caller must call thaw_processes() if something fails),
-		 * but it cleans up leftover PF_FREEZE requests.
-		 */
 		printk("\n");
 		printk(KERN_ERR "Freezing of tasks %s after %d.%02d seconds "
 		       "(%d tasks refusing to freeze, wq_busy=%d):\n",
@@ -103,14 +98,11 @@ static int try_to_freeze_tasks(bool sig_only)
 		       elapsed_csecs / 100, elapsed_csecs % 100,
 		       todo - wq_busy, wq_busy);
 
-		thaw_workqueues();
-
 		read_lock(&tasklist_lock);
 		do_each_thread(g, p) {
 			if (!wakeup && !freezer_should_skip(p) &&
 			    freezing(p) && !frozen(p))
 				sched_show_task(p);
-			cancel_freezing(p);
 		} while_each_thread(g, p);
 		read_unlock(&tasklist_lock);
 	} else {
@@ -123,6 +115,8 @@ static int try_to_freeze_tasks(bool sig_only)
 
 /**
  * freeze_processes - Signal user space processes to enter the refrigerator.
+ *
+ * On success, returns 0.  On failure, -errno and system is fully thawed.
  */
 int freeze_processes(void)
 {
@@ -137,11 +131,15 @@ int freeze_processes(void)
 	printk("\n");
 	BUG_ON(in_atomic());
 
+	if (error)
+		thaw_processes();
 	return error;
 }
 
 /**
  * freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
+ *
+ * On success, returns 0.  On failure, -errno and system is fully thawed.
  */
 int freeze_kernel_threads(void)
 {
@@ -155,6 +153,8 @@ int freeze_kernel_threads(void)
 	printk("\n");
 	BUG_ON(in_atomic());
 
+	if (error)
+		thaw_processes();
 	return error;
 }
 
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index fdd4263..633e9f9 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -104,13 +104,11 @@ static int suspend_prepare(void)
 		goto Finish;
 
 	error = suspend_freeze_processes();
-	if (error) {
-		suspend_stats.failed_freeze++;
-		dpm_save_failed_step(SUSPEND_FREEZE);
-	} else
+	if (!error)
 		return 0;
 
-	suspend_thaw_processes();
+	suspend_stats.failed_freeze++;
+	dpm_save_failed_step(SUSPEND_FREEZE);
 	usermodehelper_enable();
  Finish:
 	pm_notifier_call_chain(PM_POST_SUSPEND);
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 42ddbc6..afe7737 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -256,10 +256,8 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 			break;
 
 		error = freeze_processes();
-		if (error) {
-			thaw_processes();
+		if (error)
 			usermodehelper_enable();
-		}
 		if (!error)
 			data->frozen = 1;
 		break;
-- 
1.7.3.1


  parent reply	other threads:[~2011-11-03 22:25 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
2011-10-31 19:05 ` [PATCH 01/17] freezer: fix current->state restoration race in refrigerator() Tejun Heo
2011-10-31 19:05 ` [PATCH 02/17] freezer: don't unnecessarily set PF_NOFREEZE explicitly Tejun Heo
2011-10-31 19:05 ` [PATCH 03/17] freezer: unexport refrigerator() and update try_to_freeze() slightly Tejun Heo
2011-10-31 19:05 ` [PATCH 04/17] freezer: implement and use kthread_freezable_should_stop() Tejun Heo
2011-10-31 19:05 ` [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation Tejun Heo
2011-11-06 12:04   ` Srivatsa S. Bhat
2011-11-06 16:51     ` Tejun Heo
2011-11-06 17:10       ` Srivatsa S. Bhat
2011-10-31 19:05 ` [PATCH 06/17] freezer: remove racy clear_freeze_flag() and set PF_NOFREEZE on dead tasks Tejun Heo
2011-10-31 19:05 ` [PATCH 07/17] freezer: don't distinguish nosig tasks on thaw Tejun Heo
2011-10-31 19:05 ` [PATCH 08/17] freezer: use dedicated lock instead of task_lock() + memory barrier Tejun Heo
2011-10-31 19:05 ` [PATCH 09/17] freezer: make freezing indicate freeze condition in effect Tejun Heo
2011-10-31 19:05 ` [PATCH 10/17] freezer: test freezable conditions while holding freezer_lock Tejun Heo
2011-10-31 19:05 ` [PATCH 11/17] freezer: kill PF_FREEZING Tejun Heo
2011-10-31 19:05 ` [PATCH 12/17] freezer: clean up freeze_processes() failure path Tejun Heo
2011-11-03 19:09   ` Srivatsa S. Bhat
2011-11-03 22:25   ` Tejun Heo [this message]
2011-10-31 19:05 ` [PATCH 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE Tejun Heo
2011-10-31 22:33   ` [PATCH UPDATED " Tejun Heo
2011-10-31 19:05 ` [PATCH 14/17] freezer: make freezing() test freeze conditions in effect instead " Tejun Heo
2011-10-31 22:34   ` [PATCH UPDATED " Tejun Heo
2011-10-31 19:05 ` [PATCH 15/17] freezer: remove now unused TIF_FREEZE Tejun Heo
2011-10-31 19:05 ` [PATCH 16/17] freezer: remove should_send_signal() and update frozen() Tejun Heo
2011-10-31 19:05 ` [PATCH 17/17] freezer: fix set_freezable[_with_signal]() race Tejun Heo
2011-10-31 22:34 ` [PATCH 18/17] freezer: restructure __refrigerator() Tejun Heo
2011-10-31 22:35 ` [PATCH 19/17] freezer: use lock_task_sighand() in fake_signal_wake_up() Tejun Heo
2011-10-31 22:35 ` [PATCH UPDATED 20/17] freezer: remove unused @sig_only from freeze_task() Tejun Heo
2011-10-31 22:40 ` [PATCHSET UPDATED pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111103222538.GQ4417@google.com \
    --to=tj@kernel.org \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthltc@us.ibm.com \
    --cc=oleg@redhat.com \
    --cc=paul@paulmenage.org \
    --cc=rjw@sisk.pl \
    --cc=srivatsa.bhat@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.