From: Tejun Heo <tj@kernel.org>
To: rjw@sisk.pl, menage@google.com, linux-kernel@vger.kernel.org
Cc: arnd@arndb.de, oleg@redhat.com, Tejun Heo <tj@kernel.org>
Subject: [PATCH 10/16] freezer: fix set_freezable[_with_signal]() race
Date: Fri, 19 Aug 2011 16:16:16 +0200 [thread overview]
Message-ID: <1313763382-12341-11-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1313763382-12341-1-git-send-email-tj@kernel.org>
A kthread doing set_freezable*() may race with on-going PM freeze and
the freezer might think all tasks are frozen while the new freezable
kthread is marrily proceeding to execute code paths which aren't
supposed to be executing during PM freeze.
This can be fixed by modifying and testing the related PF flags inside
freezer_lock and removing mostly unnecessary early tests from the
callers of freeze/thaw_task().
* Remove all unnecessary tests from kerne/power/process.c and add
PF_NOFREEZE test to freeze_task(), which is meaningful as it avoids
sending unsolicted wakeups to nofreeze tasks.
* Reimplement set_freezable[_with_signal]() using __set_freezable()
such that freezable PF flags are modified under freezer_lock and
try_to_freeze() is called afterwards. Combined with the above
change, this eliminates race condition against freezing.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/freezer.h | 9 +++++----
kernel/freezer.c | 28 +++++++++++++++++++++++++++-
kernel/power/process.c | 16 +---------------
3 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 80a455d..59a6e97 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -61,6 +61,7 @@ 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);
+extern bool __set_freezable(bool with_signal);
#ifdef CONFIG_CGROUP_FREEZER
extern int cgroup_freezing_or_frozen(struct task_struct *task);
@@ -118,18 +119,18 @@ static inline int freezer_should_skip(struct task_struct *p)
/*
* Tell the freezer that the current task should be frozen by it
*/
-static inline void set_freezable(void)
+static inline bool set_freezable(void)
{
- current->flags &= ~PF_NOFREEZE;
+ return __set_freezable(false);
}
/*
* Tell the freezer that the current task should be frozen by it and that it
* should send a fake signal to the task to freeze it.
*/
-static inline void set_freezable_with_signal(void)
+static inline bool set_freezable_with_signal(void)
{
- current->flags &= ~(PF_NOFREEZE | PF_FREEZER_NOSIG);
+ return __set_freezable(true);
}
/*
diff --git a/kernel/freezer.c b/kernel/freezer.c
index d6165cd..501f1b7 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -106,7 +106,8 @@ bool freeze_task(struct task_struct *p, bool sig_only)
spin_lock_irqsave(&freezer_lock, flags);
- if (sig_only && !should_send_signal(p))
+ if ((p->flags & PF_NOFREEZE) ||
+ (sig_only && !should_send_signal(p)))
goto out_unlock;
if (frozen(p))
@@ -162,3 +163,28 @@ void __thaw_task(struct task_struct *p)
wake_up_process(p);
spin_unlock_irqrestore(&freezer_lock, flags);
}
+
+/**
+ * __set_freezable - make %current freezable
+ * @with_signal: do we want %TIF_SIGPENDING for notification too?
+ *
+ * Mark %current freezable and enter refrigerator if necessary.
+ */
+bool __set_freezable(bool with_signal)
+{
+ might_sleep();
+
+ /*
+ * Modify flags while holding freezer_lock. This ensures the
+ * freezer notices that we aren't frozen yet or the freezing
+ * condition is visible to try_to_freeze() below.
+ */
+ spin_lock_irq(&freezer_lock);
+ current->flags &= ~PF_NOFREEZE;
+ if (with_signal)
+ current->flags &= ~PF_FREEZER_NOSIG;
+ spin_unlock_irq(&freezer_lock);
+
+ return try_to_freeze();
+}
+EXPORT_SYMBOL(__set_freezable);
diff --git a/kernel/power/process.c b/kernel/power/process.c
index f075c2f..7618909 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -22,14 +22,6 @@
*/
#define TIMEOUT (20 * HZ)
-static inline int freezable(struct task_struct * p)
-{
- if ((p == current) ||
- (p->flags & PF_NOFREEZE))
- return 0;
- return 1;
-}
-
static int try_to_freeze_tasks(bool sig_only)
{
struct task_struct *g, *p;
@@ -52,10 +44,7 @@ static int try_to_freeze_tasks(bool sig_only)
todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (frozen(p) || !freezable(p))
- continue;
-
- if (!freeze_task(p, sig_only))
+ if (p == current || !freeze_task(p, sig_only))
continue;
/*
@@ -171,9 +160,6 @@ void thaw_processes(void)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (!freezable(p))
- continue;
-
if (cgroup_freezing_or_frozen(p))
continue;
--
1.7.6
next prev parent reply other threads:[~2011-08-19 14:16 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-19 14:16 [PATCHSET] freezer: fix various bugs and simplify implementation Tejun Heo
2011-08-19 14:16 ` [PATCH 01/16] freezer: fix current->state restoration race in refrigerator() Tejun Heo
2011-08-19 15:52 ` Oleg Nesterov
2011-08-19 16:11 ` Tejun Heo
2011-08-19 21:08 ` Rafael J. Wysocki
2011-08-20 8:13 ` Tejun Heo
2011-08-19 14:16 ` [PATCH 02/16] freezer: don't unnecessarily set PF_NOFREEZE explicitly Tejun Heo
2011-08-19 16:43 ` Gustavo Padovan
2011-08-22 15:05 ` Samuel Ortiz
2011-08-19 14:16 ` [PATCH 03/16] freezer: unexport refrigerator() and update try_to_freeze() slightly Tejun Heo
2011-08-19 14:16 ` [PATCH 04/16] freezer: implement and use kthread_freezable_should_stop() Tejun Heo
2011-08-19 20:07 ` Henrique de Moraes Holschuh
2011-08-21 19:14 ` Oleg Nesterov
2011-08-22 9:53 ` Tejun Heo
2011-08-23 15:42 ` Oleg Nesterov
2011-08-19 14:16 ` [PATCH 05/16] freezer: rename thaw_process() to __thaw_task() and simplify the implementation Tejun Heo
2011-08-19 15:37 ` Paul Menage
2011-08-24 2:28 ` Matt Helsley
2011-08-19 14:16 ` [PATCH 06/16] freezer: make exiting tasks properly unfreezable Tejun Heo
2011-08-23 15:52 ` Oleg Nesterov
2011-08-23 19:44 ` Tejun Heo
2011-08-24 14:14 ` Oleg Nesterov
2011-08-25 15:59 ` Tejun Heo
2011-08-25 16:56 ` Oleg Nesterov
2011-08-25 21:01 ` Rafael J. Wysocki
2011-08-25 21:54 ` Tejun Heo
2011-08-26 21:09 ` Rafael J. Wysocki
2011-08-27 10:35 ` Tejun Heo
2011-08-27 10:51 ` Rafael J. Wysocki
2011-08-27 11:02 ` Tejun Heo
2011-08-27 12:22 ` Rafael J. Wysocki
2011-08-25 21:52 ` Tejun Heo
2011-08-24 22:34 ` Matt Helsley
2011-08-25 15:25 ` Oleg Nesterov
2011-08-25 16:11 ` Tejun Heo
2011-08-19 14:16 ` [PATCH 07/16] freezer: don't distinguish nosig tasks on thaw Tejun Heo
2011-08-19 21:14 ` Rafael J. Wysocki
2011-08-20 8:10 ` Tejun Heo
2011-08-20 8:39 ` Rafael J. Wysocki
2011-08-19 14:16 ` [PATCH 08/16] freezer: use dedicated lock instead of task_lock() + memory barrier Tejun Heo
2011-08-28 17:51 ` Oleg Nesterov
2011-08-28 18:21 ` Oleg Nesterov
2011-08-29 7:20 ` Tejun Heo
2011-08-19 14:16 ` [PATCH 09/16] freezer: make freezing indicate freeze condition in effect Tejun Heo
2011-08-28 17:56 ` Oleg Nesterov
2011-08-29 7:31 ` Tejun Heo
2011-08-29 17:44 ` Oleg Nesterov
2011-08-19 14:16 ` Tejun Heo [this message]
2011-08-28 18:01 ` [PATCH 10/16] freezer: fix set_freezable[_with_signal]() race Oleg Nesterov
2011-08-29 7:38 ` Tejun Heo
2011-08-19 14:16 ` [PATCH 11/16] freezer: kill PF_FREEZING Tejun Heo
2011-08-19 14:16 ` [PATCH 12/16] freezer: clean up freeze_processes() failure path Tejun Heo
2011-08-28 18:09 ` Oleg Nesterov
2011-08-29 7:28 ` Tejun Heo
2011-08-29 7:40 ` Rafael J. Wysocki
2011-08-19 14:16 ` [PATCH 13/16] cgroup_freezer: prepare for removal of TIF_FREEZE Tejun Heo
2011-08-19 15:40 ` Paul Menage
2011-08-28 17:39 ` Oleg Nesterov
2011-08-29 6:30 ` Tejun Heo
2011-08-19 14:16 ` [PATCH 14/16] freezer: make freezing() test freeze conditions in effect instead " Tejun Heo
2011-08-19 15:43 ` Paul Menage
2011-08-29 15:49 ` Oleg Nesterov
2011-08-29 15:56 ` Oleg Nesterov
2011-08-29 16:30 ` Oleg Nesterov
2011-08-29 16:17 ` Oleg Nesterov
2011-08-19 14:16 ` [PATCH 15/16] freezer: remove now unused TIF_FREEZE Tejun Heo
2011-08-19 14:16 ` [PATCH 16/16] freezer: remove should_send_signal() and update frozen() Tejun Heo
2011-08-19 14:23 ` [PATCHSET] freezer: fix various bugs and simplify implementation Tejun Heo
2011-08-19 15:34 ` Paul Menage
2011-08-19 16:25 ` Tejun Heo
2011-08-24 1:10 ` Matt Helsley
2011-08-19 21:00 ` Rafael J. Wysocki
2011-08-20 8:14 ` Tejun Heo
2011-09-05 8:52 ` [BUG] CPU hotplug, freezer: Freezing of tasks failed after 20.00 seconds Srivatsa S. Bhat
2011-09-05 14:15 ` Tejun Heo
2011-09-06 5:08 ` Tejun Heo
2011-09-06 6:01 ` Rafael J. Wysocki
2011-10-02 19:13 ` Srivatsa S. Bhat
2011-10-02 19:33 ` Rafael J. Wysocki
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=1313763382-12341-11-git-send-email-tj@kernel.org \
--to=tj@kernel.org \
--cc=arnd@arndb.de \
--cc=linux-kernel@vger.kernel.org \
--cc=menage@google.com \
--cc=oleg@redhat.com \
--cc=rjw@sisk.pl \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox