* [PATCH 01/17] freezer: fix current->state restoration race in refrigerator()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 02/17] freezer: don't unnecessarily set PF_NOFREEZE explicitly Tejun Heo
` (19 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
refrigerator() saves current->state before entering frozen state and
restores it before returning using __set_current_state(); however,
this is racy, for example, please consider the following sequence.
set_current_state(TASK_INTERRUPTIBLE);
try_to_freeze();
if (kthread_should_stop())
break;
schedule();
If kthread_stop() races with ->state restoration, the restoration can
restore ->state to TASK_INTERRUPTIBLE after kthread_stop() sets it to
TASK_RUNNING but kthread_should_stop() may still see zero
->should_stop because there's no memory barrier between restoring
TASK_INTERRUPTIBLE and kthread_should_stop() test.
This isn't restricted to kthread_should_stop(). current->state is
often used in memory barrier based synchronization and silently
restoring it w/o mb breaks them.
Use set_current_state() instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/freezer.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 66a594e..19a20b2 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -58,7 +58,13 @@ void refrigerator(void)
current->flags &= ~PF_FREEZING;
pr_debug("%s left refrigerator\n", current->comm);
- __set_current_state(save);
+
+ /*
+ * Restore saved task state before returning. The mb'd version
+ * needs to be used; otherwise, it might silently break
+ * synchronization which depends on ordered task state change.
+ */
+ set_current_state(save);
}
EXPORT_SYMBOL(refrigerator);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/17] freezer: don't unnecessarily set PF_NOFREEZE explicitly
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 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 03/17] freezer: unexport refrigerator() and update try_to_freeze() slightly Tejun Heo
` (18 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, Marcel Holtmann, wwang
Some drivers set PF_NOFREEZE in their kthread functions which is
completely unnecessary and racy - some part of freezer code doesn't
consider cases where PF_NOFREEZE is set asynchronous to freezer
operations.
In general, there's no reason to allow setting PF_NOFREEZE explicitly.
Remove them and change the documentation to note that setting
PF_NOFREEZE directly isn't allowed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: "Gustavo F. Padovan" <padovan@profusion.mobi>
Acked-by: Samuel Ortiz <sameo@linux.intel.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: wwang <wei_wang@realsil.com.cn>
---
Documentation/power/freezing-of-tasks.txt | 2 +-
drivers/bluetooth/btmrvl_main.c | 2 --
drivers/mfd/twl4030-irq.c | 3 ---
drivers/mfd/twl6030-irq.c | 2 --
drivers/staging/rts_pstor/rtsx.c | 2 --
5 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/Documentation/power/freezing-of-tasks.txt b/Documentation/power/freezing-of-tasks.txt
index 38b5724..710c965 100644
--- a/Documentation/power/freezing-of-tasks.txt
+++ b/Documentation/power/freezing-of-tasks.txt
@@ -67,7 +67,7 @@ III. Which kernel threads are freezable?
Kernel threads are not freezable by default. However, a kernel thread may clear
PF_NOFREEZE for itself by calling set_freezable() (the resetting of PF_NOFREEZE
-directly is strongly discouraged). From this point it is regarded as freezable
+directly is not allowed). From this point it is regarded as freezable
and must call try_to_freeze() in a suitable place.
IV. Why do we do that?
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index 548d1d9..57312d4 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -473,8 +473,6 @@ static int btmrvl_service_main_thread(void *data)
init_waitqueue_entry(&wait, current);
- current->flags |= PF_NOFREEZE;
-
for (;;) {
add_wait_queue(&thread->wait_q, &wait);
diff --git a/drivers/mfd/twl4030-irq.c b/drivers/mfd/twl4030-irq.c
index 8a7ee31..6aae831 100644
--- a/drivers/mfd/twl4030-irq.c
+++ b/drivers/mfd/twl4030-irq.c
@@ -289,9 +289,6 @@ static int twl4030_irq_thread(void *data)
static unsigned i2c_errors;
static const unsigned max_i2c_errors = 100;
-
- current->flags |= PF_NOFREEZE;
-
while (!kthread_should_stop()) {
int ret;
int module_irq;
diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c
index eb3b5f8..6fd7795 100644
--- a/drivers/mfd/twl6030-irq.c
+++ b/drivers/mfd/twl6030-irq.c
@@ -96,8 +96,6 @@ static int twl6030_irq_thread(void *data)
static const unsigned max_i2c_errors = 100;
int ret;
- current->flags |= PF_NOFREEZE;
-
while (!kthread_should_stop()) {
int i;
union {
diff --git a/drivers/staging/rts_pstor/rtsx.c b/drivers/staging/rts_pstor/rtsx.c
index 480b0ed..8a7803c 100644
--- a/drivers/staging/rts_pstor/rtsx.c
+++ b/drivers/staging/rts_pstor/rtsx.c
@@ -466,8 +466,6 @@ static int rtsx_control_thread(void *__dev)
struct rtsx_chip *chip = dev->chip;
struct Scsi_Host *host = rtsx_to_host(dev);
- current->flags |= PF_NOFREEZE;
-
for (;;) {
if (wait_for_completion_interruptible(&dev->cmnd_ready))
break;
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/17] freezer: unexport refrigerator() and update try_to_freeze() slightly
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 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 04/17] freezer: implement and use kthread_freezable_should_stop() Tejun Heo
` (17 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, Samuel Ortiz, Chris Mason,
Theodore Ts'o, Steven Whitehouse, Andrew Morton, Jan Kara,
KONISHI Ryusuke, Christoph Hellwig
There is no reason to export two functions for entering the
refrigerator. Calling refrigerator() instead of try_to_freeze()
doesn't save anything noticeable or removes any race condition.
* Rename refrigerator() to __refrigerator() and make it return bool
indicating whether it scheduled out for freezing.
* Update try_to_freeze() to return bool and relay the return value of
__refrigerator() if freezing().
* Convert all refrigerator() users to try_to_freeze().
* Update documentation accordingly.
* While at it, add might_sleep() to try_to_freeze().
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Steven Whitehouse <swhiteho@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>
Cc: KONISHI Ryusuke <konishi.ryusuke@lab.ntt.co.jp>
Cc: Christoph Hellwig <hch@infradead.org>
---
Documentation/power/freezing-of-tasks.txt | 12 ++++++------
drivers/net/irda/stir4200.c | 2 +-
fs/btrfs/async-thread.c | 2 +-
fs/btrfs/disk-io.c | 8 ++------
fs/ext4/super.c | 3 +--
fs/gfs2/log.c | 4 ++--
fs/gfs2/quota.c | 4 ++--
fs/jbd/journal.c | 2 +-
fs/jbd2/journal.c | 2 +-
fs/jfs/jfs_logmgr.c | 2 +-
fs/jfs/jfs_txnmgr.c | 4 ++--
fs/nilfs2/segment.c | 2 +-
fs/xfs/xfs_buf.c | 2 +-
include/linux/freezer.h | 17 ++++++++---------
kernel/freezer.c | 10 +++++++---
15 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/Documentation/power/freezing-of-tasks.txt b/Documentation/power/freezing-of-tasks.txt
index 710c965..cbea897 100644
--- a/Documentation/power/freezing-of-tasks.txt
+++ b/Documentation/power/freezing-of-tasks.txt
@@ -21,7 +21,7 @@ freeze_processes() (defined in kernel/power/process.c) is called. It executes
try_to_freeze_tasks() that sets TIF_FREEZE for all of the freezable tasks and
either wakes them up, if they are kernel threads, or sends fake signals to them,
if they are user space processes. A task that has TIF_FREEZE set, should react
-to it by calling the function called refrigerator() (defined in
+to it by calling the function called __refrigerator() (defined in
kernel/power/process.c), which sets the task's PF_FROZEN flag, changes its state
to TASK_UNINTERRUPTIBLE and makes it loop until PF_FROZEN is cleared for it.
Then, we say that the task is 'frozen' and therefore the set of functions
@@ -29,10 +29,10 @@ handling this mechanism is referred to as 'the freezer' (these functions are
defined in kernel/power/process.c and include/linux/freezer.h). User space
processes are generally frozen before kernel threads.
-It is not recommended to call refrigerator() directly. Instead, it is
-recommended to use the try_to_freeze() function (defined in
-include/linux/freezer.h), that checks the task's TIF_FREEZE flag and makes the
-task enter refrigerator() if the flag is set.
+__refrigerator() must not be called directly. Instead, use the
+try_to_freeze() function (defined in include/linux/freezer.h), that checks
+the task's TIF_FREEZE flag and makes the task enter __refrigerator() if the
+flag is set.
For user space processes try_to_freeze() is called automatically from the
signal-handling code, but the freezable kernel threads need to call it
@@ -61,7 +61,7 @@ wait_event_freezable() and wait_event_freezable_timeout() macros.
After the system memory state has been restored from a hibernation image and
devices have been reinitialized, the function thaw_processes() is called in
order to clear the PF_FROZEN flag for each frozen task. Then, the tasks that
-have been frozen leave refrigerator() and continue running.
+have been frozen leave __refrigerator() and continue running.
III. Which kernel threads are freezable?
diff --git a/drivers/net/irda/stir4200.c b/drivers/net/irda/stir4200.c
index 41c96b3..e880c79 100644
--- a/drivers/net/irda/stir4200.c
+++ b/drivers/net/irda/stir4200.c
@@ -750,7 +750,7 @@ static int stir_transmit_thread(void *arg)
write_reg(stir, REG_CTRL1, CTRL1_TXPWD|CTRL1_RXPWD);
- refrigerator();
+ try_to_freeze();
if (change_speed(stir, stir->speed))
break;
diff --git a/fs/btrfs/async-thread.c b/fs/btrfs/async-thread.c
index 7ec1409..98ab240 100644
--- a/fs/btrfs/async-thread.c
+++ b/fs/btrfs/async-thread.c
@@ -340,7 +340,7 @@ again:
if (freezing(current)) {
worker->working = 0;
spin_unlock_irq(&worker->lock);
- refrigerator();
+ try_to_freeze();
} else {
spin_unlock_irq(&worker->lock);
if (!kthread_should_stop()) {
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 07b3ac6..64b1c07 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1508,9 +1508,7 @@ static int cleaner_kthread(void *arg)
btrfs_run_defrag_inodes(root->fs_info);
}
- if (freezing(current)) {
- refrigerator();
- } else {
+ if (!try_to_freeze()) {
set_current_state(TASK_INTERRUPTIBLE);
if (!kthread_should_stop())
schedule();
@@ -1564,9 +1562,7 @@ sleep:
wake_up_process(root->fs_info->cleaner_kthread);
mutex_unlock(&root->fs_info->transaction_kthread_mutex);
- if (freezing(current)) {
- refrigerator();
- } else {
+ if (!try_to_freeze()) {
set_current_state(TASK_INTERRUPTIBLE);
if (!kthread_should_stop() &&
!btrfs_transaction_blocked(root->fs_info))
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 44d0c8d..45e5c43 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2853,8 +2853,7 @@ cont_thread:
}
mutex_unlock(&eli->li_list_mtx);
- if (freezing(current))
- refrigerator();
+ try_to_freeze();
cur = jiffies;
if ((time_after_eq(cur, next_wakeup)) ||
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 5986464..8154d42 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -951,8 +951,8 @@ int gfs2_logd(void *data)
wake_up(&sdp->sd_log_waitq);
t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
- if (freezing(current))
- refrigerator();
+
+ try_to_freeze();
do {
prepare_to_wait(&sdp->sd_logd_waitq, &wait,
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 7e528dc..d49669e 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1427,8 +1427,8 @@ int gfs2_quotad(void *data)
/* Check for & recover partially truncated inodes */
quotad_check_trunc_list(sdp);
- if (freezing(current))
- refrigerator();
+ try_to_freeze();
+
t = min(quotad_timeo, statfs_timeo);
prepare_to_wait(&sdp->sd_quota_wait, &wait, TASK_INTERRUPTIBLE);
diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 9fe061f..908fdf0 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -166,7 +166,7 @@ loop:
*/
jbd_debug(1, "Now suspending kjournald\n");
spin_unlock(&journal->j_state_lock);
- refrigerator();
+ try_to_freeze();
spin_lock(&journal->j_state_lock);
} else {
/*
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index f24df13..874d1c4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -173,7 +173,7 @@ loop:
*/
jbd_debug(1, "Now suspending kjournald2\n");
write_unlock(&journal->j_state_lock);
- refrigerator();
+ try_to_freeze();
write_lock(&journal->j_state_lock);
} else {
/*
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index 583636f..c8c2d35 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -2348,7 +2348,7 @@ int jfsIOWait(void *arg)
if (freezing(current)) {
spin_unlock_irq(&log_redrive_lock);
- refrigerator();
+ try_to_freeze();
} else {
set_current_state(TASK_INTERRUPTIBLE);
spin_unlock_irq(&log_redrive_lock);
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index af96060..bb8b661 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -2800,7 +2800,7 @@ int jfs_lazycommit(void *arg)
if (freezing(current)) {
LAZY_UNLOCK(flags);
- refrigerator();
+ try_to_freeze();
} else {
DECLARE_WAITQUEUE(wq, current);
@@ -2994,7 +2994,7 @@ int jfs_sync(void *arg)
if (freezing(current)) {
TXN_UNLOCK();
- refrigerator();
+ try_to_freeze();
} else {
set_current_state(TASK_INTERRUPTIBLE);
TXN_UNLOCK();
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index bb24ab6..0e72ad6 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2470,7 +2470,7 @@ static int nilfs_segctor_thread(void *arg)
if (freezing(current)) {
spin_unlock(&sci->sc_state_lock);
- refrigerator();
+ try_to_freeze();
spin_lock(&sci->sc_state_lock);
} else {
DEFINE_WAIT(wait);
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index cf0ac05..0188299 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1703,7 +1703,7 @@ xfsbufd(
if (unlikely(freezing(current))) {
set_bit(XBT_FORCE_SLEEP, &target->bt_flags);
- refrigerator();
+ try_to_freeze();
} else {
clear_bit(XBT_FORCE_SLEEP, &target->bt_flags);
}
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index a49b529..984b367 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -47,18 +47,17 @@ static inline bool should_send_signal(struct task_struct *p)
/* Takes and releases task alloc lock using task_lock() */
extern int thaw_process(struct task_struct *p);
-extern void refrigerator(void);
+extern bool __refrigerator(void);
extern int freeze_processes(void);
extern int freeze_kernel_threads(void);
extern void thaw_processes(void);
-static inline int try_to_freeze(void)
+static inline bool try_to_freeze(void)
{
- if (freezing(current)) {
- refrigerator();
- return 1;
- } else
- return 0;
+ might_sleep();
+ if (likely(!freezing(current)))
+ return false;
+ return __refrigerator();
}
extern bool freeze_task(struct task_struct *p, bool sig_only);
@@ -186,12 +185,12 @@ static inline void set_freeze_flag(struct task_struct *p) {}
static inline void clear_freeze_flag(struct task_struct *p) {}
static inline int thaw_process(struct task_struct *p) { return 1; }
-static inline void refrigerator(void) {}
+static inline bool __refrigerator(void) { return false; }
static inline int freeze_processes(void) { return -ENOSYS; }
static inline int freeze_kernel_threads(void) { return -ENOSYS; }
static inline void thaw_processes(void) {}
-static inline int try_to_freeze(void) { return 0; }
+static inline bool try_to_freeze(void) { return false; }
static inline void freezer_do_not_count(void) {}
static inline void freezer_count(void) {}
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 19a20b2..8ab0bdb 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -23,10 +23,11 @@ static inline void frozen_process(void)
}
/* Refrigerator is place where frozen processes are stored :-). */
-void refrigerator(void)
+bool __refrigerator(void)
{
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
+ bool was_frozen = false;
long save;
task_lock(current);
@@ -35,7 +36,7 @@ void refrigerator(void)
task_unlock(current);
} else {
task_unlock(current);
- return;
+ return was_frozen;
}
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -51,6 +52,7 @@ void refrigerator(void)
set_current_state(TASK_UNINTERRUPTIBLE);
if (!frozen(current))
break;
+ was_frozen = true;
schedule();
}
@@ -65,8 +67,10 @@ void refrigerator(void)
* synchronization which depends on ordered task state change.
*/
set_current_state(save);
+
+ return was_frozen;
}
-EXPORT_SYMBOL(refrigerator);
+EXPORT_SYMBOL(__refrigerator);
static void fake_signal_wake_up(struct task_struct *p)
{
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/17] freezer: implement and use kthread_freezable_should_stop()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (2 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 03/17] freezer: unexport refrigerator() and update try_to_freeze() slightly Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation Tejun Heo
` (16 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, Jens Axboe
Writeback and thinkpad_acpi have been using thaw_process() to prevent
deadlock between the freezer and kthread_stop(); unfortunately, this
is inherently racy - nothing prevents freezing from happening between
thaw_process() and kthread_stop().
This patch implements kthread_freezable_should_stop() which enters
refrigerator if necessary but is guaranteed to return if
kthread_stop() is invoked. Both thaw_process() users are converted to
use the new function.
Note that this deadlock condition exists for many of freezable
kthreads. They need to be converted to use the new should_stop or
freezable workqueue.
Tested with synthetic test case.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Oleg Nesterov <oleg@redhat.com>
---
drivers/platform/x86/thinkpad_acpi.c | 15 ++++++---------
fs/fs-writeback.c | 4 +---
include/linux/freezer.h | 6 +++---
include/linux/kthread.h | 1 +
kernel/freezer.c | 6 ++++--
kernel/kthread.c | 25 +++++++++++++++++++++++++
mm/backing-dev.c | 8 ++------
7 files changed, 42 insertions(+), 23 deletions(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 7b82868..4b11fc9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2456,8 +2456,9 @@ static int hotkey_kthread(void *data)
u32 poll_mask, event_mask;
unsigned int si, so;
unsigned long t;
- unsigned int change_detector, must_reset;
+ unsigned int change_detector;
unsigned int poll_freq;
+ bool was_frozen;
mutex_lock(&hotkey_thread_mutex);
@@ -2488,14 +2489,14 @@ static int hotkey_kthread(void *data)
t = 100; /* should never happen... */
}
t = msleep_interruptible(t);
- if (unlikely(kthread_should_stop()))
+ if (unlikely(kthread_freezable_should_stop(&was_frozen)))
break;
- must_reset = try_to_freeze();
- if (t > 0 && !must_reset)
+
+ if (t > 0 && !was_frozen)
continue;
mutex_lock(&hotkey_thread_data_mutex);
- if (must_reset || hotkey_config_change != change_detector) {
+ if (was_frozen || hotkey_config_change != change_detector) {
/* forget old state on thaw or config change */
si = so;
t = 0;
@@ -2528,10 +2529,6 @@ exit:
static void hotkey_poll_stop_sync(void)
{
if (tpacpi_hotkey_task) {
- if (frozen(tpacpi_hotkey_task) ||
- freezing(tpacpi_hotkey_task))
- thaw_process(tpacpi_hotkey_task);
-
kthread_stop(tpacpi_hotkey_task);
tpacpi_hotkey_task = NULL;
mutex_lock(&hotkey_thread_mutex);
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 04cf3b9..b36edb8 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -922,7 +922,7 @@ int bdi_writeback_thread(void *data)
trace_writeback_thread_start(bdi);
- while (!kthread_should_stop()) {
+ while (!kthread_freezable_should_stop(NULL)) {
/*
* Remove own delayed wake-up timer, since we are already awake
* and we'll take care of the preriodic write-back.
@@ -952,8 +952,6 @@ int bdi_writeback_thread(void *data)
*/
schedule();
}
-
- try_to_freeze();
}
/* Flush any work that raced with us exiting */
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 984b367..4a73dd2 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -47,7 +47,7 @@ static inline bool should_send_signal(struct task_struct *p)
/* Takes and releases task alloc lock using task_lock() */
extern int thaw_process(struct task_struct *p);
-extern bool __refrigerator(void);
+extern bool __refrigerator(bool check_kthr_stop);
extern int freeze_processes(void);
extern int freeze_kernel_threads(void);
extern void thaw_processes(void);
@@ -57,7 +57,7 @@ static inline bool try_to_freeze(void)
might_sleep();
if (likely(!freezing(current)))
return false;
- return __refrigerator();
+ return __refrigerator(false);
}
extern bool freeze_task(struct task_struct *p, bool sig_only);
@@ -185,7 +185,7 @@ static inline void set_freeze_flag(struct task_struct *p) {}
static inline void clear_freeze_flag(struct task_struct *p) {}
static inline int thaw_process(struct task_struct *p) { return 1; }
-static inline bool __refrigerator(void) { return false; }
+static inline bool __refrigerator(bool check_kthr_stop) { return false; }
static inline int freeze_processes(void) { return -ENOSYS; }
static inline int freeze_kernel_threads(void) { return -ENOSYS; }
static inline void thaw_processes(void) {}
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 1e923e5..6c1903d 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -35,6 +35,7 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
int kthread_should_stop(void);
+bool kthread_freezable_should_stop(bool *was_frozen);
void *kthread_data(struct task_struct *k);
int kthreadd(void *unused);
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 8ab0bdb..57ba75f 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -9,6 +9,7 @@
#include <linux/module.h>
#include <linux/syscalls.h>
#include <linux/freezer.h>
+#include <linux/kthread.h>
/*
* freezing is complete, mark current process as frozen
@@ -23,7 +24,7 @@ static inline void frozen_process(void)
}
/* Refrigerator is place where frozen processes are stored :-). */
-bool __refrigerator(void)
+bool __refrigerator(bool check_kthr_stop)
{
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
@@ -50,7 +51,8 @@ bool __refrigerator(void)
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
- if (!frozen(current))
+ if (!frozen(current) ||
+ (check_kthr_stop && kthread_should_stop()))
break;
was_frozen = true;
schedule();
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 4ba7ccc..a6cbeea 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -59,6 +59,31 @@ int kthread_should_stop(void)
EXPORT_SYMBOL(kthread_should_stop);
/**
+ * kthread_freezable_should_stop - should this freezable kthread return now?
+ * @was_frozen: optional out parameter, indicates whether %current was frozen
+ *
+ * kthread_should_stop() for freezable kthreads, which will enter
+ * refrigerator if necessary. This function is safe from kthread_stop() /
+ * freezer deadlock and freezable kthreads should use this function instead
+ * of calling try_to_freeze() directly.
+ */
+bool kthread_freezable_should_stop(bool *was_frozen)
+{
+ bool frozen = false;
+
+ might_sleep();
+
+ if (unlikely(freezing(current)))
+ frozen = __refrigerator(true);
+
+ if (was_frozen)
+ *was_frozen = frozen;
+
+ return kthread_should_stop();
+}
+EXPORT_SYMBOL_GPL(kthread_freezable_should_stop);
+
+/**
* kthread_data - return data value specified on kthread creation
* @task: kthread task in question
*
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index a87da52..b08460c1 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -598,14 +598,10 @@ static void bdi_wb_shutdown(struct backing_dev_info *bdi)
/*
* Finally, kill the kernel thread. We don't need to be RCU
- * safe anymore, since the bdi is gone from visibility. Force
- * unfreeze of the thread before calling kthread_stop(), otherwise
- * it would never exet if it is currently stuck in the refrigerator.
+ * safe anymore, since the bdi is gone from visibility.
*/
- if (bdi->wb.task) {
- thaw_process(bdi->wb.task);
+ if (bdi->wb.task)
kthread_stop(bdi->wb.task);
- }
}
/*
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (3 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 04/17] freezer: implement and use kthread_freezable_should_stop() Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-11-06 12:04 ` 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
` (15 subsequent siblings)
20 siblings, 1 reply; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, Paul Menage
thaw_process() now has only internal users - system and cgroup
freezers. Remove the unnecessary return value, rename, unexport and
collapse __thaw_process() into it. This will help further updates to
the freezer code.
-v2: minor style update as suggested by Matt.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Paul Menage <menage@google.com>
Cc: Matt Helsley <matthltc@us.ibm.com>
---
include/linux/freezer.h | 3 +--
kernel/cgroup_freezer.c | 7 +++----
kernel/freezer.c | 31 ++++++++++++-------------------
kernel/power/process.c | 2 +-
4 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 4a73dd2..253bcfe 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -45,7 +45,7 @@ static inline bool should_send_signal(struct task_struct *p)
}
/* Takes and releases task alloc lock using task_lock() */
-extern int thaw_process(struct task_struct *p);
+extern void __thaw_task(struct task_struct *t);
extern bool __refrigerator(bool check_kthr_stop);
extern int freeze_processes(void);
@@ -183,7 +183,6 @@ static inline int frozen(struct task_struct *p) { return 0; }
static inline int freezing(struct task_struct *p) { return 0; }
static inline void set_freeze_flag(struct task_struct *p) {}
static inline void clear_freeze_flag(struct task_struct *p) {}
-static inline int thaw_process(struct task_struct *p) { return 1; }
static inline bool __refrigerator(bool check_kthr_stop) { return false; }
static inline int freeze_processes(void) { return -ENOSYS; }
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index e691818..e7fa0ec 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -130,7 +130,7 @@ struct cgroup_subsys freezer_subsys;
* write_lock css_set_lock (cgroup iterator start)
* task->alloc_lock
* read_lock css_set_lock (cgroup iterator start)
- * task->alloc_lock (inside thaw_process(), prevents race with refrigerator())
+ * task->alloc_lock (inside __thaw_task(), prevents race with refrigerator())
* sighand->siglock
*/
static struct cgroup_subsys_state *freezer_create(struct cgroup_subsys *ss,
@@ -300,9 +300,8 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
struct task_struct *task;
cgroup_iter_start(cgroup, &it);
- while ((task = cgroup_iter_next(cgroup, &it))) {
- thaw_process(task);
- }
+ while ((task = cgroup_iter_next(cgroup, &it)))
+ __thaw_task(task);
cgroup_iter_end(cgroup, &it);
freezer->state = CGROUP_THAWED;
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 57ba75f..8faa0e0 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -145,18 +145,8 @@ void cancel_freezing(struct task_struct *p)
}
}
-static int __thaw_process(struct task_struct *p)
-{
- if (frozen(p)) {
- p->flags &= ~PF_FROZEN;
- return 1;
- }
- clear_freeze_flag(p);
- return 0;
-}
-
/*
- * Wake up a frozen process
+ * Wake up a frozen task
*
* task_lock() is needed to prevent the race with refrigerator() which may
* occur if the freezing of tasks fails. Namely, without the lock, if the
@@ -164,15 +154,18 @@ static int __thaw_process(struct task_struct *p)
* refrigerator() could call frozen_process(), in which case the task would be
* frozen and no one would thaw it.
*/
-int thaw_process(struct task_struct *p)
+void __thaw_task(struct task_struct *p)
{
+ bool was_frozen;
+
task_lock(p);
- if (__thaw_process(p) == 1) {
- task_unlock(p);
- wake_up_process(p);
- return 1;
- }
+ was_frozen = frozen(p);
+ if (was_frozen)
+ p->flags &= ~PF_FROZEN;
+ else
+ clear_freeze_flag(p);
task_unlock(p);
- return 0;
+
+ if (was_frozen)
+ wake_up_process(p);
}
-EXPORT_SYMBOL(thaw_process);
diff --git a/kernel/power/process.c b/kernel/power/process.c
index addbbe5..fe27872 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -186,7 +186,7 @@ static void thaw_tasks(bool nosig_only)
if (cgroup_freezing_or_frozen(p))
continue;
- thaw_process(p);
+ __thaw_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation
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
0 siblings, 1 reply; 29+ messages in thread
From: Srivatsa S. Bhat @ 2011-11-06 12:04 UTC (permalink / raw)
To: Tejun Heo
Cc: rjw, paul, linux-kernel, linux-pm, arnd, oleg, matthltc,
Paul Menage
On 11/01/2011 12:35 AM, Tejun Heo wrote:
> thaw_process() now has only internal users - system and cgroup
> freezers. Remove the unnecessary return value, rename, unexport and
> collapse __thaw_process() into it. This will help further updates to
> the freezer code.
>
> -v2: minor style update as suggested by Matt.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Paul Menage <menage@google.com>
> Cc: Matt Helsley <matthltc@us.ibm.com>
> ---
> include/linux/freezer.h | 3 +--
> kernel/cgroup_freezer.c | 7 +++----
> kernel/freezer.c | 31 ++++++++++++-------------------
> kernel/power/process.c | 2 +-
> 4 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 4a73dd2..253bcfe 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -45,7 +45,7 @@ static inline bool should_send_signal(struct task_struct *p)
> }
>
> /* Takes and releases task alloc lock using task_lock() */
> -extern int thaw_process(struct task_struct *p);
> +extern void __thaw_task(struct task_struct *t);
>
> extern bool __refrigerator(bool check_kthr_stop);
> extern int freeze_processes(void);
> @@ -183,7 +183,6 @@ static inline int frozen(struct task_struct *p) { return 0; }
> static inline int freezing(struct task_struct *p) { return 0; }
> static inline void set_freeze_flag(struct task_struct *p) {}
> static inline void clear_freeze_flag(struct task_struct *p) {}
> -static inline int thaw_process(struct task_struct *p) { return 1; }
>
> static inline bool __refrigerator(bool check_kthr_stop) { return false; }
> static inline int freeze_processes(void) { return -ENOSYS; }
> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
> index e691818..e7fa0ec 100644
> --- a/kernel/cgroup_freezer.c
> +++ b/kernel/cgroup_freezer.c
> @@ -130,7 +130,7 @@ struct cgroup_subsys freezer_subsys;
> * write_lock css_set_lock (cgroup iterator start)
> * task->alloc_lock
> * read_lock css_set_lock (cgroup iterator start)
> - * task->alloc_lock (inside thaw_process(), prevents race with refrigerator())
> + * task->alloc_lock (inside __thaw_task(), prevents race with refrigerator())
^^^^^^^^^
Shouldn't this be __refrigerator()?
> * sighand->siglock
> */
> static struct cgroup_subsys_state *freezer_create(struct cgroup_subsys *ss,
> @@ -300,9 +300,8 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
> struct task_struct *task;
>
> cgroup_iter_start(cgroup, &it);
> - while ((task = cgroup_iter_next(cgroup, &it))) {
> - thaw_process(task);
> - }
> + while ((task = cgroup_iter_next(cgroup, &it)))
> + __thaw_task(task);
> cgroup_iter_end(cgroup, &it);
>
> freezer->state = CGROUP_THAWED;
> diff --git a/kernel/freezer.c b/kernel/freezer.c
> index 57ba75f..8faa0e0 100644
> --- a/kernel/freezer.c
> +++ b/kernel/freezer.c
> @@ -145,18 +145,8 @@ void cancel_freezing(struct task_struct *p)
> }
> }
>
> -static int __thaw_process(struct task_struct *p)
> -{
> - if (frozen(p)) {
> - p->flags &= ~PF_FROZEN;
> - return 1;
> - }
> - clear_freeze_flag(p);
> - return 0;
> -}
> -
> /*
> - * Wake up a frozen process
> + * Wake up a frozen task
> *
> * task_lock() is needed to prevent the race with refrigerator() which may
^^^^^^
__refrigerator()?
> * occur if the freezing of tasks fails. Namely, without the lock, if the
> @@ -164,15 +154,18 @@ static int __thaw_process(struct task_struct *p)
> * refrigerator() could call frozen_process(), in which case the task would be
> * frozen and no one would thaw it.
> */
> -int thaw_process(struct task_struct *p)
> +void __thaw_task(struct task_struct *p)
> {
> + bool was_frozen;
> +
> task_lock(p);
> - if (__thaw_process(p) == 1) {
> - task_unlock(p);
> - wake_up_process(p);
> - return 1;
> - }
> + was_frozen = frozen(p);
> + if (was_frozen)
> + p->flags &= ~PF_FROZEN;
> + else
> + clear_freeze_flag(p);
> task_unlock(p);
> - return 0;
> +
> + if (was_frozen)
> + wake_up_process(p);
> }
> -EXPORT_SYMBOL(thaw_process);
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index addbbe5..fe27872 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -186,7 +186,7 @@ static void thaw_tasks(bool nosig_only)
> if (cgroup_freezing_or_frozen(p))
> continue;
>
> - thaw_process(p);
> + __thaw_task(p);
> } while_each_thread(g, p);
> read_unlock(&tasklist_lock);
> }
Rebasing to latest kernel, we need to do the conversion in mm/oom_kill.c as well,
(and consequently add __thaw_task() empty function under !CONFIG_FREEZER, in
include/linux/freezer.h)
Thanks,
Srivatsa S. Bhat
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation
2011-11-06 12:04 ` Srivatsa S. Bhat
@ 2011-11-06 16:51 ` Tejun Heo
2011-11-06 17:10 ` Srivatsa S. Bhat
0 siblings, 1 reply; 29+ messages in thread
From: Tejun Heo @ 2011-11-06 16:51 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: rjw, paul, linux-kernel, linux-pm, arnd, oleg, matthltc,
Paul Menage
Hello,
On Sun, Nov 06, 2011 at 05:34:27PM +0530, Srivatsa S. Bhat wrote:
> On 11/01/2011 12:35 AM, Tejun Heo wrote:
> > * task_lock() is needed to prevent the race with refrigerator() which may
> ^^^^^^
> __refrigerator()?
I don't think this really matters. Maybe just drop () from the
comment?
> Rebasing to latest kernel, we need to do the conversion in mm/oom_kill.c as well,
> (and consequently add __thaw_task() empty function under !CONFIG_FREEZER, in
> include/linux/freezer.h)
Yes, for merging, we should do that, but for longer term, as we can
modify freezing condition with relative ease now, I think we should
make tasks being killed ignore freezer instead of relying on
explicitly calling __thaw_task() which is inherently racy.
Thank you.
--
tejun
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation
2011-11-06 16:51 ` Tejun Heo
@ 2011-11-06 17:10 ` Srivatsa S. Bhat
0 siblings, 0 replies; 29+ messages in thread
From: Srivatsa S. Bhat @ 2011-11-06 17:10 UTC (permalink / raw)
To: Tejun Heo
Cc: rjw, paul, linux-kernel, linux-pm, arnd, oleg, matthltc,
Paul Menage
On 11/06/2011 10:21 PM, Tejun Heo wrote:
> Hello,
>
> On Sun, Nov 06, 2011 at 05:34:27PM +0530, Srivatsa S. Bhat wrote:
>> On 11/01/2011 12:35 AM, Tejun Heo wrote:
>>> * task_lock() is needed to prevent the race with refrigerator() which may
>> ^^^^^^
>> __refrigerator()?
>
> I don't think this really matters. Maybe just drop () from the
> comment?
Sure, that would do. It was rather trivial anyway...
>
>> Rebasing to latest kernel, we need to do the conversion in mm/oom_kill.c as well,
>> (and consequently add __thaw_task() empty function under !CONFIG_FREEZER, in
>> include/linux/freezer.h)
>
> Yes, for merging, we should do that, but for longer term, as we can
> modify freezing condition with relative ease now, I think we should
> make tasks being killed ignore freezer instead of relying on
> explicitly calling __thaw_task() which is inherently racy.
>
OK.. And yes, many thanks for your efforts to stabilize the freezer
subsystem :)
Thanks,
Srivatsa S. Bhat
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 06/17] freezer: remove racy clear_freeze_flag() and set PF_NOFREEZE on dead tasks
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (4 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 05/17] freezer: rename thaw_process() to __thaw_task() and simplify the implementation Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 07/17] freezer: don't distinguish nosig tasks on thaw Tejun Heo
` (14 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
clear_freeze_flag() in exit_mm() is racy. Freezing can start
afterwards. Remove it. Skipping freezer for exiting task will be
properly implemented later.
Also, freezable() was testing exit_state directly to make system
freezer ignore dead tasks. Let the exiting task set PF_NOFREEZE after
entering TASK_DEAD instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
kernel/exit.c | 3 +--
kernel/power/process.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 2913b35..3d6f570 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -679,8 +679,6 @@ static void exit_mm(struct task_struct * tsk)
tsk->mm = NULL;
up_read(&mm->mmap_sem);
enter_lazy_tlb(mm, current);
- /* We don't want this task to be frozen prematurely */
- clear_freeze_flag(tsk);
if (tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
atomic_dec(&mm->oom_disable_count);
task_unlock(tsk);
@@ -1042,6 +1040,7 @@ NORET_TYPE void do_exit(long code)
exit_rcu();
/* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD;
+ tsk->flags |= PF_NOFREEZE; /* tell freezer to ignore us */
schedule();
BUG();
/* Avoid "noreturn function does return". */
diff --git a/kernel/power/process.c b/kernel/power/process.c
index fe27872..23822dc 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -25,8 +25,7 @@
static inline int freezable(struct task_struct * p)
{
if ((p == current) ||
- (p->flags & PF_NOFREEZE) ||
- (p->exit_state != 0))
+ (p->flags & PF_NOFREEZE))
return 0;
return 1;
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 07/17] freezer: don't distinguish nosig tasks on thaw
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (5 preceding siblings ...)
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 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 08/17] freezer: use dedicated lock instead of task_lock() + memory barrier Tejun Heo
` (13 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
There's no point in thawing nosig tasks before others. There's no
ordering requirement between the two groups on thaw, which the staged
thawing can't guarantee anyway. Simplify thaw_processes() by removing
the distinction and collapsing thaw_tasks() into thaw_processes().
This will help further updates to freezer.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/power/process.c | 20 +++++++-------------
1 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 23822dc..9db048f 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -170,34 +170,28 @@ int freeze_kernel_threads(void)
return error;
}
-static void thaw_tasks(bool nosig_only)
+void thaw_processes(void)
{
struct task_struct *g, *p;
+ oom_killer_enable();
+
+ printk("Restarting tasks ... ");
+
+ thaw_workqueues();
+
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!freezable(p))
continue;
- if (nosig_only && should_send_signal(p))
- continue;
-
if (cgroup_freezing_or_frozen(p))
continue;
__thaw_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
-}
-
-void thaw_processes(void)
-{
- oom_killer_enable();
- printk("Restarting tasks ... ");
- thaw_workqueues();
- thaw_tasks(true);
- thaw_tasks(false);
schedule();
printk("done.\n");
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 08/17] freezer: use dedicated lock instead of task_lock() + memory barrier
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (6 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 07/17] freezer: don't distinguish nosig tasks on thaw Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 09/17] freezer: make freezing indicate freeze condition in effect Tejun Heo
` (12 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
Freezer synchronization is needlessly complicated - it's by no means a
hot path and the priority is staying unintrusive and safe. This patch
makes it simply use a dedicated lock instead of piggy-backing on
task_lock() and playing with memory barriers.
On the failure path of try_to_freeze_tasks(), locking is moved from it
to cancel_freezing(). This makes the frozen() test racy but the race
here is a non-issue as the warning is printed for tasks which failed
to enter frozen for 20 seconds and race on PF_FROZEN at the last
moment doesn't change anything.
This simplifies freezer implementation and eases further changes
including some race fixes.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
kernel/freezer.c | 84 +++++++++++++++++++++---------------------------
kernel/power/process.c | 2 -
2 files changed, 37 insertions(+), 49 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 8faa0e0..10dd2e0 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -11,17 +11,8 @@
#include <linux/freezer.h>
#include <linux/kthread.h>
-/*
- * freezing is complete, mark current process as frozen
- */
-static inline void frozen_process(void)
-{
- if (!unlikely(current->flags & PF_NOFREEZE)) {
- current->flags |= PF_FROZEN;
- smp_wmb();
- }
- clear_freeze_flag(current);
-}
+/* protects freezing and frozen transitions */
+static DEFINE_SPINLOCK(freezer_lock);
/* Refrigerator is place where frozen processes are stored :-). */
bool __refrigerator(bool check_kthr_stop)
@@ -31,14 +22,16 @@ bool __refrigerator(bool check_kthr_stop)
bool was_frozen = false;
long save;
- task_lock(current);
- if (freezing(current)) {
- frozen_process();
- task_unlock(current);
- } else {
- task_unlock(current);
+ spin_lock_irq(&freezer_lock);
+ if (!freezing(current)) {
+ spin_unlock_irq(&freezer_lock);
return was_frozen;
}
+ if (!(current->flags & PF_NOFREEZE))
+ current->flags |= PF_FROZEN;
+ clear_freeze_flag(current);
+ spin_unlock_irq(&freezer_lock);
+
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -99,21 +92,18 @@ static void fake_signal_wake_up(struct task_struct *p)
*/
bool freeze_task(struct task_struct *p, bool sig_only)
{
- /*
- * We first check if the task is freezing and next if it has already
- * been frozen to avoid the race with frozen_process() which first marks
- * the task as frozen and next clears its TIF_FREEZE.
- */
- if (!freezing(p)) {
- smp_rmb();
- if (frozen(p))
- return false;
-
- if (!sig_only || should_send_signal(p))
- set_freeze_flag(p);
- else
- return false;
- }
+ unsigned long flags;
+ bool ret = false;
+
+ spin_lock_irqsave(&freezer_lock, flags);
+
+ if (sig_only && !should_send_signal(p))
+ goto out_unlock;
+
+ if (frozen(p))
+ goto out_unlock;
+
+ set_freeze_flag(p);
if (should_send_signal(p)) {
fake_signal_wake_up(p);
@@ -123,26 +113,28 @@ bool freeze_task(struct task_struct *p, bool sig_only)
* TASK_RUNNING transition can't race with task state
* testing in try_to_freeze_tasks().
*/
- } else if (sig_only) {
- return false;
} else {
wake_up_state(p, TASK_INTERRUPTIBLE);
}
-
- return true;
+ ret = true;
+out_unlock:
+ spin_unlock_irqrestore(&freezer_lock, flags);
+ 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_irqsave(&p->sighand->siglock, flags);
+ spin_lock(&p->sighand->siglock);
recalc_sigpending_and_wake(p);
- spin_unlock_irqrestore(&p->sighand->siglock, flags);
+ spin_unlock(&p->sighand->siglock);
}
+ spin_unlock_irqrestore(&freezer_lock, flags);
}
/*
@@ -156,16 +148,14 @@ void cancel_freezing(struct task_struct *p)
*/
void __thaw_task(struct task_struct *p)
{
- bool was_frozen;
+ unsigned long flags;
- task_lock(p);
- was_frozen = frozen(p);
- if (was_frozen)
+ spin_lock_irqsave(&freezer_lock, flags);
+ if (frozen(p)) {
p->flags &= ~PF_FROZEN;
- else
- clear_freeze_flag(p);
- task_unlock(p);
-
- if (was_frozen)
wake_up_process(p);
+ } else {
+ clear_freeze_flag(p);
+ }
+ spin_unlock_irqrestore(&freezer_lock, flags);
}
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 9db048f..bd420ca 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -118,11 +118,9 @@ static int try_to_freeze_tasks(bool sig_only)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- task_lock(p);
if (!wakeup && freezing(p) && !freezer_should_skip(p))
sched_show_task(p);
cancel_freezing(p);
- task_unlock(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
} else {
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 09/17] freezer: make freezing indicate freeze condition in effect
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (7 preceding siblings ...)
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 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 10/17] freezer: test freezable conditions while holding freezer_lock Tejun Heo
` (11 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
Currently freezing (TIF_FREEZE) and frozen (PF_FROZEN) states are
interlocked - freezing is set to request freeze and when the task
actually freezes, it clears freezing and sets frozen.
This interlocking makes things more complex than necessary - freezing
doesn't mean there's freezing condition in effect and frozen doesn't
match the task actually entering and leaving frozen state (it's
cleared by the thawing task).
This patch makes freezing indicate that freeze condition is in effect.
A task enters and stays frozen if freezing. This makes PF_FROZEN
manipulation done only by the task itself and prevents wakeup from
__thaw_task() leaking outside of refrigerator.
The only place which needs to tell freezing && !frozen is
try_to_freeze_task() to whine about tasks which don't enter frozen.
It's updated to test the condition explicitly.
With the change, frozen() state my linger after __thaw_task() until
the task wakes up and exits fridge. This can trigger BUG_ON() in
update_if_frozen(). Work it around by testing freezing() && frozen()
instead of frozen().
-v2: Oleg pointed out missing re-check of freezing() when trying to
clear FROZEN and possible spurious BUG_ON() trigger in
update_if_frozen(). Both fixed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Menage <paul@paulmenage.org>
---
kernel/cgroup_freezer.c | 2 +-
kernel/freezer.c | 42 ++++++++++++++++++++++++------------------
kernel/power/process.c | 3 ++-
3 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index e7fa0ec..1a73727 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -231,7 +231,7 @@ static void update_if_frozen(struct cgroup *cgroup,
cgroup_iter_start(cgroup, &it);
while ((task = cgroup_iter_next(cgroup, &it))) {
ntotal++;
- if (frozen(task))
+ if (freezing(task) && frozen(task))
nfrozen++;
}
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 10dd2e0..05afd68 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -22,14 +22,19 @@ bool __refrigerator(bool check_kthr_stop)
bool was_frozen = false;
long save;
+ /*
+ * Enter FROZEN. If NOFREEZE, schedule immediate thawing by
+ * clearing freezing.
+ */
spin_lock_irq(&freezer_lock);
+repeat:
if (!freezing(current)) {
spin_unlock_irq(&freezer_lock);
return was_frozen;
}
- if (!(current->flags & PF_NOFREEZE))
- current->flags |= PF_FROZEN;
- clear_freeze_flag(current);
+ if (current->flags & PF_NOFREEZE)
+ clear_freeze_flag(current);
+ current->flags |= PF_FROZEN;
spin_unlock_irq(&freezer_lock);
save = current->state;
@@ -44,7 +49,7 @@ bool __refrigerator(bool check_kthr_stop)
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
- if (!frozen(current) ||
+ if (!freezing(current) ||
(check_kthr_stop && kthread_should_stop()))
break;
was_frozen = true;
@@ -54,6 +59,13 @@ bool __refrigerator(bool check_kthr_stop)
/* Remove the accounting blocker */
current->flags &= ~PF_FREEZING;
+ /* leave FROZEN */
+ spin_lock_irq(&freezer_lock);
+ if (freezing(current))
+ goto repeat;
+ current->flags &= ~PF_FROZEN;
+ spin_unlock_irq(&freezer_lock);
+
pr_debug("%s left refrigerator\n", current->comm);
/*
@@ -137,25 +149,19 @@ void cancel_freezing(struct task_struct *p)
spin_unlock_irqrestore(&freezer_lock, flags);
}
-/*
- * Wake up a frozen task
- *
- * task_lock() is needed to prevent the race with refrigerator() which may
- * occur if the freezing of tasks fails. Namely, without the lock, if the
- * freezing of tasks failed, thaw_tasks() might have run before a task in
- * refrigerator() could call frozen_process(), in which case the task would be
- * frozen and no one would thaw it.
- */
void __thaw_task(struct task_struct *p)
{
unsigned long flags;
+ /*
+ * Clear freezing and kick @p if FROZEN. Clearing is guaranteed to
+ * be visible to @p as waking up implies wmb. Waking up inside
+ * freezer_lock also prevents wakeups from leaking outside
+ * refrigerator.
+ */
spin_lock_irqsave(&freezer_lock, flags);
- if (frozen(p)) {
- p->flags &= ~PF_FROZEN;
+ clear_freeze_flag(p);
+ if (frozen(p))
wake_up_process(p);
- } else {
- clear_freeze_flag(p);
- }
spin_unlock_irqrestore(&freezer_lock, flags);
}
diff --git a/kernel/power/process.c b/kernel/power/process.c
index bd420ca..e6e2739 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -118,7 +118,8 @@ static int try_to_freeze_tasks(bool sig_only)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (!wakeup && freezing(p) && !freezer_should_skip(p))
+ if (!wakeup && !freezer_should_skip(p) &&
+ freezing(p) && !frozen(p))
sched_show_task(p);
cancel_freezing(p);
} while_each_thread(g, p);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/17] freezer: test freezable conditions while holding freezer_lock
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (8 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 09/17] freezer: make freezing indicate freeze condition in effect Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 11/17] freezer: kill PF_FREEZING Tejun Heo
` (10 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
try_to_freeze_tasks() and thaw_processes() use freezable() and
frozen() as preliminary tests before initiating operations on a task.
These are done without any synchronization and hinder with
synchronization cleanup without any real performance benefits.
In try_to_freeze_tasks(), open code self test and move PF_NOFREEZE and
frozen() tests inside freezer_lock in freeze_task().
thaw_processes() can simply drop freezable() test as frozen() test in
__thaw_task() is enough.
Note: This used to be a part of larger patch to fix set_freezable()
race. Separated out to satisfy ordering among dependent fixes.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
kernel/freezer.c | 3 ++-
kernel/power/process.c | 16 +---------------
2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 05afd68..5c911c0 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -109,7 +109,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))
diff --git a/kernel/power/process.c b/kernel/power/process.c
index e6e2739..e59676f 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;
/*
@@ -181,9 +170,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.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 11/17] freezer: kill PF_FREEZING
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (9 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 10/17] freezer: test freezable conditions while holding freezer_lock Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 12/17] freezer: clean up freeze_processes() failure path Tejun Heo
` (9 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
With the previous changes, there's no meaningful difference between
PF_FREEZING and PF_FROZEN. Remove PF_FREEZING and use PF_FROZEN
instead in task_contributes_to_load().
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/sched.h | 3 +--
kernel/freezer.c | 6 ------
2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e8acce7..57f916d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -220,7 +220,7 @@ extern char ___assert_task_state[1 - 2*!!(
((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
#define task_contributes_to_load(task) \
((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
- (task->flags & PF_FREEZING) == 0)
+ (task->flags & PF_FROZEN) == 0)
#define __set_task_state(tsk, state_value) \
do { (tsk)->state = (state_value); } while (0)
@@ -1766,7 +1766,6 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
#define PF_MEMALLOC 0x00000800 /* Allocating memory */
#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */
#define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */
-#define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */
#define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */
#define PF_FROZEN 0x00010000 /* frozen for system suspend */
#define PF_FSTRANS 0x00020000 /* inside a filesystem transaction */
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 5c911c0..3be1e36 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -44,9 +44,6 @@ repeat:
recalc_sigpending(); /* We sent fake signal, clean it up */
spin_unlock_irq(¤t->sighand->siglock);
- /* prevent accounting of that task to load */
- current->flags |= PF_FREEZING;
-
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
if (!freezing(current) ||
@@ -56,9 +53,6 @@ repeat:
schedule();
}
- /* Remove the accounting blocker */
- current->flags &= ~PF_FREEZING;
-
/* leave FROZEN */
spin_lock_irq(&freezer_lock);
if (freezing(current))
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/17] freezer: clean up freeze_processes() failure path
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (10 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 11/17] freezer: kill PF_FREEZING Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-11-03 19:09 ` Srivatsa S. Bhat
2011-11-03 22:25 ` [PATCH UPDATED " Tejun Heo
2011-10-31 19:05 ` [PATCH 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE Tejun Heo
` (8 subsequent siblings)
20 siblings, 2 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
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 thaw_processes() call thaw_processes() on failure instead so
that the system is fully thawed on failure. Unnecessary
thaw_processes() calls are removed from kernel/power/hibernate.c and
user.c.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/freezer.h | 1 -
kernel/freezer.c | 25 +++++++++----------------
kernel/power/hibernate.c | 15 ++-------------
kernel/power/process.c | 12 ++++--------
kernel/power/user.c | 4 +---
5 files changed, 16 insertions(+), 41 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..85ecaec 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 {
@@ -137,6 +129,8 @@ int freeze_processes(void)
printk("\n");
BUG_ON(in_atomic());
+ if (error)
+ thaw_processes();
return error;
}
@@ -155,6 +149,8 @@ int freeze_kernel_threads(void)
printk("\n");
BUG_ON(in_atomic());
+ if (error)
+ thaw_processes();
return error;
}
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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 12/17] freezer: clean up freeze_processes() failure path
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 ` [PATCH UPDATED " Tejun Heo
1 sibling, 0 replies; 29+ messages in thread
From: Srivatsa S. Bhat @ 2011-11-03 19:09 UTC (permalink / raw)
To: Tejun Heo; +Cc: rjw, paul, linux-kernel, linux-pm, arnd, oleg, matthltc
Hi,
On 11/01/2011 12:35 AM, Tejun Heo wrote:
> 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 thaw_processes() call thaw_processes() on failure instead so
^^^^^^^^^^^^^^
You meant freeze_processes() and freeze_kernel_threads() here, right?
> that the system is fully thawed on failure.
I really like this approach of handling freezing failures.
> Unnecessary
> thaw_processes() calls are removed from kernel/power/hibernate.c and
> user.c.
>
Did you miss kernel/power/suspend.c?
Shouldn't we be also doing something like this:
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index fdd4263..57aaca5 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -110,7 +110,6 @@ static int suspend_prepare(void)
} else
return 0;
- suspend_thaw_processes();
usermodehelper_enable();
Finish:
pm_notifier_call_chain(PM_POST_SUSPEND);
---
Regards,
Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Linux Technology Center,
IBM India Systems and Technology Lab
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH UPDATED 12/17] freezer: clean up freeze_processes() failure path
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
1 sibling, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-11-03 22:25 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Srivatsa S. Bhat
>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
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (11 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 12/17] freezer: clean up freeze_processes() failure path Tejun Heo
@ 2011-10-31 19:05 ` 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
` (7 subsequent siblings)
20 siblings, 1 reply; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, Li Zefan
TIF_FREEZE will be removed soon and freezing() will directly test
whether any freezing condition is in effect. Make the following
changes in preparation.
* Rename cgroup_freezing_or_frozen() to cgroup_freezing() and make it
return bool.
* Make cgroup_freezing() access task_freezer() under rcu read lock
instead of task_lock(). This makes the state dereferencing racy
against task moving to another cgroup; however, it was already racy
without this change as ->state dereference wasn't synchronized.
This will be later dealt with using attach hooks.
* freezer->state is now set before trying to push tasks into the
target state.
-v2: Oleg pointed out that freeze_change_state() was setting
freeze->state incorrectly to CGROUP_FROZEN instead of
CGROUP_FREEZING. Fixed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Paul Menage <paul@paulmenage.org>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Oleg Nesterov <oleg@redhat.com>
---
include/linux/freezer.h | 6 +++---
kernel/cgroup_freezer.c | 36 ++++++++++++------------------------
kernel/power/process.c | 2 +-
3 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index e6657778..243ec0b 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -63,11 +63,11 @@ static inline bool try_to_freeze(void)
extern bool freeze_task(struct task_struct *p, bool sig_only);
#ifdef CONFIG_CGROUP_FREEZER
-extern int cgroup_freezing_or_frozen(struct task_struct *task);
+extern bool cgroup_freezing(struct task_struct *task);
#else /* !CONFIG_CGROUP_FREEZER */
-static inline int cgroup_freezing_or_frozen(struct task_struct *task)
+static inline bool cgroup_freezing(struct task_struct *task)
{
- return 0;
+ return false;
}
#endif /* !CONFIG_CGROUP_FREEZER */
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 1a73727..2fae8b2 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -48,19 +48,17 @@ static inline struct freezer *task_freezer(struct task_struct *task)
struct freezer, css);
}
-static inline int __cgroup_freezing_or_frozen(struct task_struct *task)
+bool cgroup_freezing(struct task_struct *task)
{
- enum freezer_state state = task_freezer(task)->state;
- return (state == CGROUP_FREEZING) || (state == CGROUP_FROZEN);
-}
+ enum freezer_state state;
+ bool ret;
-int cgroup_freezing_or_frozen(struct task_struct *task)
-{
- int result;
- task_lock(task);
- result = __cgroup_freezing_or_frozen(task);
- task_unlock(task);
- return result;
+ rcu_read_lock();
+ state = task_freezer(task)->state;
+ ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+ rcu_read_unlock();
+
+ return ret;
}
/*
@@ -102,9 +100,6 @@ struct cgroup_subsys freezer_subsys;
* freezer_can_attach():
* cgroup_mutex (held by caller of can_attach)
*
- * cgroup_freezing_or_frozen():
- * task->alloc_lock (to get task's cgroup)
- *
* freezer_fork() (preserving fork() performance means can't take cgroup_mutex):
* freezer->lock
* sighand->siglock (if the cgroup is freezing)
@@ -177,13 +172,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
static int freezer_can_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
{
- rcu_read_lock();
- if (__cgroup_freezing_or_frozen(tsk)) {
- rcu_read_unlock();
- return -EBUSY;
- }
- rcu_read_unlock();
- return 0;
+ return cgroup_freezing(tsk) ? -EBUSY : 0;
}
static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
@@ -279,7 +268,6 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
struct task_struct *task;
unsigned int num_cant_freeze_now = 0;
- freezer->state = CGROUP_FREEZING;
cgroup_iter_start(cgroup, &it);
while ((task = cgroup_iter_next(cgroup, &it))) {
if (!freeze_task(task, true))
@@ -303,8 +291,6 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
while ((task = cgroup_iter_next(cgroup, &it)))
__thaw_task(task);
cgroup_iter_end(cgroup, &it);
-
- freezer->state = CGROUP_THAWED;
}
static int freezer_change_state(struct cgroup *cgroup,
@@ -323,9 +309,11 @@ static int freezer_change_state(struct cgroup *cgroup,
switch (goal_state) {
case CGROUP_THAWED:
+ freezer->state = CGROUP_THAWED;
unfreeze_cgroup(cgroup, freezer);
break;
case CGROUP_FROZEN:
+ freezer->state = CGROUP_FREEZING;
retval = try_to_freeze_cgroup(cgroup, freezer);
break;
default:
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 85ecaec..54ba145 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -166,7 +166,7 @@ void thaw_processes(void)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (cgroup_freezing_or_frozen(p))
+ if (cgroup_freezing(p))
continue;
__thaw_task(p);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH UPDATED 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE
2011-10-31 19:05 ` [PATCH 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE Tejun Heo
@ 2011-10-31 22:33 ` Tejun Heo
0 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:33 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Li Zefan
TIF_FREEZE will be removed soon and freezing() will directly test
whether any freezing condition is in effect. Make the following
changes in preparation.
* Rename cgroup_freezing_or_frozen() to cgroup_freezing() and make it
return bool.
* Make cgroup_freezing() access task_freezer() under rcu read lock
instead of task_lock(). This makes the state dereferencing racy
against task moving to another cgroup; however, it was already racy
without this change as ->state dereference wasn't synchronized.
This will be later dealt with using attach hooks.
* freezer->state is now set before trying to push tasks into the
target state.
-v2: Oleg pointed out that freeze_change_state() was setting
freeze->state incorrectly to CGROUP_FROZEN instead of
CGROUP_FREEZING. Fixed.
-v3: Matt pointed out that setting CGROUP_FROZEN used to always invoke
try_to_freeze_cgroup() regardless of the current state. Patch
updated such that the actual freeze/thaw operations are always
performed on invocation. This shouldn't make any difference
unless something is broken.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Paul Menage <paul@paulmenage.org>
Cc: Li Zefan <lizf@cn.fujitsu.com>
Cc: Oleg Nesterov <oleg@redhat.com>
---
Later fix patched merged. Please read reply to the head message for
details. Thanks.
include/linux/freezer.h | 6 +++---
kernel/cgroup_freezer.c | 40 +++++++++++++---------------------------
kernel/power/process.c | 2 +-
3 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index e6657778..243ec0b 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -63,11 +63,11 @@ static inline bool try_to_freeze(void)
extern bool freeze_task(struct task_struct *p, bool sig_only);
#ifdef CONFIG_CGROUP_FREEZER
-extern int cgroup_freezing_or_frozen(struct task_struct *task);
+extern bool cgroup_freezing(struct task_struct *task);
#else /* !CONFIG_CGROUP_FREEZER */
-static inline int cgroup_freezing_or_frozen(struct task_struct *task)
+static inline bool cgroup_freezing(struct task_struct *task)
{
- return 0;
+ return false;
}
#endif /* !CONFIG_CGROUP_FREEZER */
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 1a73727..c72ec7e 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -48,19 +48,17 @@ static inline struct freezer *task_freezer(struct task_struct *task)
struct freezer, css);
}
-static inline int __cgroup_freezing_or_frozen(struct task_struct *task)
+bool cgroup_freezing(struct task_struct *task)
{
- enum freezer_state state = task_freezer(task)->state;
- return (state == CGROUP_FREEZING) || (state == CGROUP_FROZEN);
-}
+ enum freezer_state state;
+ bool ret;
-int cgroup_freezing_or_frozen(struct task_struct *task)
-{
- int result;
- task_lock(task);
- result = __cgroup_freezing_or_frozen(task);
- task_unlock(task);
- return result;
+ rcu_read_lock();
+ state = task_freezer(task)->state;
+ ret = state == CGROUP_FREEZING || state == CGROUP_FROZEN;
+ rcu_read_unlock();
+
+ return ret;
}
/*
@@ -102,9 +100,6 @@ struct cgroup_subsys freezer_subsys;
* freezer_can_attach():
* cgroup_mutex (held by caller of can_attach)
*
- * cgroup_freezing_or_frozen():
- * task->alloc_lock (to get task's cgroup)
- *
* freezer_fork() (preserving fork() performance means can't take cgroup_mutex):
* freezer->lock
* sighand->siglock (if the cgroup is freezing)
@@ -177,13 +172,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
static int freezer_can_attach_task(struct cgroup *cgrp, struct task_struct *tsk)
{
- rcu_read_lock();
- if (__cgroup_freezing_or_frozen(tsk)) {
- rcu_read_unlock();
- return -EBUSY;
- }
- rcu_read_unlock();
- return 0;
+ return cgroup_freezing(tsk) ? -EBUSY : 0;
}
static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
@@ -279,7 +268,6 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
struct task_struct *task;
unsigned int num_cant_freeze_now = 0;
- freezer->state = CGROUP_FREEZING;
cgroup_iter_start(cgroup, &it);
while ((task = cgroup_iter_next(cgroup, &it))) {
if (!freeze_task(task, true))
@@ -303,8 +291,6 @@ static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
while ((task = cgroup_iter_next(cgroup, &it)))
__thaw_task(task);
cgroup_iter_end(cgroup, &it);
-
- freezer->state = CGROUP_THAWED;
}
static int freezer_change_state(struct cgroup *cgroup,
@@ -318,20 +304,20 @@ static int freezer_change_state(struct cgroup *cgroup,
spin_lock_irq(&freezer->lock);
update_if_frozen(cgroup, freezer);
- if (goal_state == freezer->state)
- goto out;
switch (goal_state) {
case CGROUP_THAWED:
+ freezer->state = CGROUP_THAWED;
unfreeze_cgroup(cgroup, freezer);
break;
case CGROUP_FROZEN:
+ freezer->state = CGROUP_FREEZING;
retval = try_to_freeze_cgroup(cgroup, freezer);
break;
default:
BUG();
}
-out:
+
spin_unlock_irq(&freezer->lock);
return retval;
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 85ecaec..54ba145 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -166,7 +166,7 @@ void thaw_processes(void)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (cgroup_freezing_or_frozen(p))
+ if (cgroup_freezing(p))
continue;
__thaw_task(p);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 14/17] freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (12 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 13/17] cgroup_freezer: prepare for removal of TIF_FREEZE Tejun Heo
@ 2011-10-31 19:05 ` 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
` (6 subsequent siblings)
20 siblings, 1 reply; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
Using TIF_FREEZE for freezing worked when there was only single
freezing condition (the PM one); however, now there is also the
cgroup_freezer and single bit flag is getting clumsy.
thaw_processes() is already testing whether cgroup freezing in in
effect to avoid thawing tasks which were frozen by both PM and cgroup
freezers.
This is racy (nothing prevents race against cgroup freezing) and
fragile. A much simpler way is to test actual freeze conditions from
freezing() - ie. directly test whether PM or cgroup freezing is in
effect.
This patch adds variables to indicate whether and what type of
freezing conditions are in effect and reimplements freezing() such
that it directly tests whether any of the two freezing conditions is
active and the task should freeze. On fast path, freezing() is still
very cheap - it only tests system_freezing_cnt.
This makes the clumsy dancing aroung TIF_FREEZE unnecessary and
freeze/thaw operations more usual - updating state variables for the
new state and nudging target tasks so that they notice the new state
and comply. As long as the nudging happens after state update, it's
race-free.
* This allows use of freezing() in freeze_task(). Replace the open
coded tests with freezing().
* p != current test is added to warning printing conditions in
try_to_freeze_tasks() failure path. This is necessary as freezing()
is now true for the task which initiated freezing too.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Paul Menage <paul@paulmenage.org> (for the cgroup portions)
---
include/linux/freezer.h | 33 +++++++++----------------
kernel/cgroup_freezer.c | 8 +++++-
kernel/fork.c | 1 -
kernel/freezer.c | 62 ++++++++++++++++++++++++++++++----------------
kernel/power/process.c | 15 ++++++++---
5 files changed, 70 insertions(+), 49 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 243ec0b..6152e46 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -5,8 +5,13 @@
#include <linux/sched.h>
#include <linux/wait.h>
+#include <linux/atomic.h>
#ifdef CONFIG_FREEZER
+extern atomic_t system_freezing_cnt; /* nr of freezing conds in effect */
+extern bool pm_freezing; /* PM freezing in effect */
+extern bool pm_nosig_freezing; /* PM nosig freezing in effect */
+
/*
* Check if a process has been frozen
*/
@@ -15,28 +20,16 @@ static inline int frozen(struct task_struct *p)
return p->flags & PF_FROZEN;
}
-/*
- * Check if there is a request to freeze a process
- */
-static inline int freezing(struct task_struct *p)
-{
- return test_tsk_thread_flag(p, TIF_FREEZE);
-}
+extern bool freezing_slow_path(struct task_struct *p);
/*
- * Request that a process be frozen
- */
-static inline void set_freeze_flag(struct task_struct *p)
-{
- set_tsk_thread_flag(p, TIF_FREEZE);
-}
-
-/*
- * Sometimes we may need to cancel the previous 'freeze' request
+ * Check if there is a request to freeze a process
*/
-static inline void clear_freeze_flag(struct task_struct *p)
+static inline bool freezing(struct task_struct *p)
{
- clear_tsk_thread_flag(p, TIF_FREEZE);
+ if (likely(!atomic_read(&system_freezing_cnt)))
+ return false;
+ return freezing_slow_path(p);
}
static inline bool should_send_signal(struct task_struct *p)
@@ -179,9 +172,7 @@ static inline void set_freezable_with_signal(void)
})
#else /* !CONFIG_FREEZER */
static inline int frozen(struct task_struct *p) { return 0; }
-static inline int freezing(struct task_struct *p) { return 0; }
-static inline void set_freeze_flag(struct task_struct *p) {}
-static inline void clear_freeze_flag(struct task_struct *p) {}
+static inline bool freezing(struct task_struct *p) { return false; }
static inline bool __refrigerator(bool check_kthr_stop) { return false; }
static inline int freeze_processes(void) { return -ENOSYS; }
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 2fae8b2..c3b77c6 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -145,7 +145,11 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup_subsys *ss,
static void freezer_destroy(struct cgroup_subsys *ss,
struct cgroup *cgroup)
{
- kfree(cgroup_freezer(cgroup));
+ struct freezer *freezer = cgroup_freezer(cgroup);
+
+ if (freezer->state != CGROUP_THAWED)
+ atomic_dec(&system_freezing_cnt);
+ kfree(freezer);
}
/*
@@ -309,10 +313,12 @@ static int freezer_change_state(struct cgroup *cgroup,
switch (goal_state) {
case CGROUP_THAWED:
+ atomic_dec(&system_freezing_cnt);
freezer->state = CGROUP_THAWED;
unfreeze_cgroup(cgroup, freezer);
break;
case CGROUP_FROZEN:
+ atomic_inc(&system_freezing_cnt);
freezer->state = CGROUP_FREEZING;
retval = try_to_freeze_cgroup(cgroup, freezer);
break;
diff --git a/kernel/fork.c b/kernel/fork.c
index 8e6b6f4..fa7beb3 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1000,7 +1000,6 @@ static void copy_flags(unsigned long clone_flags, struct task_struct *p)
new_flags |= PF_FORKNOEXEC;
new_flags |= PF_STARTING;
p->flags = new_flags;
- clear_freeze_flag(p);
}
SYSCALL_DEFINE1(set_tid_address, int __user *, tidptr)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 7a9f706..a797f41 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -11,9 +11,41 @@
#include <linux/freezer.h>
#include <linux/kthread.h>
+/* total number of freezing conditions in effect */
+atomic_t system_freezing_cnt = ATOMIC_INIT(0);
+EXPORT_SYMBOL(system_freezing_cnt);
+
+/* indicate whether PM freezing is in effect, protected by pm_mutex */
+bool pm_freezing;
+bool pm_nosig_freezing;
+
/* protects freezing and frozen transitions */
static DEFINE_SPINLOCK(freezer_lock);
+/**
+ * freezing_slow_path - slow path for testing whether a task needs to be frozen
+ * @p: task to be tested
+ *
+ * This function is called by freezing() if system_freezing_cnt isn't zero
+ * and tests whether @p needs to enter and stay in frozen state. Can be
+ * called under any context. The freezers are responsible for ensuring the
+ * target tasks see the updated state.
+ */
+bool freezing_slow_path(struct task_struct *p)
+{
+ if (p->flags & PF_NOFREEZE)
+ return false;
+
+ if (pm_nosig_freezing || cgroup_freezing(p))
+ return true;
+
+ if (pm_freezing && !(p->flags & PF_FREEZER_NOSIG))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(freezing_slow_path);
+
/* Refrigerator is place where frozen processes are stored :-). */
bool __refrigerator(bool check_kthr_stop)
{
@@ -23,17 +55,11 @@ bool __refrigerator(bool check_kthr_stop)
long save;
/*
- * Enter FROZEN. If NOFREEZE, schedule immediate thawing by
- * clearing freezing.
+ * No point in checking freezing() again - the caller already did.
+ * Proceed to enter FROZEN.
*/
spin_lock_irq(&freezer_lock);
repeat:
- if (!freezing(current)) {
- spin_unlock_irq(&freezer_lock);
- return was_frozen;
- }
- if (current->flags & PF_NOFREEZE)
- clear_freeze_flag(current);
current->flags |= PF_FROZEN;
spin_unlock_irq(&freezer_lock);
@@ -99,18 +125,12 @@ static void fake_signal_wake_up(struct task_struct *p)
bool freeze_task(struct task_struct *p, bool sig_only)
{
unsigned long flags;
- bool ret = false;
spin_lock_irqsave(&freezer_lock, flags);
-
- if ((p->flags & PF_NOFREEZE) ||
- (sig_only && !should_send_signal(p)))
- goto out_unlock;
-
- if (frozen(p))
- goto out_unlock;
-
- set_freeze_flag(p);
+ if (!freezing(p) || frozen(p)) {
+ spin_unlock_irqrestore(&freezer_lock, flags);
+ return false;
+ }
if (should_send_signal(p)) {
fake_signal_wake_up(p);
@@ -123,10 +143,9 @@ bool freeze_task(struct task_struct *p, bool sig_only)
} else {
wake_up_state(p, TASK_INTERRUPTIBLE);
}
- ret = true;
-out_unlock:
+
spin_unlock_irqrestore(&freezer_lock, flags);
- return ret;
+ return true;
}
void __thaw_task(struct task_struct *p)
@@ -143,7 +162,6 @@ void __thaw_task(struct task_struct *p)
* avoid leaving dangling TIF_SIGPENDING behind.
*/
spin_lock_irqsave(&freezer_lock, flags);
- clear_freeze_flag(p);
if (frozen(p)) {
wake_up_process(p);
} else {
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 54ba145..eed8995 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -101,7 +101,7 @@ static int try_to_freeze_tasks(bool sig_only)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!wakeup && !freezer_should_skip(p) &&
- freezing(p) && !frozen(p))
+ p != current && freezing(p) && !frozen(p))
sched_show_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
@@ -120,7 +120,11 @@ int freeze_processes(void)
{
int error;
+ if (!pm_freezing)
+ atomic_inc(&system_freezing_cnt);
+
printk("Freezing user space processes ... ");
+ pm_freezing = true;
error = try_to_freeze_tasks(true);
if (!error) {
printk("done.");
@@ -142,6 +146,7 @@ int freeze_kernel_threads(void)
int error;
printk("Freezing remaining freezable tasks ... ");
+ pm_nosig_freezing = true;
error = try_to_freeze_tasks(false);
if (!error)
printk("done.");
@@ -158,6 +163,11 @@ void thaw_processes(void)
{
struct task_struct *g, *p;
+ if (pm_freezing)
+ atomic_dec(&system_freezing_cnt);
+ pm_freezing = false;
+ pm_nosig_freezing = false;
+
oom_killer_enable();
printk("Restarting tasks ... ");
@@ -166,9 +176,6 @@ void thaw_processes(void)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (cgroup_freezing(p))
- continue;
-
__thaw_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH UPDATED 14/17] freezer: make freezing() test freeze conditions in effect instead of TIF_FREEZE
2011-10-31 19:05 ` [PATCH 14/17] freezer: make freezing() test freeze conditions in effect instead " Tejun Heo
@ 2011-10-31 22:34 ` Tejun Heo
0 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:34 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc
Using TIF_FREEZE for freezing worked when there was only single
freezing condition (the PM one); however, now there is also the
cgroup_freezer and single bit flag is getting clumsy.
thaw_processes() is already testing whether cgroup freezing in in
effect to avoid thawing tasks which were frozen by both PM and cgroup
freezers.
This is racy (nothing prevents race against cgroup freezing) and
fragile. A much simpler way is to test actual freeze conditions from
freezing() - ie. directly test whether PM or cgroup freezing is in
effect.
This patch adds variables to indicate whether and what type of
freezing conditions are in effect and reimplements freezing() such
that it directly tests whether any of the two freezing conditions is
active and the task should freeze. On fast path, freezing() is still
very cheap - it only tests system_freezing_cnt.
This makes the clumsy dancing aroung TIF_FREEZE unnecessary and
freeze/thaw operations more usual - updating state variables for the
new state and nudging target tasks so that they notice the new state
and comply. As long as the nudging happens after state update, it's
race-free.
* This allows use of freezing() in freeze_task(). Replace the open
coded tests with freezing().
* p != current test is added to warning printing conditions in
try_to_freeze_tasks() failure path. This is necessary as freezing()
is now true for the task which initiated freezing too.
-v2: Oleg pointed out that re-freezing FROZEN cgroup could increment
system_freezing_cnt. Fixed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Paul Menage <paul@paulmenage.org> (for the cgroup portions)
---
Later fix patched merged. Please read reply to the head message for
details. Thanks.
include/linux/freezer.h | 33 +++++++++----------------
kernel/cgroup_freezer.c | 10 +++++++-
kernel/fork.c | 1 -
kernel/freezer.c | 62 ++++++++++++++++++++++++++++++----------------
kernel/power/process.c | 15 ++++++++---
5 files changed, 72 insertions(+), 49 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 243ec0b..6152e46 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -5,8 +5,13 @@
#include <linux/sched.h>
#include <linux/wait.h>
+#include <linux/atomic.h>
#ifdef CONFIG_FREEZER
+extern atomic_t system_freezing_cnt; /* nr of freezing conds in effect */
+extern bool pm_freezing; /* PM freezing in effect */
+extern bool pm_nosig_freezing; /* PM nosig freezing in effect */
+
/*
* Check if a process has been frozen
*/
@@ -15,28 +20,16 @@ static inline int frozen(struct task_struct *p)
return p->flags & PF_FROZEN;
}
-/*
- * Check if there is a request to freeze a process
- */
-static inline int freezing(struct task_struct *p)
-{
- return test_tsk_thread_flag(p, TIF_FREEZE);
-}
+extern bool freezing_slow_path(struct task_struct *p);
/*
- * Request that a process be frozen
- */
-static inline void set_freeze_flag(struct task_struct *p)
-{
- set_tsk_thread_flag(p, TIF_FREEZE);
-}
-
-/*
- * Sometimes we may need to cancel the previous 'freeze' request
+ * Check if there is a request to freeze a process
*/
-static inline void clear_freeze_flag(struct task_struct *p)
+static inline bool freezing(struct task_struct *p)
{
- clear_tsk_thread_flag(p, TIF_FREEZE);
+ if (likely(!atomic_read(&system_freezing_cnt)))
+ return false;
+ return freezing_slow_path(p);
}
static inline bool should_send_signal(struct task_struct *p)
@@ -179,9 +172,7 @@ static inline void set_freezable_with_signal(void)
})
#else /* !CONFIG_FREEZER */
static inline int frozen(struct task_struct *p) { return 0; }
-static inline int freezing(struct task_struct *p) { return 0; }
-static inline void set_freeze_flag(struct task_struct *p) {}
-static inline void clear_freeze_flag(struct task_struct *p) {}
+static inline bool freezing(struct task_struct *p) { return false; }
static inline bool __refrigerator(bool check_kthr_stop) { return false; }
static inline int freeze_processes(void) { return -ENOSYS; }
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index c72ec7e..5f45423 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -145,7 +145,11 @@ static struct cgroup_subsys_state *freezer_create(struct cgroup_subsys *ss,
static void freezer_destroy(struct cgroup_subsys *ss,
struct cgroup *cgroup)
{
- kfree(cgroup_freezer(cgroup));
+ struct freezer *freezer = cgroup_freezer(cgroup);
+
+ if (freezer->state != CGROUP_THAWED)
+ atomic_dec(&system_freezing_cnt);
+ kfree(freezer);
}
/*
@@ -307,10 +311,14 @@ static int freezer_change_state(struct cgroup *cgroup,
switch (goal_state) {
case CGROUP_THAWED:
+ if (freezer->state != CGROUP_THAWED)
+ atomic_dec(&system_freezing_cnt);
freezer->state = CGROUP_THAWED;
unfreeze_cgroup(cgroup, freezer);
break;
case CGROUP_FROZEN:
+ if (freezer->state == CGROUP_THAWED)
+ atomic_inc(&system_freezing_cnt);
freezer->state = CGROUP_FREEZING;
retval = try_to_freeze_cgroup(cgroup, freezer);
break;
diff --git a/kernel/fork.c b/kernel/fork.c
index 8e6b6f4..fa7beb3 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1000,7 +1000,6 @@ static void copy_flags(unsigned long clone_flags, struct task_struct *p)
new_flags |= PF_FORKNOEXEC;
new_flags |= PF_STARTING;
p->flags = new_flags;
- clear_freeze_flag(p);
}
SYSCALL_DEFINE1(set_tid_address, int __user *, tidptr)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 7a9f706..a797f41 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -11,9 +11,41 @@
#include <linux/freezer.h>
#include <linux/kthread.h>
+/* total number of freezing conditions in effect */
+atomic_t system_freezing_cnt = ATOMIC_INIT(0);
+EXPORT_SYMBOL(system_freezing_cnt);
+
+/* indicate whether PM freezing is in effect, protected by pm_mutex */
+bool pm_freezing;
+bool pm_nosig_freezing;
+
/* protects freezing and frozen transitions */
static DEFINE_SPINLOCK(freezer_lock);
+/**
+ * freezing_slow_path - slow path for testing whether a task needs to be frozen
+ * @p: task to be tested
+ *
+ * This function is called by freezing() if system_freezing_cnt isn't zero
+ * and tests whether @p needs to enter and stay in frozen state. Can be
+ * called under any context. The freezers are responsible for ensuring the
+ * target tasks see the updated state.
+ */
+bool freezing_slow_path(struct task_struct *p)
+{
+ if (p->flags & PF_NOFREEZE)
+ return false;
+
+ if (pm_nosig_freezing || cgroup_freezing(p))
+ return true;
+
+ if (pm_freezing && !(p->flags & PF_FREEZER_NOSIG))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(freezing_slow_path);
+
/* Refrigerator is place where frozen processes are stored :-). */
bool __refrigerator(bool check_kthr_stop)
{
@@ -23,17 +55,11 @@ bool __refrigerator(bool check_kthr_stop)
long save;
/*
- * Enter FROZEN. If NOFREEZE, schedule immediate thawing by
- * clearing freezing.
+ * No point in checking freezing() again - the caller already did.
+ * Proceed to enter FROZEN.
*/
spin_lock_irq(&freezer_lock);
repeat:
- if (!freezing(current)) {
- spin_unlock_irq(&freezer_lock);
- return was_frozen;
- }
- if (current->flags & PF_NOFREEZE)
- clear_freeze_flag(current);
current->flags |= PF_FROZEN;
spin_unlock_irq(&freezer_lock);
@@ -99,18 +125,12 @@ static void fake_signal_wake_up(struct task_struct *p)
bool freeze_task(struct task_struct *p, bool sig_only)
{
unsigned long flags;
- bool ret = false;
spin_lock_irqsave(&freezer_lock, flags);
-
- if ((p->flags & PF_NOFREEZE) ||
- (sig_only && !should_send_signal(p)))
- goto out_unlock;
-
- if (frozen(p))
- goto out_unlock;
-
- set_freeze_flag(p);
+ if (!freezing(p) || frozen(p)) {
+ spin_unlock_irqrestore(&freezer_lock, flags);
+ return false;
+ }
if (should_send_signal(p)) {
fake_signal_wake_up(p);
@@ -123,10 +143,9 @@ bool freeze_task(struct task_struct *p, bool sig_only)
} else {
wake_up_state(p, TASK_INTERRUPTIBLE);
}
- ret = true;
-out_unlock:
+
spin_unlock_irqrestore(&freezer_lock, flags);
- return ret;
+ return true;
}
void __thaw_task(struct task_struct *p)
@@ -143,7 +162,6 @@ void __thaw_task(struct task_struct *p)
* avoid leaving dangling TIF_SIGPENDING behind.
*/
spin_lock_irqsave(&freezer_lock, flags);
- clear_freeze_flag(p);
if (frozen(p)) {
wake_up_process(p);
} else {
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 54ba145..eed8995 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -101,7 +101,7 @@ static int try_to_freeze_tasks(bool sig_only)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
if (!wakeup && !freezer_should_skip(p) &&
- freezing(p) && !frozen(p))
+ p != current && freezing(p) && !frozen(p))
sched_show_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
@@ -120,7 +120,11 @@ int freeze_processes(void)
{
int error;
+ if (!pm_freezing)
+ atomic_inc(&system_freezing_cnt);
+
printk("Freezing user space processes ... ");
+ pm_freezing = true;
error = try_to_freeze_tasks(true);
if (!error) {
printk("done.");
@@ -142,6 +146,7 @@ int freeze_kernel_threads(void)
int error;
printk("Freezing remaining freezable tasks ... ");
+ pm_nosig_freezing = true;
error = try_to_freeze_tasks(false);
if (!error)
printk("done.");
@@ -158,6 +163,11 @@ void thaw_processes(void)
{
struct task_struct *g, *p;
+ if (pm_freezing)
+ atomic_dec(&system_freezing_cnt);
+ pm_freezing = false;
+ pm_nosig_freezing = false;
+
oom_killer_enable();
printk("Restarting tasks ... ");
@@ -166,9 +176,6 @@ void thaw_processes(void)
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (cgroup_freezing(p))
- continue;
-
__thaw_task(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 15/17] freezer: remove now unused TIF_FREEZE
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (13 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 14/17] freezer: make freezing() test freeze conditions in effect instead " Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 16/17] freezer: remove should_send_signal() and update frozen() Tejun Heo
` (5 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm
Cc: arnd, oleg, matthltc, Tejun Heo, linux-arch
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
---
arch/alpha/include/asm/thread_info.h | 2 --
arch/arm/include/asm/thread_info.h | 2 --
arch/avr32/include/asm/thread_info.h | 2 --
arch/blackfin/include/asm/thread_info.h | 2 --
arch/cris/include/asm/thread_info.h | 2 --
arch/frv/include/asm/thread_info.h | 2 --
arch/h8300/include/asm/thread_info.h | 2 --
arch/ia64/include/asm/thread_info.h | 2 --
arch/m32r/include/asm/thread_info.h | 2 --
arch/m68k/include/asm/thread_info.h | 1 -
arch/microblaze/include/asm/thread_info.h | 2 --
arch/mips/include/asm/thread_info.h | 2 --
arch/mn10300/include/asm/thread_info.h | 2 --
arch/parisc/include/asm/thread_info.h | 2 --
arch/powerpc/include/asm/thread_info.h | 2 --
arch/s390/include/asm/thread_info.h | 2 --
arch/sh/include/asm/thread_info.h | 2 --
arch/sparc/include/asm/thread_info_32.h | 2 --
arch/sparc/include/asm/thread_info_64.h | 2 --
arch/um/include/asm/thread_info.h | 2 --
arch/unicore32/include/asm/thread_info.h | 2 --
arch/x86/include/asm/thread_info.h | 2 --
arch/xtensa/include/asm/thread_info.h | 2 --
23 files changed, 0 insertions(+), 45 deletions(-)
diff --git a/arch/alpha/include/asm/thread_info.h b/arch/alpha/include/asm/thread_info.h
index ff73db0..28335bd 100644
--- a/arch/alpha/include/asm/thread_info.h
+++ b/arch/alpha/include/asm/thread_info.h
@@ -79,7 +79,6 @@ register struct thread_info *__current_thread_info __asm__("$8");
#define TIF_UAC_SIGBUS 12 /* ! userspace part of 'osf_sysinfo' */
#define TIF_MEMDIE 13 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 14 /* restore signal mask in do_signal */
-#define TIF_FREEZE 16 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1<<TIF_SIGPENDING)
@@ -87,7 +86,6 @@ register struct thread_info *__current_thread_info __asm__("$8");
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
/* Work to do on interrupt/exception return. */
#define _TIF_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index 7b5cc8d..0f30c3a 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -142,7 +142,6 @@ extern void vfp_flush_hwstate(struct thread_info *);
#define TIF_POLLING_NRFLAG 16
#define TIF_USING_IWMMXT 17
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
-#define TIF_FREEZE 19
#define TIF_RESTORE_SIGMASK 20
#define TIF_SECCOMP 21
@@ -152,7 +151,6 @@ extern void vfp_flush_hwstate(struct thread_info *);
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
#define _TIF_USING_IWMMXT (1 << TIF_USING_IWMMXT)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
diff --git a/arch/avr32/include/asm/thread_info.h b/arch/avr32/include/asm/thread_info.h
index 7a9c03d..e5deda4 100644
--- a/arch/avr32/include/asm/thread_info.h
+++ b/arch/avr32/include/asm/thread_info.h
@@ -85,7 +85,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_RESTORE_SIGMASK 7 /* restore signal mask in do_signal */
#define TIF_CPU_GOING_TO_SLEEP 8 /* CPU is entering sleep 0 mode */
#define TIF_NOTIFY_RESUME 9 /* callback before returning to user */
-#define TIF_FREEZE 29
#define TIF_DEBUG 30 /* debugging enabled */
#define TIF_USERSPACE 31 /* true if FS sets userspace */
@@ -98,7 +97,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
#define _TIF_CPU_GOING_TO_SLEEP (1 << TIF_CPU_GOING_TO_SLEEP)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
/* Note: The masks below must never span more than 16 bits! */
diff --git a/arch/blackfin/include/asm/thread_info.h b/arch/blackfin/include/asm/thread_info.h
index 02560fd..53ad100 100644
--- a/arch/blackfin/include/asm/thread_info.h
+++ b/arch/blackfin/include/asm/thread_info.h
@@ -100,7 +100,6 @@ static inline struct thread_info *current_thread_info(void)
TIF_NEED_RESCHED */
#define TIF_MEMDIE 4 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 5 /* restore signal mask in do_signal() */
-#define TIF_FREEZE 6 /* is freezing for suspend */
#define TIF_IRQ_SYNC 7 /* sync pipeline stage */
#define TIF_NOTIFY_RESUME 8 /* callback before returning to user */
#define TIF_SINGLESTEP 9
@@ -111,7 +110,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_IRQ_SYNC (1<<TIF_IRQ_SYNC)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
#define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP)
diff --git a/arch/cris/include/asm/thread_info.h b/arch/cris/include/asm/thread_info.h
index 332f19c..29b9288 100644
--- a/arch/cris/include/asm/thread_info.h
+++ b/arch/cris/include/asm/thread_info.h
@@ -86,7 +86,6 @@ struct thread_info {
#define TIF_RESTORE_SIGMASK 9 /* restore signal mask in do_signal() */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 17 /* is terminating due to OOM killer */
-#define TIF_FREEZE 18 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
@@ -94,7 +93,6 @@ struct thread_info {
#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
#define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
diff --git a/arch/frv/include/asm/thread_info.h b/arch/frv/include/asm/thread_info.h
index cefbe73..92d83ea 100644
--- a/arch/frv/include/asm/thread_info.h
+++ b/arch/frv/include/asm/thread_info.h
@@ -111,7 +111,6 @@ register struct thread_info *__current_thread_info asm("gr15");
#define TIF_RESTORE_SIGMASK 5 /* restore signal mask in do_signal() */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 17 /* is terminating due to OOM killer */
-#define TIF_FREEZE 18 /* freezing for suspend */
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
@@ -120,7 +119,6 @@ register struct thread_info *__current_thread_info asm("gr15");
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
#define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
diff --git a/arch/h8300/include/asm/thread_info.h b/arch/h8300/include/asm/thread_info.h
index d6f1784..9c126e0 100644
--- a/arch/h8300/include/asm/thread_info.h
+++ b/arch/h8300/include/asm/thread_info.h
@@ -90,7 +90,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_MEMDIE 4 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 5 /* restore signal mask in do_signal() */
#define TIF_NOTIFY_RESUME 6 /* callback before returning to user */
-#define TIF_FREEZE 16 /* is freezing for suspend */
/* as above, but as bit values */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
@@ -99,7 +98,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
diff --git a/arch/ia64/include/asm/thread_info.h b/arch/ia64/include/asm/thread_info.h
index ff0cc84..e054bcc 100644
--- a/arch/ia64/include/asm/thread_info.h
+++ b/arch/ia64/include/asm/thread_info.h
@@ -113,7 +113,6 @@ struct thread_info {
#define TIF_MEMDIE 17 /* is terminating due to OOM killer */
#define TIF_MCA_INIT 18 /* this task is processing MCA or INIT */
#define TIF_DB_DISABLED 19 /* debug trap disabled for fsyscall */
-#define TIF_FREEZE 20 /* is freezing for suspend */
#define TIF_RESTORE_RSE 21 /* user RBS is newer than kernel RBS */
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
@@ -126,7 +125,6 @@ struct thread_info {
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
#define _TIF_MCA_INIT (1 << TIF_MCA_INIT)
#define _TIF_DB_DISABLED (1 << TIF_DB_DISABLED)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_RESTORE_RSE (1 << TIF_RESTORE_RSE)
/* "work to do on user-return" bits */
diff --git a/arch/m32r/include/asm/thread_info.h b/arch/m32r/include/asm/thread_info.h
index 0227dba..bf8fa3c 100644
--- a/arch/m32r/include/asm/thread_info.h
+++ b/arch/m32r/include/asm/thread_info.h
@@ -138,7 +138,6 @@ static inline unsigned int get_thread_fault_code(void)
#define TIF_USEDFPU 16 /* FPU was used by this task this quantum (SMP) */
#define TIF_POLLING_NRFLAG 17 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
-#define TIF_FREEZE 19 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1<<TIF_SIGPENDING)
@@ -149,7 +148,6 @@ static inline unsigned int get_thread_fault_code(void)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_USEDFPU (1<<TIF_USEDFPU)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
#define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
diff --git a/arch/m68k/include/asm/thread_info.h b/arch/m68k/include/asm/thread_info.h
index 7909889..294df15 100644
--- a/arch/m68k/include/asm/thread_info.h
+++ b/arch/m68k/include/asm/thread_info.h
@@ -103,7 +103,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_DELAYED_TRACE 14 /* single step a syscall */
#define TIF_SYSCALL_TRACE 15 /* syscall trace active */
#define TIF_MEMDIE 16 /* is terminating due to OOM killer */
-#define TIF_FREEZE 17 /* thread is freezing for suspend */
#define TIF_RESTORE_SIGMASK 18 /* restore signal mask in do_signal */
#endif /* _ASM_M68K_THREAD_INFO_H */
diff --git a/arch/microblaze/include/asm/thread_info.h b/arch/microblaze/include/asm/thread_info.h
index b73da2a..1a8ab6a 100644
--- a/arch/microblaze/include/asm/thread_info.h
+++ b/arch/microblaze/include/asm/thread_info.h
@@ -125,7 +125,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_MEMDIE 6 /* is terminating due to OOM killer */
#define TIF_SYSCALL_AUDIT 9 /* syscall auditing active */
#define TIF_SECCOMP 10 /* secure computing */
-#define TIF_FREEZE 14 /* Freezing for suspend */
/* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_POLLING_NRFLAG 16
@@ -137,7 +136,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_IRET (1 << TIF_IRET)
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
diff --git a/arch/mips/include/asm/thread_info.h b/arch/mips/include/asm/thread_info.h
index 97f8bf6..0d85d8e 100644
--- a/arch/mips/include/asm/thread_info.h
+++ b/arch/mips/include/asm/thread_info.h
@@ -117,7 +117,6 @@ register struct thread_info *__current_thread_info __asm__("$28");
#define TIF_USEDFPU 16 /* FPU was used by this task this quantum (SMP) */
#define TIF_POLLING_NRFLAG 17 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
-#define TIF_FREEZE 19
#define TIF_FIXADE 20 /* Fix address errors in software */
#define TIF_LOGADE 21 /* Log address errors to syslog */
#define TIF_32BIT_REGS 22 /* also implies 16/32 fprs */
@@ -141,7 +140,6 @@ register struct thread_info *__current_thread_info __asm__("$28");
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_USEDFPU (1<<TIF_USEDFPU)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_FIXADE (1<<TIF_FIXADE)
#define _TIF_LOGADE (1<<TIF_LOGADE)
#define _TIF_32BIT_REGS (1<<TIF_32BIT_REGS)
diff --git a/arch/mn10300/include/asm/thread_info.h b/arch/mn10300/include/asm/thread_info.h
index 87c2130..28cf521 100644
--- a/arch/mn10300/include/asm/thread_info.h
+++ b/arch/mn10300/include/asm/thread_info.h
@@ -165,7 +165,6 @@ extern void free_thread_info(struct thread_info *);
#define TIF_RESTORE_SIGMASK 5 /* restore signal mask in do_signal() */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 17 /* is terminating due to OOM killer */
-#define TIF_FREEZE 18 /* freezing for suspend */
#define _TIF_SYSCALL_TRACE +(1 << TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME +(1 << TIF_NOTIFY_RESUME)
@@ -174,7 +173,6 @@ extern void free_thread_info(struct thread_info *);
#define _TIF_SINGLESTEP +(1 << TIF_SINGLESTEP)
#define _TIF_RESTORE_SIGMASK +(1 << TIF_RESTORE_SIGMASK)
#define _TIF_POLLING_NRFLAG +(1 << TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE +(1 << TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
#define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h
index aa8de72..6d9c7c7 100644
--- a/arch/parisc/include/asm/thread_info.h
+++ b/arch/parisc/include/asm/thread_info.h
@@ -58,7 +58,6 @@ struct thread_info {
#define TIF_32BIT 4 /* 32 bit binary */
#define TIF_MEMDIE 5 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 6 /* restore saved signal mask */
-#define TIF_FREEZE 7 /* is freezing for suspend */
#define TIF_NOTIFY_RESUME 8 /* callback before returning to user */
#define TIF_SINGLESTEP 9 /* single stepping? */
#define TIF_BLOCKSTEP 10 /* branch stepping? */
@@ -69,7 +68,6 @@ struct thread_info {
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
#define _TIF_32BIT (1 << TIF_32BIT)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP)
#define _TIF_BLOCKSTEP (1 << TIF_BLOCKSTEP)
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 836f231..96471494 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -109,7 +109,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_RESTOREALL 11 /* Restore all regs (implies NOERROR) */
#define TIF_NOERROR 12 /* Force successful syscall return */
#define TIF_NOTIFY_RESUME 13 /* callback before returning to user */
-#define TIF_FREEZE 14 /* Freezing for suspend */
#define TIF_SYSCALL_TRACEPOINT 15 /* syscall tracepoint instrumentation */
#define TIF_RUNLATCH 16 /* Is the runlatch enabled? */
@@ -127,7 +126,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_RESTOREALL (1<<TIF_RESTOREALL)
#define _TIF_NOERROR (1<<TIF_NOERROR)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)
#define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
#define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
diff --git a/arch/s390/include/asm/thread_info.h b/arch/s390/include/asm/thread_info.h
index 1a5dbb6..589708a 100644
--- a/arch/s390/include/asm/thread_info.h
+++ b/arch/s390/include/asm/thread_info.h
@@ -101,7 +101,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 19 /* restore signal mask in do_signal() */
#define TIF_SINGLE_STEP 20 /* This task is single stepped */
-#define TIF_FREEZE 21 /* thread is freezing for suspend */
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
@@ -118,7 +117,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_31BIT (1<<TIF_31BIT)
#define _TIF_SINGLE_STEP (1<<TIF_FREEZE)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#ifdef CONFIG_64BIT
#define is_32bit_task() (test_thread_flag(TIF_31BIT))
diff --git a/arch/sh/include/asm/thread_info.h b/arch/sh/include/asm/thread_info.h
index ea2d508..20ee40a 100644
--- a/arch/sh/include/asm/thread_info.h
+++ b/arch/sh/include/asm/thread_info.h
@@ -122,7 +122,6 @@ extern void init_thread_xstate(void);
#define TIF_SYSCALL_TRACEPOINT 8 /* for ftrace syscall instrumentation */
#define TIF_POLLING_NRFLAG 17 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
-#define TIF_FREEZE 19 /* Freezing for suspend */
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
@@ -133,7 +132,6 @@ extern void init_thread_xstate(void);
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
/*
* _TIF_ALLWORK_MASK and _TIF_WORK_MASK need to fit within 2 bytes, or we
diff --git a/arch/sparc/include/asm/thread_info_32.h b/arch/sparc/include/asm/thread_info_32.h
index fa57532..5cc5888 100644
--- a/arch/sparc/include/asm/thread_info_32.h
+++ b/arch/sparc/include/asm/thread_info_32.h
@@ -133,7 +133,6 @@ BTFIXUPDEF_CALL(void, free_thread_info, struct thread_info *)
#define TIF_POLLING_NRFLAG 9 /* true if poll_idle() is polling
* TIF_NEED_RESCHED */
#define TIF_MEMDIE 10 /* is terminating due to OOM killer */
-#define TIF_FREEZE 11 /* is freezing for suspend */
/* as above, but as bit values */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
@@ -147,7 +146,6 @@ BTFIXUPDEF_CALL(void, free_thread_info, struct thread_info *)
#define _TIF_DO_NOTIFY_RESUME_MASK (_TIF_NOTIFY_RESUME | \
_TIF_SIGPENDING | \
_TIF_RESTORE_SIGMASK)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#endif /* __KERNEL__ */
diff --git a/arch/sparc/include/asm/thread_info_64.h b/arch/sparc/include/asm/thread_info_64.h
index 60d86be..01d057f 100644
--- a/arch/sparc/include/asm/thread_info_64.h
+++ b/arch/sparc/include/asm/thread_info_64.h
@@ -225,7 +225,6 @@ register struct thread_info *current_thread_info_reg asm("g6");
/* flag bit 12 is available */
#define TIF_MEMDIE 13 /* is terminating due to OOM killer */
#define TIF_POLLING_NRFLAG 14
-#define TIF_FREEZE 15 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME)
@@ -237,7 +236,6 @@ register struct thread_info *current_thread_info_reg asm("g6");
#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
#define _TIF_SYSCALL_TRACEPOINT (1<<TIF_SYSCALL_TRACEPOINT)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_USER_WORK_MASK ((0xff << TI_FLAG_WSAVED_SHIFT) | \
_TIF_DO_NOTIFY_RESUME_MASK | \
diff --git a/arch/um/include/asm/thread_info.h b/arch/um/include/asm/thread_info.h
index 5bd1bad..200c4ab 100644
--- a/arch/um/include/asm/thread_info.h
+++ b/arch/um/include/asm/thread_info.h
@@ -71,7 +71,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_MEMDIE 5 /* is terminating due to OOM killer */
#define TIF_SYSCALL_AUDIT 6
#define TIF_RESTORE_SIGMASK 7
-#define TIF_FREEZE 16 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
@@ -80,6 +79,5 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_MEMDIE (1 << TIF_MEMDIE)
#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#endif
diff --git a/arch/unicore32/include/asm/thread_info.h b/arch/unicore32/include/asm/thread_info.h
index c270e9e..89f7557 100644
--- a/arch/unicore32/include/asm/thread_info.h
+++ b/arch/unicore32/include/asm/thread_info.h
@@ -135,14 +135,12 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
#define TIF_MEMDIE 18
-#define TIF_FREEZE 19
#define TIF_RESTORE_SIGMASK 20
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
/*
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index a1fe5c1..32125af 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -90,7 +90,6 @@ struct thread_info {
#define TIF_MEMDIE 20 /* is terminating due to OOM killer */
#define TIF_DEBUG 21 /* uses debug registers */
#define TIF_IO_BITMAP 22 /* uses I/O bitmap */
-#define TIF_FREEZE 23 /* is freezing for suspend */
#define TIF_FORCED_TF 24 /* true if TF in eflags artificially */
#define TIF_BLOCKSTEP 25 /* set when we want DEBUGCTLMSR_BTF */
#define TIF_LAZY_MMU_UPDATES 27 /* task is updating the mmu lazily */
@@ -112,7 +111,6 @@ struct thread_info {
#define _TIF_FORK (1 << TIF_FORK)
#define _TIF_DEBUG (1 << TIF_DEBUG)
#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)
-#define _TIF_FREEZE (1 << TIF_FREEZE)
#define _TIF_FORCED_TF (1 << TIF_FORCED_TF)
#define _TIF_BLOCKSTEP (1 << TIF_BLOCKSTEP)
#define _TIF_LAZY_MMU_UPDATES (1 << TIF_LAZY_MMU_UPDATES)
diff --git a/arch/xtensa/include/asm/thread_info.h b/arch/xtensa/include/asm/thread_info.h
index 7be8acc..6abbedd 100644
--- a/arch/xtensa/include/asm/thread_info.h
+++ b/arch/xtensa/include/asm/thread_info.h
@@ -132,7 +132,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_MEMDIE 5 /* is terminating due to OOM killer */
#define TIF_RESTORE_SIGMASK 6 /* restore signal mask in do_signal() */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
-#define TIF_FREEZE 17 /* is freezing for suspend */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1<<TIF_SIGPENDING)
@@ -141,7 +140,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_IRET (1<<TIF_IRET)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
-#define _TIF_FREEZE (1<<TIF_FREEZE)
#define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */
#define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 16/17] freezer: remove should_send_signal() and update frozen()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (14 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 15/17] freezer: remove now unused TIF_FREEZE Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 19:05 ` [PATCH 17/17] freezer: fix set_freezable[_with_signal]() race Tejun Heo
` (4 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
should_send_signal() is only used in freezer.c. Exporting them only
increases chance of abuse. Open code the two users and remove it.
Update frozen() to return bool.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
include/linux/freezer.h | 9 ++-------
kernel/freezer.c | 2 +-
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 6152e46..3dd2110 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -15,7 +15,7 @@ extern bool pm_nosig_freezing; /* PM nosig freezing in effect */
/*
* Check if a process has been frozen
*/
-static inline int frozen(struct task_struct *p)
+static inline bool frozen(struct task_struct *p)
{
return p->flags & PF_FROZEN;
}
@@ -32,11 +32,6 @@ static inline bool freezing(struct task_struct *p)
return freezing_slow_path(p);
}
-static inline bool should_send_signal(struct task_struct *p)
-{
- return !(p->flags & PF_FREEZER_NOSIG);
-}
-
/* Takes and releases task alloc lock using task_lock() */
extern void __thaw_task(struct task_struct *t);
@@ -171,7 +166,7 @@ static inline void set_freezable_with_signal(void)
__retval; \
})
#else /* !CONFIG_FREEZER */
-static inline int frozen(struct task_struct *p) { return 0; }
+static inline bool frozen(struct task_struct *p) { return false; }
static inline bool freezing(struct task_struct *p) { return false; }
static inline bool __refrigerator(bool check_kthr_stop) { return false; }
diff --git a/kernel/freezer.c b/kernel/freezer.c
index a797f41..c89608d 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -132,7 +132,7 @@ bool freeze_task(struct task_struct *p, bool sig_only)
return false;
}
- if (should_send_signal(p)) {
+ if (!(p->flags & PF_FREEZER_NOSIG)) {
fake_signal_wake_up(p);
/*
* fake_signal_wake_up() goes through p's scheduler
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 17/17] freezer: fix set_freezable[_with_signal]() race
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (15 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 16/17] freezer: remove should_send_signal() and update frozen() Tejun Heo
@ 2011-10-31 19:05 ` Tejun Heo
2011-10-31 22:34 ` [PATCH 18/17] freezer: restructure __refrigerator() Tejun Heo
` (3 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 19:05 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc, Tejun Heo
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 merrily proceeding to execute code paths which aren't
supposed to be executing during PM freeze.
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. This eliminates race condition
against freezing.
Note: Separated out from larger patch to resolve fix order dependency
Oleg pointed out.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
include/linux/freezer.h | 9 +++++----
kernel/freezer.c | 25 +++++++++++++++++++++++++
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 3dd2110..ac0513e 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -49,6 +49,7 @@ static inline bool try_to_freeze(void)
}
extern bool freeze_task(struct task_struct *p, bool sig_only);
+extern bool __set_freezable(bool with_signal);
#ifdef CONFIG_CGROUP_FREEZER
extern bool cgroup_freezing(struct task_struct *task);
@@ -106,18 +107,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 c89608d..a95250c 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -171,3 +171,28 @@ void __thaw_task(struct task_struct *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);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 18/17] freezer: restructure __refrigerator()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (16 preceding siblings ...)
2011-10-31 19:05 ` [PATCH 17/17] freezer: fix set_freezable[_with_signal]() race Tejun Heo
@ 2011-10-31 22:34 ` Tejun Heo
2011-10-31 22:35 ` [PATCH 19/17] freezer: use lock_task_sighand() in fake_signal_wake_up() Tejun Heo
` (2 subsequent siblings)
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:34 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc
If another freeze happens before all tasks leave FROZEN state after
being thawed, the freezer can see the existing FROZEN and consider the
tasks to be frozen but they can clear FROZEN without checking the new
freezing().
Oleg suggested restructuring __refrigerator() such that there's single
condition check section inside freezer_lock and sigpending is cleared
afterwards, which fixes the problem and simplifies the code.
Restructure accordingly.
-v2: Frozen loop exited without releasing freezer_lock. Fixed.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
kernel/freezer.c | 32 +++++++++++---------------------
1 files changed, 11 insertions(+), 21 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index a95250c..10f9840 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -52,39 +52,29 @@ bool __refrigerator(bool check_kthr_stop)
/* Hmm, should we be allowed to suspend when there are realtime
processes around? */
bool was_frozen = false;
- long save;
+ long save = current->state;
- /*
- * No point in checking freezing() again - the caller already did.
- * Proceed to enter FROZEN.
- */
- spin_lock_irq(&freezer_lock);
-repeat:
- current->flags |= PF_FROZEN;
- spin_unlock_irq(&freezer_lock);
-
- save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
- spin_lock_irq(¤t->sighand->siglock);
- recalc_sigpending(); /* We sent fake signal, clean it up */
- spin_unlock_irq(¤t->sighand->siglock);
-
for (;;) {
set_current_state(TASK_UNINTERRUPTIBLE);
+
+ spin_lock_irq(&freezer_lock);
+ current->flags |= PF_FROZEN;
if (!freezing(current) ||
(check_kthr_stop && kthread_should_stop()))
+ current->flags &= ~PF_FROZEN;
+ spin_unlock_irq(&freezer_lock);
+
+ if (!(current->flags & PF_FROZEN))
break;
was_frozen = true;
schedule();
}
- /* leave FROZEN */
- spin_lock_irq(&freezer_lock);
- if (freezing(current))
- goto repeat;
- current->flags &= ~PF_FROZEN;
- spin_unlock_irq(&freezer_lock);
+ spin_lock_irq(¤t->sighand->siglock);
+ recalc_sigpending(); /* We sent fake signal, clean it up */
+ spin_unlock_irq(¤t->sighand->siglock);
pr_debug("%s left refrigerator\n", current->comm);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 19/17] freezer: use lock_task_sighand() in fake_signal_wake_up()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (17 preceding siblings ...)
2011-10-31 22:34 ` [PATCH 18/17] freezer: restructure __refrigerator() Tejun Heo
@ 2011-10-31 22:35 ` 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
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:35 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc
cgroup_freezer calls freeze_task() without holding tasklist_lock and,
if the task is exiting, its ->sighand may be gone by the time
fake_signal_wake_up() is called. Use lock_task_sighand() instead of
accessing ->sighand directly.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Paul Menage <paul@paulmenage.org>
---
kernel/freezer.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/freezer.c b/kernel/freezer.c
index 10f9840..a9c18c7 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -93,9 +93,10 @@ static void fake_signal_wake_up(struct task_struct *p)
{
unsigned long flags;
- spin_lock_irqsave(&p->sighand->siglock, flags);
- signal_wake_up(p, 1);
- spin_unlock_irqrestore(&p->sighand->siglock, flags);
+ if (lock_task_sighand(p, &flags)) {
+ signal_wake_up(p, 1);
+ unlock_task_sighand(p, &flags);
+ }
}
/**
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH UPDATED 20/17] freezer: remove unused @sig_only from freeze_task()
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (18 preceding siblings ...)
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 ` Tejun Heo
2011-10-31 22:40 ` [PATCHSET UPDATED pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:35 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc
After "freezer: make freezing() test freeze conditions in effect
instead of TIF_FREEZE", freezing() returns authoritative answer on
whether the current task should freeze or not and freeze_task()
doesn't need or use @sig_only. Remove it.
While at it, rewrite function comment for freeze_task() and rename
@sig_only to @user_only in try_to_freeze_tasks().
This patch doesn't cause any functional change.
Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Oleg Nesterov <oleg@redhat.com>
---
include/linux/freezer.h | 2 +-
kernel/cgroup_freezer.c | 4 ++--
kernel/freezer.c | 21 +++++++++------------
kernel/power/process.c | 8 ++++----
4 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index ac0513e..4b3dc4c 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -48,7 +48,7 @@ static inline bool try_to_freeze(void)
return __refrigerator(false);
}
-extern bool freeze_task(struct task_struct *p, bool sig_only);
+extern bool freeze_task(struct task_struct *p);
extern bool __set_freezable(bool with_signal);
#ifdef CONFIG_CGROUP_FREEZER
diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
index 5f45423..d161958 100644
--- a/kernel/cgroup_freezer.c
+++ b/kernel/cgroup_freezer.c
@@ -206,7 +206,7 @@ static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
/* Locking avoids race with FREEZING -> THAWED transitions. */
if (freezer->state == CGROUP_FREEZING)
- freeze_task(task, true);
+ freeze_task(task);
spin_unlock_irq(&freezer->lock);
}
@@ -274,7 +274,7 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
cgroup_iter_start(cgroup, &it);
while ((task = cgroup_iter_next(cgroup, &it))) {
- if (!freeze_task(task, true))
+ if (!freeze_task(task))
continue;
if (frozen(task))
continue;
diff --git a/kernel/freezer.c b/kernel/freezer.c
index a9c18c7..0845321 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -100,20 +100,17 @@ static void fake_signal_wake_up(struct task_struct *p)
}
/**
- * freeze_task - send a freeze request to given task
- * @p: task to send the request to
- * @sig_only: if set, the request will only be sent if the task has the
- * PF_FREEZER_NOSIG flag unset
- * Return value: 'false', if @sig_only is set and the task has
- * PF_FREEZER_NOSIG set or the task is frozen, 'true', otherwise
+ * freeze_task - send a freeze request to given task
+ * @p: task to send the request to
*
- * The freeze request is sent by setting the tasks's TIF_FREEZE flag and
- * either sending a fake signal to it or waking it up, depending on whether
- * or not it has PF_FREEZER_NOSIG set. If @sig_only is set and the task
- * has PF_FREEZER_NOSIG set (ie. it is a typical kernel thread), its
- * TIF_FREEZE flag will not be set.
+ * If @p is freezing, the freeze request is sent by setting %TIF_FREEZE
+ * flag and either sending a fake signal to it or waking it up, depending
+ * on whether it has %PF_FREEZER_NOSIG set.
+ *
+ * RETURNS:
+ * %false, if @p is not freezing or already frozen; %true, otherwise
*/
-bool freeze_task(struct task_struct *p, bool sig_only)
+bool freeze_task(struct task_struct *p)
{
unsigned long flags;
diff --git a/kernel/power/process.c b/kernel/power/process.c
index eed8995..8bddc48 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -22,7 +22,7 @@
*/
#define TIMEOUT (20 * HZ)
-static int try_to_freeze_tasks(bool sig_only)
+static int try_to_freeze_tasks(bool user_only)
{
struct task_struct *g, *p;
unsigned long end_time;
@@ -37,14 +37,14 @@ static int try_to_freeze_tasks(bool sig_only)
end_time = jiffies + TIMEOUT;
- if (!sig_only)
+ if (!user_only)
freeze_workqueues_begin();
while (true) {
todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
- if (p == current || !freeze_task(p, sig_only))
+ if (p == current || !freeze_task(p))
continue;
/*
@@ -65,7 +65,7 @@ static int try_to_freeze_tasks(bool sig_only)
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
- if (!sig_only) {
+ if (!user_only) {
wq_busy = freeze_workqueues_busy();
todo += wq_busy;
}
--
1.7.3.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCHSET UPDATED pm] freezer: fix various bugs and simplify implementation, take#2
2011-10-31 19:05 [PATCHSET pm] freezer: fix various bugs and simplify implementation, take#2 Tejun Heo
` (19 preceding siblings ...)
2011-10-31 22:35 ` [PATCH UPDATED 20/17] freezer: remove unused @sig_only from freeze_task() Tejun Heo
@ 2011-10-31 22:40 ` Tejun Heo
20 siblings, 0 replies; 29+ messages in thread
From: Tejun Heo @ 2011-10-31 22:40 UTC (permalink / raw)
To: rjw, paul, linux-kernel, linux-pm; +Cc: arnd, oleg, matthltc
Hello,
On Mon, Oct 31, 2011 at 12:05:11PM -0700, Tejun Heo wrote:
> 0001-freezer-fix-current-state-restoration-race-in-refrig.patch
> 0002-freezer-don-t-unnecessarily-set-PF_NOFREEZE-explicit.patch
> 0003-freezer-unexport-refrigerator-and-update-try_to_free.patch
> 0004-freezer-implement-and-use-kthread_freezable_should_s.patch
> 0005-freezer-rename-thaw_process-to-__thaw_task-and-simpl.patch
> 0006-freezer-remove-racy-clear_freeze_flag-and-set-PF_NOF.patch
> 0007-freezer-don-t-distinguish-nosig-tasks-on-thaw.patch
> 0008-freezer-use-dedicated-lock-instead-of-task_lock-memo.patch
> 0009-freezer-make-freezing-indicate-freeze-condition-in-e.patch
> 0010-freezer-test-freezable-conditions-while-holding-free.patch
> 0011-freezer-kill-PF_FREEZING.patch
> 0012-freezer-clean-up-freeze_processes-failure-path.patch
> 0013-cgroup_freezer-prepare-for-removal-of-TIF_FREEZE.patch
> 0014-freezer-make-freezing-test-freeze-conditions-in-effe.patch
> 0015-freezer-remove-now-unused-TIF_FREEZE.patch
> 0016-freezer-remove-should_send_signal-and-update-frozen.patch
> 0017-freezer-fix-set_freezable-_with_signal-race.patch
I completely forgot about the second patchset[1] fixing problems of
the first one. The first two are folded into respective patches in
the series (0013 and 0014 updated). The following three patches have
been appended to the series.
0018-freezer-restructure-__refrigerator.patch
0019-freezer-use-lock_task_sighand-in-fake_signal_wake_up.patch
0020-freezer-remove-unused-sig_only-from-freeze_task.patch
There still are three more patches which got lost. I'll repost them
later.
The git branch has been updated to reflect the changes.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git pm-freezer
The new HEAD is efb69700c8 "freezer: remove unused @sig_only from
freeze_task()".
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.kernel/1187553
^ permalink raw reply [flat|nested] 29+ messages in thread