* [RFC][PATCH -mm 0/7] Freezer update (updated)
@ 2007-07-11 22:06 Rafael J. Wysocki
2007-07-11 22:08 ` [RFC][PATCH -mm 1/7] Freezer: Document relationship with memory shrinking Rafael J. Wysocki
` (6 more replies)
0 siblings, 7 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:06 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
Hi,
This is the second "release" of my latest freezer update. I have updated the
patches to take your comments into account (thanks a lot for them) and
changed their ordering.
The patches in these series update the freezer to eliminate some existing
shortcomings, so I'm still considering them as 2.6.23 material.
The patches do the following:
* update the freezer documentation to describe, previously omitted, important
reason for freezing tasks (ie. memory shrinking)
* remove sys_sync() from the freezer and make the suspend/hibernation code
invoke it explicitly
* prevent new tasks from inheriting TIF_FREEZE set from the parents
* introduce freezer-firendly wrappers around wait_event_interruptible()
and wait_event_interruptible_timeout()
* prevent the freezer from sending signals to kernel threads
* increase the verbosity of the freezer slightly
* make the freezer use the freezing timeout more efficiently
The details are in the changelogs.
Comments welcome.
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 1/7] Freezer: Document relationship with memory shrinking
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
@ 2007-07-11 22:08 ` Rafael J. Wysocki
2007-07-11 22:10 ` [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes Rafael J. Wysocki
` (5 subsequent siblings)
6 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:08 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
One important reason to freeze tasks, which is that we don't want them to
allocate memory after freeing it for the hibernation image, has not been
documented. Fix it.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
Documentation/power/freezing-of-tasks.txt | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
Index: linux-2.6.22-rc6-mm1/Documentation/power/freezing-of-tasks.txt
===================================================================
--- linux-2.6.22-rc6-mm1.orig/Documentation/power/freezing-of-tasks.txt 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/Documentation/power/freezing-of-tasks.txt 2007-07-11 20:50:24.000000000 +0200
@@ -81,7 +81,16 @@ hibernation image has been created and b
The majority of these are user space processes, but if any of the kernel threads
may cause something like this to happen, they have to be freezable.
-2. The second reason is to prevent user space processes and some kernel threads
+2. Next, to create the hibernation image we need to free a sufficient amount of
+memory (approximately 50% of available RAM) and we need to do that before
+devices are deactivated, because we generally need them for swapping out. Then,
+after the memory for the image has been freed, we don't want tasks to allocate
+additional memory and we prevent them from doing that by freezing them earlier.
+[Of course, this also means that device drivers should not allocate substantial
+amounts of memory from their .suspend() callbacks before hibernation, but this
+is e separate issue.]
+
+3. The third reason is to prevent user space processes and some kernel threads
from interfering with the suspending and resuming of devices. A user space
process running on a second CPU while we are suspending devices may, for
example, be troublesome and without the freezing of tasks we would need some
@@ -111,7 +120,7 @@ frozen before the driver's .suspend() ca
thawed after the driver's .resume() callback has run, so it won't be accessing
the device while it's suspended.
-3. Another reason for freezing tasks is to prevent user space processes from
+4. Another reason for freezing tasks is to prevent user space processes from
realizing that hibernation (or suspend) operation takes place. Ideally, user
space processes should not notice that such a system-wide operation has occurred
and should continue running without any problems after the restore (or resume
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
2007-07-11 22:08 ` [RFC][PATCH -mm 1/7] Freezer: Document relationship with memory shrinking Rafael J. Wysocki
@ 2007-07-11 22:10 ` Rafael J. Wysocki
2007-07-11 22:42 ` Pavel Machek
2007-07-11 22:12 ` [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set Rafael J. Wysocki
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:10 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
The syncing of filesystems from within the freezer is generally not needed.
Also, if there's an ext3 filesystem loopback-mounted from a FUSE one, the
syncing results in writes to it and deadlocks. Similarly, it will deadlock if
FUSE implements sync.
Change freeze_processes() so that it doesn't execute sys_sync() and make the
suspend and hibernation code paths sync filesystems independently of the
freezer.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/disk.c | 4 ++++
kernel/power/main.c | 6 ++++++
kernel/power/process.c | 1 -
kernel/power/user.c | 4 ++++
4 files changed, 14 insertions(+), 1 deletion(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/disk.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/disk.c 2007-07-11 20:48:05.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/disk.c 2007-07-11 20:48:23.000000000 +0200
@@ -297,6 +297,10 @@ int hibernate(void)
if (error)
goto Exit;
+ printk("Syncing filesystems ... ");
+ sys_sync();
+ printk("done.\n");
+
error = prepare_processes();
if (error)
goto Finish;
Index: linux-2.6.22-rc6-mm1/kernel/power/main.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/main.c 2007-07-11 20:48:05.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/main.c 2007-07-11 20:48:23.000000000 +0200
@@ -20,6 +20,7 @@
#include <linux/resume-trace.h>
#include <linux/freezer.h>
#include <linux/vmstat.h>
+#include <linux/syscalls.h>
#include "power.h"
@@ -234,9 +235,14 @@ static int enter_state(suspend_state_t s
if (!valid_state(state))
return -ENODEV;
+
if (!mutex_trylock(&pm_mutex))
return -EBUSY;
+ printk("Syncing filesystems ... ");
+ sys_sync();
+ printk("done.\n");
+
pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]);
if ((error = suspend_prepare()))
goto Unlock;
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c 2007-07-11 20:48:05.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c 2007-07-11 20:48:23.000000000 +0200
@@ -191,7 +191,6 @@ int freeze_processes(void)
if (error)
return error;
- sys_sync();
error = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
if (error)
return error;
Index: linux-2.6.22-rc6-mm1/kernel/power/user.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/user.c 2007-07-11 20:48:05.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/user.c 2007-07-11 20:48:23.000000000 +0200
@@ -153,6 +153,10 @@ static int snapshot_ioctl(struct inode *
mutex_lock(&pm_mutex);
error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
if (!error) {
+ printk("Syncing filesystems ... ");
+ sys_sync();
+ printk("done.\n");
+
error = freeze_processes();
if (error)
thaw_processes();
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
2007-07-11 22:08 ` [RFC][PATCH -mm 1/7] Freezer: Document relationship with memory shrinking Rafael J. Wysocki
2007-07-11 22:10 ` [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes Rafael J. Wysocki
@ 2007-07-11 22:12 ` Rafael J. Wysocki
2007-07-11 22:25 ` Nigel Cunningham
2007-07-11 22:13 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros Rafael J. Wysocki
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:12 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
Tasks should go to the refrigerator only if explicitly requested to do that by
the freezer and not as a result of inheriting the TIF_FREEZE flag set from the
parent. Make it happen.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
kernel/fork.c | 1 +
1 file changed, 1 insertion(+)
Index: linux-2.6.22-rc6-mm1/kernel/fork.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/fork.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/fork.c 2007-07-11 20:50:42.000000000 +0200
@@ -932,6 +932,7 @@ static inline void copy_flags(unsigned l
if (!(clone_flags & CLONE_PTRACE))
p->ptrace = 0;
p->flags = new_flags;
+ clear_freeze_flag(p);
}
asmlinkage long sys_set_tid_address(int __user *tidptr)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
` (2 preceding siblings ...)
2007-07-11 22:12 ` [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set Rafael J. Wysocki
@ 2007-07-11 22:13 ` Rafael J. Wysocki
2007-07-11 22:56 ` Pavel Machek
2007-07-11 23:02 ` Oleg Nesterov
2007-07-11 22:14 ` [RFC][PATCH -mm 5/7] Freezer: Do not send signals to kernel threads (updated) Rafael J. Wysocki
` (2 subsequent siblings)
6 siblings, 2 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:13 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
Introduce freezer-friendly wrappers around wait_event_interruptible() and
wait_event_interruptible_timeout(), originally defined in <linux/wait.h>, to
be used in freezable kernel threads. Make some of the freezable kernel threads
use them.
This is necessary for the freezer to stop sending signals to kernel threads,
which is implemented in the next patch.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/input/gameport/gameport.c | 3 --
drivers/input/serio/serio.c | 3 --
drivers/input/touchscreen/ucb1400_ts.c | 3 --
drivers/media/dvb/dvb-core/dvb_frontend.c | 3 +-
drivers/usb/core/hub.c | 3 --
drivers/usb/storage/usb.c | 5 ---
include/linux/freezer.h | 40 ++++++++++++++++++++++++++++--
7 files changed, 45 insertions(+), 15 deletions(-)
Index: linux-2.6.22-rc6-mm1/include/linux/freezer.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/freezer.h 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/freezer.h 2007-07-11 20:51:14.000000000 +0200
@@ -4,6 +4,7 @@
#define FREEZER_H_INCLUDED
#include <linux/sched.h>
+#include <linux/wait.h>
#ifdef CONFIG_PM
/*
@@ -126,7 +127,35 @@ static inline void set_freezable(void)
current->flags &= ~PF_NOFREEZE;
}
-#else
+/*
+ * Freezer-friendly wrappers around wait_event_interruptible() and
+ * wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
+ */
+
+#define wait_event_freezable(wq, condition) \
+({ \
+ int __ret; \
+ do { \
+ __ret = wait_event_interruptible(wq, \
+ (condition) || freezing(current)); \
+ try_to_freeze(); \
+ } while (!(condition)); \
+ __ret; \
+})
+
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ do { \
+ __ret = wait_event_interruptible_timeout(wq, \
+ (condition) || freezing(current), \
+ __ret); \
+ try_to_freeze(); \
+ } while (!(condition)); \
+ __ret; \
+})
+#else /* CONFIG_PM */
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) {}
@@ -143,6 +172,13 @@ static inline void freezer_do_not_count(
static inline void freezer_count(void) {}
static inline int freezer_should_skip(struct task_struct *p) { return 0; }
static inline void set_freezable(void) {}
-#endif
+
+#define wait_event_freezable(wq, condition) \
+ wait_event_interruptible(wq, condition)
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+ wait_event_interruptible_timeout(wq, condition, timeout)
+
+#endif /* CONFIG_PM */
#endif /* FREEZER_H_INCLUDED */
Index: linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/gameport/gameport.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c 2007-07-11 20:51:14.000000000 +0200
@@ -448,9 +448,8 @@ static int gameport_thread(void *nothing
set_freezable();
do {
gameport_handle_event();
- wait_event_interruptible(gameport_wait,
+ wait_event_freezable(gameport_wait,
kthread_should_stop() || !list_empty(&gameport_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "gameport: kgameportd exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/serio/serio.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c 2007-07-11 20:51:14.000000000 +0200
@@ -387,9 +387,8 @@ static int serio_thread(void *nothing)
set_freezable();
do {
serio_handle_event();
- wait_event_interruptible(serio_wait,
+ wait_event_freezable(serio_wait,
kthread_should_stop() || !list_empty(&serio_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "serio: kseriod exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/touchscreen/ucb1400_ts.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c 2007-07-11 20:51:14.000000000 +0200
@@ -334,10 +334,9 @@ static int ucb1400_ts_thread(void *_ucb)
timeout = msecs_to_jiffies(10);
}
- wait_event_interruptible_timeout(ucb->ts_wait,
+ wait_event_freezable_timeout(ucb->ts_wait,
ucb->irq_pending || ucb->ts_restart || kthread_should_stop(),
timeout);
- try_to_freeze();
}
/* Send the "pen off" if we are stopping with the pen still active */
Index: linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:51:14.000000000 +0200
@@ -528,7 +528,8 @@ static int dvb_frontend_thread(void *dat
up(&fepriv->sem); /* is locked when we enter the thread... */
restart:
timeout = wait_event_interruptible_timeout(fepriv->wait_queue,
- dvb_frontend_should_wakeup(fe) || kthread_should_stop(),
+ dvb_frontend_should_wakeup(fe) || kthread_should_stop()
+ || freezing(current),
fepriv->delay);
if (kthread_should_stop() || dvb_frontend_is_exiting(fe)) {
Index: linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/core/hub.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c 2007-07-11 20:51:14.000000000 +0200
@@ -2729,10 +2729,9 @@ static int hub_thread(void *__unused)
set_freezable();
do {
hub_events();
- wait_event_interruptible(khubd_wait,
+ wait_event_freezable(khubd_wait,
!list_empty(&hub_event_list) ||
kthread_should_stop());
- try_to_freeze();
} while (!kthread_should_stop() || !list_empty(&hub_event_list));
pr_debug("%s: khubd exiting\n", usbcore_name);
Index: linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/storage/usb.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c 2007-07-11 20:51:14.000000000 +0200
@@ -913,12 +913,9 @@ static int usb_stor_scan_thread(void * _
if (delay_use > 0) {
printk(KERN_DEBUG "usb-storage: waiting for device "
"to settle before scanning\n");
-retry:
- wait_event_interruptible_timeout(us->delay_wait,
+ wait_event_freezable_timeout(us->delay_wait,
test_bit(US_FLIDX_DISCONNECTING, &us->flags),
delay_use * HZ);
- if (try_to_freeze())
- goto retry;
}
/* If the device is still connected, perform the scanning */
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 5/7] Freezer: Do not send signals to kernel threads (updated)
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
` (3 preceding siblings ...)
2007-07-11 22:13 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros Rafael J. Wysocki
@ 2007-07-11 22:14 ` Rafael J. Wysocki
2007-07-11 22:16 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose Rafael J. Wysocki
2007-07-11 22:17 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Rafael J. Wysocki
6 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:14 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
The freezer should not send signals to kernel threads, since that may lead to
subtle problems. In particular, commit b74d0deb968e1f85942f17080eace015ce3c332c
has changed recalc_sigpending_tsk() so that it doesn't clear TIF_SIGPENDING.
For this reason, if the freezer continues to send fake signals to kernel threads
and the freezing of kernel threads fails, some of them may be running with
TIF_SIGPENDING set forever.
Accordingly, recalc_sigpending_tsk() shouldn't set the task's TIF_SIGPENDING
flag if TIF_FREEZE is set.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
Documentation/power/freezing-of-tasks.txt | 31 +++++---
kernel/power/process.c | 107 ++++++++++++++++++++----------
kernel/signal.c | 1
3 files changed, 93 insertions(+), 46 deletions(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c 2007-07-11 20:50:28.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c 2007-07-11 20:51:48.000000000 +0200
@@ -75,21 +75,79 @@ void refrigerator(void)
__set_current_state(save);
}
-static void freeze_task(struct task_struct *p)
+static void fake_signal_wake_up(struct task_struct *p, int resume)
{
unsigned long flags;
- if (!freezing(p)) {
+ spin_lock_irqsave(&p->sighand->siglock, flags);
+ signal_wake_up(p, resume);
+ spin_unlock_irqrestore(&p->sighand->siglock, flags);
+}
+
+static void send_fake_signal(struct task_struct *p)
+{
+ if (p->state == TASK_STOPPED)
+ force_sig_specific(SIGSTOP, p);
+ fake_signal_wake_up(p, p->state == TASK_STOPPED);
+}
+
+static int has_mm(struct task_struct *p)
+{
+ return (p->mm && !(p->flags & PF_BORROWED_MM));
+}
+
+/**
+ * freeze_task - send a freeze request to given task
+ * @p: task to send the request to
+ * @with_mm_only: if set, the request will only be sent if the task has its
+ * own mm
+ * Return value: 0, if @with_mm_only is set and the task has no mm of its
+ * own or the task is frozen, 1, otherwise
+ *
+ * The freeze request is sent by seting 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 its own mm (ie. it is a user land task). If @with_mm_only
+ * is set and the task has no mm of its own (ie. it is a kernel thread),
+ * its TIF_FREEZE flag should not be set.
+ *
+ * The task_lock() is necessary to prevent races with exit_mm() or
+ * use_mm()/unuse_mm() from occuring.
+ */
+static int freeze_task(struct task_struct *p, int with_mm_only)
+{
+ int ret = 1;
+
+ task_lock(p);
+ if (freezing(p)) {
+ if (has_mm(p)) {
+ if (!signal_pending(p))
+ fake_signal_wake_up(p, 0);
+ } else {
+ if (with_mm_only)
+ ret = 0;
+ else
+ wake_up_state(p, TASK_INTERRUPTIBLE);
+ }
+ } else {
rmb();
- if (!frozen(p)) {
- set_freeze_flag(p);
- if (p->state == TASK_STOPPED)
- force_sig_specific(SIGSTOP, p);
- spin_lock_irqsave(&p->sighand->siglock, flags);
- signal_wake_up(p, p->state == TASK_STOPPED);
- spin_unlock_irqrestore(&p->sighand->siglock, flags);
+ if (frozen(p)) {
+ ret = 0;
+ } else {
+ if (has_mm(p)) {
+ set_freeze_flag(p);
+ send_fake_signal(p);
+ } else {
+ if (with_mm_only) {
+ ret = 0;
+ } else {
+ set_freeze_flag(p);
+ wake_up_state(p, TASK_INTERRUPTIBLE);
+ }
+ }
}
}
+ task_unlock(p);
+ return ret;
}
static void cancel_freezing(struct task_struct *p)
@@ -119,31 +177,14 @@ static int try_to_freeze_tasks(int freez
if (frozen(p) || !freezeable(p))
continue;
- if (freeze_user_space) {
- if (p->state == TASK_TRACED &&
- frozen(p->parent)) {
- cancel_freezing(p);
- continue;
- }
- /*
- * Kernel threads should not have TIF_FREEZE set
- * at this point, so we must ensure that either
- * p->mm is not NULL *and* PF_BORROWED_MM is
- * unset, or TIF_FRREZE is left unset.
- * The task_lock() is necessary to prevent races
- * with exit_mm() or use_mm()/unuse_mm() from
- * occuring.
- */
- task_lock(p);
- if (!p->mm || (p->flags & PF_BORROWED_MM)) {
- task_unlock(p);
- continue;
- }
- freeze_task(p);
- task_unlock(p);
- } else {
- freeze_task(p);
+ if (p->state == TASK_TRACED && frozen(p->parent)) {
+ cancel_freezing(p);
+ continue;
}
+
+ if (!freeze_task(p, freeze_user_space))
+ continue;
+
if (!freezer_should_skip(p))
todo++;
} while_each_thread(g, p);
Index: linux-2.6.22-rc6-mm1/kernel/signal.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/signal.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/signal.c 2007-07-11 20:51:48.000000000 +0200
@@ -99,7 +99,6 @@ static inline int has_pending_signals(si
static int recalc_sigpending_tsk(struct task_struct *t)
{
if (t->signal->group_stop_count > 0 ||
- (freezing(t)) ||
PENDING(&t->pending, &t->blocked) ||
PENDING(&t->signal->shared_pending, &t->blocked)) {
set_tsk_thread_flag(t, TIF_SIGPENDING);
Index: linux-2.6.22-rc6-mm1/Documentation/power/freezing-of-tasks.txt
===================================================================
--- linux-2.6.22-rc6-mm1.orig/Documentation/power/freezing-of-tasks.txt 2007-07-11 20:50:24.000000000 +0200
+++ linux-2.6.22-rc6-mm1/Documentation/power/freezing-of-tasks.txt 2007-07-11 20:51:48.000000000 +0200
@@ -19,12 +19,13 @@ we only consider hibernation, but the de
Namely, as the first step of the hibernation procedure the function
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
-sends a fake signal to each of them. A task that receives such a signal and has
-TIF_FREEZE set, should react to it by calling the refrigerator() function
-(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 handling this mechanism is called 'the freezer' (these functions are
+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
+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
+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.
@@ -35,21 +36,27 @@ task enter refrigerator() if the flag is
For user space processes try_to_freeze() is called automatically from the
signal-handling code, but the freezable kernel threads need to call it
-explicitly in suitable places. The code to do this may look like the following:
+explicitly in suitable places or use the wait_event_freezable() or
+wait_event_freezable_timeout() macros (defined in include/linux/freezer.h)
+that combine interruptible sleep with checking if TIF_FREEZE is set and calling
+try_to_freeze(). The main loop of a freezable kernel thread may look like the
+following one:
+ set_freezable();
do {
hub_events();
- wait_event_interruptible(khubd_wait,
- !list_empty(&hub_event_list));
- try_to_freeze();
- } while (!signal_pending(current));
+ wait_event_freezable(khubd_wait,
+ !list_empty(&hub_event_list) ||
+ kthread_should_stop());
+ } while (!kthread_should_stop() || !list_empty(&hub_event_list));
(from drivers/usb/core/hub.c::hub_thread()).
If a freezable kernel thread fails to call try_to_freeze() after the freezer has
set TIF_FREEZE for it, the freezing of tasks will fail and the entire
hibernation operation will be cancelled. For this reason, freezable kernel
-threads must call try_to_freeze() somewhere.
+threads must call try_to_freeze() somewhere or use one of the
+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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 6/7] Freezer: Be more verbose
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
` (4 preceding siblings ...)
2007-07-11 22:14 ` [RFC][PATCH -mm 5/7] Freezer: Do not send signals to kernel threads (updated) Rafael J. Wysocki
@ 2007-07-11 22:16 ` Rafael J. Wysocki
2007-07-11 22:58 ` Pavel Machek
2007-07-11 22:17 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Rafael J. Wysocki
6 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:16 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
Increase the freezer's verbosity a bit, so that it's easier to read problem
reports related to it.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
kernel/power/process.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c 2007-07-11 20:51:48.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c 2007-07-11 20:52:07.000000000 +0200
@@ -225,20 +225,23 @@ static int try_to_freeze_tasks(int freez
*/
int freeze_processes(void)
{
- int error;
+ int error = 0;
- printk("Stopping tasks ... ");
+ printk("Freezing user space processes ... ");
error = try_to_freeze_tasks(FREEZER_USER_SPACE);
if (error)
- return error;
+ goto Exit;
+ printk("done.\n");
+ printk("Freezing remaining freezable tasks ... ");
error = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
if (error)
- return error;
-
- printk("done.\n");
+ goto Exit;
+ printk("done.");
+ Exit:
BUG_ON(in_atomic());
- return 0;
+ printk("\n");
+ return error;
}
static void thaw_tasks(int thaw_user_space)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
` (5 preceding siblings ...)
2007-07-11 22:16 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose Rafael J. Wysocki
@ 2007-07-11 22:17 ` Rafael J. Wysocki
2007-07-12 10:38 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently (updated) Rafael J. Wysocki
2007-07-13 23:43 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Pavel Machek
6 siblings, 2 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-11 22:17 UTC (permalink / raw)
To: pm list; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
From: Rafael J. Wysocki <rjw@sisk.pl>
There is the problem with try_to_freeze_tasks() that it always loops until the
timeout expires, even if it is certain to fail much earlier. Namely, if there
are uninterruptible tasks waiting for some frozen tasks to let them continue,
try_to_freeze_tasks() will certainly fail and it shouldn't waste time in that
cases.
To detect such situations, we can check if the number of tasks that haven't
frozen yet changes between subsequent iterations of the main loop in
try_to_freeze_tasks(). If this number hasn't been changing for sufficiently
long time (say, 250 ms), then most probably some uninterruptible tasks are
blocked by some frozen tasks and we should break out of this stalemate. Thus,
it seems reasonable to thaw the tasks that have already been frozen without
clearing the freeze requests of the tasks that are refusing to freeze. This
way, if these tasks are really blocked by the frozen ones, they will get extra
chance to freeze themselves after we have thawed the other tasks and before we
request those tasks to freeze again. Next, the freezing loop can be repeated
and so on, until all tasks are frozen or the timeout expires.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/process.c | 84 ++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 70 insertions(+), 14 deletions(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c 2007-07-11 22:25:50.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c 2007-07-11 23:26:00.000000000 +0200
@@ -14,10 +14,11 @@
#include <linux/syscalls.h>
#include <linux/freezer.h>
-/*
- * Timeout for stopping processes
- */
-#define TIMEOUT (20 * HZ)
+/* Timeout for the freezing of tasks */
+#define TIMEOUT (10 * HZ)
+
+/* Timeout for breaking the freezing loop if stalemate state is detected */
+#define BREAK_TIMEOUT (HZ / 4)
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -163,14 +164,31 @@ static void cancel_freezing(struct task_
}
}
+/**
+ * try_to_freeze_tasks - send freeze requests to all tasks and wait for
+ * for them to enter the refrigerator
+ * @freeze_user_space - if set, only tasks that have mm of their own are
+ * requested to freeze
+ *
+ * If this function fails, thaw_tasks() must be called to do the cleanup.
+ */
+
static int try_to_freeze_tasks(int freeze_user_space)
{
struct task_struct *g, *p;
- unsigned long end_time;
- unsigned int todo;
+ unsigned long end_time, break_time;
+ unsigned int todo, prev_todo;
+ unsigned int i = 0;
+ char *tick = "-\\|/";
+
+ printk(" ");
end_time = jiffies + TIMEOUT;
+ Repeat:
+ todo = 0;
+ break_time = 0;
do {
+ prev_todo = todo;
todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
@@ -189,23 +207,61 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
+
yield(); /* Yield is okay here */
+
if (time_after(jiffies, end_time))
break;
+
+ /*
+ * Check if we are making any progress. If we aren't, it's
+ * better to break out of this.
+ */
+ if (todo == prev_todo) {
+ if (!break_time)
+ break_time = jiffies + BREAK_TIMEOUT;
+ else if (time_after(jiffies, break_time))
+ break;
+ } else {
+ break_time = 0;
+ }
} while (todo);
+ if (todo && freeze_user_space && !time_after(jiffies, end_time)) {
+ /*
+ * Some tasks have not been able to freeze. They might be stuck
+ * in TASK_UNINTERRUPTIBLE waiting for the frozen tasks. Try to
+ * thaw the tasks that have frozen without clearing the freeze
+ * requests of the remaining tasks and repeat.
+ */
+ read_lock(&tasklist_lock);
+ do_each_thread(g, p) {
+ if (frozen(p)) {
+ p->flags &= ~PF_FROZEN;
+ wake_up_process(p);
+ }
+ } while_each_thread(g, p);
+ read_unlock(&tasklist_lock);
+
+ yield();
+
+ printk("\b%c", tick[i++%4]);
+
+ goto Repeat;
+ }
+
+ printk("\b");
+
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.
+ /*
+ * The freezing of tasks has failed. List the tasks that have
+ * refused to freeze. This also clears all pending freeze
+ * requests.
*/
printk("\n");
- printk(KERN_ERR "Freezing of %s timed out after %d seconds "
- "(%d tasks refusing to freeze):\n",
- freeze_user_space ? "user space " : "tasks ",
- TIMEOUT / HZ, todo);
+ printk(KERN_ERR "Freezing of tasks has failed.\n");
show_state();
+ printk(KERN_ERR "Tasks that have not been frozen:\n");
read_lock(&tasklist_lock);
do_each_thread(g, p) {
task_lock(p);
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set
2007-07-11 22:12 ` [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set Rafael J. Wysocki
@ 2007-07-11 22:25 ` Nigel Cunningham
0 siblings, 0 replies; 20+ messages in thread
From: Nigel Cunningham @ 2007-07-11 22:25 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Matthew Garrett, pm list, Oleg Nesterov, Pavel Machek,
Miklos Szeredi
[-- Attachment #1.1: Type: text/plain, Size: 1110 bytes --]
Hi.
On Thursday 12 July 2007 08:12:03 Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Tasks should go to the refrigerator only if explicitly requested to do that
by
> the freezer and not as a result of inheriting the TIF_FREEZE flag set from
the
> parent. áMake it happen.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
> kernel/fork.c | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: linux-2.6.22-rc6-mm1/kernel/fork.c
> ===================================================================
> --- linux-2.6.22-rc6-mm1.orig/kernel/fork.c 2007-07-11 20:48:04.000000000
+0200
> +++ linux-2.6.22-rc6-mm1/kernel/fork.c 2007-07-11 20:50:42.000000000 +0200
> @@ -932,6 +932,7 @@ static inline void copy_flags(unsigned l
> if (!(clone_flags & CLONE_PTRACE))
> p->ptrace = 0;
> p->flags = new_flags;
> + clear_freeze_flag(p);
> }
Acked-by: Nigel Cunningham <nigel@nigel.suspend2.net>
Regards,
Nigel
--
See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes
2007-07-11 22:10 ` [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes Rafael J. Wysocki
@ 2007-07-11 22:42 ` Pavel Machek
0 siblings, 0 replies; 20+ messages in thread
From: Pavel Machek @ 2007-07-11 22:42 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
On Thu 2007-07-12 00:10:43, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> The syncing of filesystems from within the freezer is generally not needed.
> Also, if there's an ext3 filesystem loopback-mounted from a FUSE one, the
> syncing results in writes to it and deadlocks. Similarly, it will deadlock if
> FUSE implements sync.
>
> Change freeze_processes() so that it doesn't execute sys_sync() and make the
> suspend and hibernation code paths sync filesystems independently of the
> freezer.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
ACK. Nice to see the fuse problem to disappear.
Pavel
> Index: linux-2.6.22-rc6-mm1/kernel/power/user.c
> ===================================================================
> --- linux-2.6.22-rc6-mm1.orig/kernel/power/user.c 2007-07-11 20:48:05.000000000 +0200
> +++ linux-2.6.22-rc6-mm1/kernel/power/user.c 2007-07-11 20:48:23.000000000 +0200
> @@ -153,6 +153,10 @@ static int snapshot_ioctl(struct inode *
> mutex_lock(&pm_mutex);
> error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
> if (!error) {
> + printk("Syncing filesystems ... ");
> + sys_sync();
> + printk("done.\n");
> +
> error = freeze_processes();
> if (error)
> thaw_processes();
I see this is back-compatible, but can we do the sync in userspace,
instead?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros
2007-07-11 22:13 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros Rafael J. Wysocki
@ 2007-07-11 22:56 ` Pavel Machek
2007-07-12 10:33 ` Rafael J. Wysocki
2007-07-11 23:02 ` Oleg Nesterov
1 sibling, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2007-07-11 22:56 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
Hi!
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Introduce freezer-friendly wrappers around wait_event_interruptible() and
> wait_event_interruptible_timeout(), originally defined in <linux/wait.h>, to
> be used in freezable kernel threads. Make some of the freezable kernel threads
> use them.
>
> This is necessary for the freezer to stop sending signals to kernel threads,
> which is implemented in the next patch.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
ACK, but...
> +/*
> + * Freezer-friendly wrappers around wait_event_interruptible() and
> + * wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
> + */
> +
> +#define wait_event_freezable(wq, condition) \
> +({ \
> + int __ret; \
> + do { \
> + __ret = wait_event_interruptible(wq, \
> + (condition) || freezing(current)); \
> + try_to_freeze(); \
> + } while (!(condition)); \
> + __ret; \
> +})
...
> Index: linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
> ===================================================================
> --- linux-2.6.22-rc6-mm1.orig/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:48:04.000000000 +0200
> +++ linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:51:14.000000000 +0200
> @@ -528,7 +528,8 @@ static int dvb_frontend_thread(void *dat
> up(&fepriv->sem); /* is locked when we enter the thread... */
> restart:
> timeout = wait_event_interruptible_timeout(fepriv->wait_queue,
> - dvb_frontend_should_wakeup(fe) || kthread_should_stop(),
> + dvb_frontend_should_wakeup(fe) || kthread_should_stop()
> + || freezing(current),
> fepriv->delay);
Should this use the new helper? This change does not seem to match the rest.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 6/7] Freezer: Be more verbose
2007-07-11 22:16 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose Rafael J. Wysocki
@ 2007-07-11 22:58 ` Pavel Machek
2007-07-12 10:39 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose (updated) Rafael J. Wysocki
0 siblings, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2007-07-11 22:58 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
On Thu 2007-07-12 00:16:00, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Increase the freezer's verbosity a bit, so that it's easier to read problem
> reports related to it.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@ucw.cz>
Tiny nit:
> int freeze_processes(void)
> {
> - int error;
> + int error = 0;
You still did not kill that = 0 :-).
> - printk("Stopping tasks ... ");
> + printk("Freezing user space processes ... ");
> error = try_to_freeze_tasks(FREEZER_USER_SPACE);
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros
2007-07-11 22:13 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros Rafael J. Wysocki
2007-07-11 22:56 ` Pavel Machek
@ 2007-07-11 23:02 ` Oleg Nesterov
2007-07-12 10:30 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated) Rafael J. Wysocki
1 sibling, 1 reply; 20+ messages in thread
From: Oleg Nesterov @ 2007-07-11 23:02 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Matthew Garrett, pm list, Pavel Machek, Miklos Szeredi
On 07/12, Rafael J. Wysocki wrote:
>
> +#define wait_event_freezable(wq, condition) \
> +({ \
> + int __ret; \
> + do { \
> + __ret = wait_event_interruptible(wq, \
> + (condition) || freezing(current)); \
> + try_to_freeze(); \
> + } while (!(condition)); \
> + __ret; \
> +})
This is still not right in general, I believe.
It will spin if the caller is signalled. You changed the freezer, but
kthread can use signals anyway.
It returns only when "condition" is true, so the "__ret" is meaningless.
Oleg.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated)
2007-07-11 23:02 ` Oleg Nesterov
@ 2007-07-12 10:30 ` Rafael J. Wysocki
2007-07-12 12:23 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated 2x) Rafael J. Wysocki
0 siblings, 1 reply; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-12 10:30 UTC (permalink / raw)
To: Oleg Nesterov, Pavel Machek; +Cc: Matthew Garrett, pm list, Miklos Szeredi
On Thursday, 12 July 2007 01:02, Oleg Nesterov wrote:
> On 07/12, Rafael J. Wysocki wrote:
> >
> > +#define wait_event_freezable(wq, condition) \
> > +({ \
> > + int __ret; \
> > + do { \
> > + __ret = wait_event_interruptible(wq, \
> > + (condition) || freezing(current)); \
> > + try_to_freeze(); \
> > + } while (!(condition)); \
> > + __ret; \
> > +})
>
> This is still not right in general, I believe.
>
> It will spin if the caller is signalled. You changed the freezer, but
> kthread can use signals anyway.
>
> It returns only when "condition" is true, so the "__ret" is meaningless.
Yes, this is a mistake. The second one is also wrong. :-(
Please see the fixed patch below.
Greetings,
Rafael
---
From: Rafael J. Wysocki <rjw@sisk.pl>
Introduce freezer-friendly wrappers around wait_event_interruptible() and
wait_event_interruptible_timeout(), originally defined in <linux/wait.h>, to
be used in freezable kernel threads. Make some of the freezable kernel threads
use them.
This is necessary for the freezer to stop sending signals to kernel threads,
which is implemented in the next patch.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/input/gameport/gameport.c | 3 --
drivers/input/serio/serio.c | 3 --
drivers/input/touchscreen/ucb1400_ts.c | 3 --
drivers/media/dvb/dvb-core/dvb_frontend.c | 3 +-
drivers/usb/core/hub.c | 3 --
drivers/usb/storage/usb.c | 5 ---
include/linux/freezer.h | 42 ++++++++++++++++++++++++++++--
7 files changed, 47 insertions(+), 15 deletions(-)
Index: linux-2.6.22-rc6-mm1/include/linux/freezer.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/freezer.h 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/include/linux/freezer.h 2007-07-12 11:44:41.000000000 +0200
@@ -4,6 +4,7 @@
#define FREEZER_H_INCLUDED
#include <linux/sched.h>
+#include <linux/wait.h>
#ifdef CONFIG_PM
/*
@@ -126,7 +127,37 @@ static inline void set_freezable(void)
current->flags &= ~PF_NOFREEZE;
}
-#else
+/*
+ * Freezer-friendly wrappers around wait_event_interruptible() and
+ * wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
+ */
+
+#define wait_event_freezable(wq, condition) \
+({ \
+ int __ret; \
+ do { \
+ __ret = wait_event_interruptible(wq, \
+ (condition) || freezing(current)); \
+ if (!__ret && !freezing(current)) \
+ break; \
+ else if (!(condition)) \
+ __ret = -ERESTARTSYS; \
+ } while (try_to_freeze()); \
+ __ret; \
+})
+
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ do { \
+ __ret = wait_event_interruptible_timeout(wq, \
+ (condition) || freezing(current), \
+ __ret); \
+ } while (try_to_freeze()); \
+ __ret; \
+})
+#else /* CONFIG_PM */
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) {}
@@ -143,6 +174,13 @@ static inline void freezer_do_not_count(
static inline void freezer_count(void) {}
static inline int freezer_should_skip(struct task_struct *p) { return 0; }
static inline void set_freezable(void) {}
-#endif
+
+#define wait_event_freezable(wq, condition) \
+ wait_event_interruptible(wq, condition)
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+ wait_event_interruptible_timeout(wq, condition, timeout)
+
+#endif /* CONFIG_PM */
#endif /* FREEZER_H_INCLUDED */
Index: linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/gameport/gameport.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c 2007-07-11 20:51:14.000000000 +0200
@@ -448,9 +448,8 @@ static int gameport_thread(void *nothing
set_freezable();
do {
gameport_handle_event();
- wait_event_interruptible(gameport_wait,
+ wait_event_freezable(gameport_wait,
kthread_should_stop() || !list_empty(&gameport_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "gameport: kgameportd exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/serio/serio.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c 2007-07-11 20:51:14.000000000 +0200
@@ -387,9 +387,8 @@ static int serio_thread(void *nothing)
set_freezable();
do {
serio_handle_event();
- wait_event_interruptible(serio_wait,
+ wait_event_freezable(serio_wait,
kthread_should_stop() || !list_empty(&serio_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "serio: kseriod exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/touchscreen/ucb1400_ts.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c 2007-07-11 20:51:14.000000000 +0200
@@ -334,10 +334,9 @@ static int ucb1400_ts_thread(void *_ucb)
timeout = msecs_to_jiffies(10);
}
- wait_event_interruptible_timeout(ucb->ts_wait,
+ wait_event_freezable_timeout(ucb->ts_wait,
ucb->irq_pending || ucb->ts_restart || kthread_should_stop(),
timeout);
- try_to_freeze();
}
/* Send the "pen off" if we are stopping with the pen still active */
Index: linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:51:14.000000000 +0200
@@ -528,7 +528,8 @@ static int dvb_frontend_thread(void *dat
up(&fepriv->sem); /* is locked when we enter the thread... */
restart:
timeout = wait_event_interruptible_timeout(fepriv->wait_queue,
- dvb_frontend_should_wakeup(fe) || kthread_should_stop(),
+ dvb_frontend_should_wakeup(fe) || kthread_should_stop()
+ || freezing(current),
fepriv->delay);
if (kthread_should_stop() || dvb_frontend_is_exiting(fe)) {
Index: linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/core/hub.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c 2007-07-11 20:51:14.000000000 +0200
@@ -2729,10 +2729,9 @@ static int hub_thread(void *__unused)
set_freezable();
do {
hub_events();
- wait_event_interruptible(khubd_wait,
+ wait_event_freezable(khubd_wait,
!list_empty(&hub_event_list) ||
kthread_should_stop());
- try_to_freeze();
} while (!kthread_should_stop() || !list_empty(&hub_event_list));
pr_debug("%s: khubd exiting\n", usbcore_name);
Index: linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/storage/usb.c 2007-07-11 20:48:04.000000000 +0200
+++ linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c 2007-07-11 20:51:14.000000000 +0200
@@ -913,12 +913,9 @@ static int usb_stor_scan_thread(void * _
if (delay_use > 0) {
printk(KERN_DEBUG "usb-storage: waiting for device "
"to settle before scanning\n");
-retry:
- wait_event_interruptible_timeout(us->delay_wait,
+ wait_event_freezable_timeout(us->delay_wait,
test_bit(US_FLIDX_DISCONNECTING, &us->flags),
delay_use * HZ);
- if (try_to_freeze())
- goto retry;
}
/* If the device is still connected, perform the scanning */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros
2007-07-11 22:56 ` Pavel Machek
@ 2007-07-12 10:33 ` Rafael J. Wysocki
0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-12 10:33 UTC (permalink / raw)
To: Pavel Machek; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
On Thursday, 12 July 2007 00:56, Pavel Machek wrote:
> Hi!
>
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Introduce freezer-friendly wrappers around wait_event_interruptible() and
> > wait_event_interruptible_timeout(), originally defined in <linux/wait.h>, to
> > be used in freezable kernel threads. Make some of the freezable kernel threads
> > use them.
> >
> > This is necessary for the freezer to stop sending signals to kernel threads,
> > which is implemented in the next patch.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> ACK, but...
>
> > +/*
> > + * Freezer-friendly wrappers around wait_event_interruptible() and
> > + * wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
> > + */
> > +
> > +#define wait_event_freezable(wq, condition) \
> > +({ \
> > + int __ret; \
> > + do { \
> > + __ret = wait_event_interruptible(wq, \
> > + (condition) || freezing(current)); \
> > + try_to_freeze(); \
> > + } while (!(condition)); \
> > + __ret; \
> > +})
>
> ...
>
> > Index: linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
> > ===================================================================
> > --- linux-2.6.22-rc6-mm1.orig/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:48:04.000000000 +0200
> > +++ linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c 2007-07-11 20:51:14.000000000 +0200
> > @@ -528,7 +528,8 @@ static int dvb_frontend_thread(void *dat
> > up(&fepriv->sem); /* is locked when we enter the thread... */
> > restart:
> > timeout = wait_event_interruptible_timeout(fepriv->wait_queue,
> > - dvb_frontend_should_wakeup(fe) || kthread_should_stop(),
> > + dvb_frontend_should_wakeup(fe) || kthread_should_stop()
> > + || freezing(current),
> > fepriv->delay);
>
> Should this use the new helper? This change does not seem to match the rest.
Yes, because it uses something like the new helper already, but with some code
in between., so I decided to add the freezing(current) only to it.
BTW, please see the updated patch in the reply to Oleg (the macros have been
fixed).
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently (updated)
2007-07-11 22:17 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Rafael J. Wysocki
@ 2007-07-12 10:38 ` Rafael J. Wysocki
2007-07-13 23:43 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Pavel Machek
1 sibling, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-12 10:38 UTC (permalink / raw)
To: linux-pm; +Cc: Matthew Garrett, Oleg Nesterov, Pavel Machek, Miklos Szeredi
[The previous version didn't pass some tests.]
---
From: Rafael J. Wysocki <rjw@sisk.pl>
There is the problem with try_to_freeze_tasks() that it always loops until the
timeout expires, even if it is certain to fail much earlier. Namely, if there
are uninterruptible tasks waiting for some frozen tasks to let them continue,
try_to_freeze_tasks() will certainly fail and it shouldn't waste time in that
cases.
To detect such situations, we can check if the number of tasks that haven't
frozen yet changes between subsequent iterations of the main loop in
try_to_freeze_tasks(). If this number hasn't been changing for sufficiently
long time (say, 250 ms), then most probably some uninterruptible tasks are
blocked by some frozen tasks and we should break out of this stalemate. Thus,
it seems reasonable to thaw the tasks that have already been frozen without
clearing the freeze requests of the tasks that are refusing to freeze. This
way, if these tasks are really blocked by the frozen ones, they will get extra
chance to freeze themselves after we have thawed the other tasks and before we
request those tasks to freeze again. Next, the freezing loop can be repeated
and so on, until all tasks are frozen or the timeout expires.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/process.c | 86 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 72 insertions(+), 14 deletions(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c 2007-07-12 11:46:10.000000000 +0200
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c 2007-07-12 12:11:41.000000000 +0200
@@ -14,10 +14,11 @@
#include <linux/syscalls.h>
#include <linux/freezer.h>
-/*
- * Timeout for stopping processes
- */
-#define TIMEOUT (20 * HZ)
+/* Timeout for the freezing of tasks */
+#define TIMEOUT (10 * HZ)
+
+/* Timeout for breaking the freezing loop if stalemate state is detected */
+#define BREAK_TIMEOUT (HZ / 4)
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -163,14 +164,31 @@ static void cancel_freezing(struct task_
}
}
+/**
+ * try_to_freeze_tasks - send freeze requests to all tasks and wait for
+ * for them to enter the refrigerator
+ * @freeze_user_space - if set, only tasks that have mm of their own are
+ * requested to freeze
+ *
+ * If this function fails, thaw_tasks() must be called to do the cleanup.
+ */
+
static int try_to_freeze_tasks(int freeze_user_space)
{
struct task_struct *g, *p;
- unsigned long end_time;
- unsigned int todo;
+ unsigned long end_time, break_time;
+ unsigned int todo, prev_todo;
+ unsigned int i = 0;
+ char *tick = "-\\|/";
+
+ printk(" ");
end_time = jiffies + TIMEOUT;
+ Repeat:
+ todo = 0;
+ break_time = 0;
do {
+ prev_todo = todo;
todo = 0;
read_lock(&tasklist_lock);
do_each_thread(g, p) {
@@ -189,23 +207,63 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
+
yield(); /* Yield is okay here */
+
if (time_after(jiffies, end_time))
break;
+
+ if (freeze_user_space) {
+ /*
+ * Check if we are making any progress. If we aren't,
+ * it's better to break out of this.
+ */
+ if (todo == prev_todo) {
+ if (!break_time)
+ break_time = jiffies + BREAK_TIMEOUT;
+ else if (time_after(jiffies, break_time))
+ break;
+ } else {
+ break_time = 0;
+ }
+ }
} while (todo);
+ if (todo && freeze_user_space && !time_after(jiffies, end_time)) {
+ /*
+ * Some tasks have not been able to freeze. They might be stuck
+ * in TASK_UNINTERRUPTIBLE waiting for the frozen tasks. Try to
+ * thaw the tasks that have frozen without clearing the freeze
+ * requests of the remaining tasks and repeat.
+ */
+ read_lock(&tasklist_lock);
+ do_each_thread(g, p) {
+ if (frozen(p)) {
+ p->flags &= ~PF_FROZEN;
+ wake_up_process(p);
+ }
+ } while_each_thread(g, p);
+ read_unlock(&tasklist_lock);
+
+ yield();
+
+ printk("\b%c", tick[i++%4]);
+
+ goto Repeat;
+ }
+
+ printk("\b");
+
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.
+ /*
+ * The freezing of tasks has failed. List the tasks that have
+ * refused to freeze. This also clears all pending freeze
+ * requests.
*/
printk("\n");
- printk(KERN_ERR "Freezing of %s timed out after %d seconds "
- "(%d tasks refusing to freeze):\n",
- freeze_user_space ? "user space " : "tasks ",
- TIMEOUT / HZ, todo);
+ printk(KERN_ERR "Freezing of tasks has failed.\n");
show_state();
+ printk(KERN_ERR "Tasks that have not been frozen:\n");
read_lock(&tasklist_lock);
do_each_thread(g, p) {
task_lock(p);
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 6/7] Freezer: Be more verbose (updated)
2007-07-11 22:58 ` Pavel Machek
@ 2007-07-12 10:39 ` Rafael J. Wysocki
0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-12 10:39 UTC (permalink / raw)
To: Pavel Machek; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
On Thursday, 12 July 2007 00:58, Pavel Machek wrote:
> On Thu 2007-07-12 00:16:00, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> >
> > Increase the freezer's verbosity a bit, so that it's easier to read problem
> > reports related to it.
> >
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
>
> Tiny nit:
>
> > int freeze_processes(void)
> > {
> > - int error;
> > + int error = 0;
>
> You still did not kill that = 0 :-).
Yes, absolutely.
Updated patch follows.
Greetings,
Rafael
---
From: Rafael J. Wysocki <rjw@sisk.pl>
Increase the freezer's verbosity a bit, so that it's easier to read problem
reports related to it.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
kernel/power/process.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
Index: linux-2.6.22-rc6-mm1/kernel/power/process.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/kernel/power/process.c
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c
@@ -227,18 +227,21 @@ int freeze_processes(void)
{
int error;
- printk("Stopping tasks ... ");
+ printk("Freezing user space processes ... ");
error = try_to_freeze_tasks(FREEZER_USER_SPACE);
if (error)
- return error;
+ goto Exit;
+ printk("done.\n");
+ printk("Freezing remaining freezable tasks ... ");
error = try_to_freeze_tasks(FREEZER_KERNEL_THREADS);
if (error)
- return error;
-
- printk("done.\n");
+ goto Exit;
+ printk("done.");
+ Exit:
BUG_ON(in_atomic());
- return 0;
+ printk("\n");
+ return error;
}
static void thaw_tasks(int thaw_user_space)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated 2x)
2007-07-12 10:30 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated) Rafael J. Wysocki
@ 2007-07-12 12:23 ` Rafael J. Wysocki
0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-12 12:23 UTC (permalink / raw)
To: linux-pm
Cc: Matthew Garrett, Miklos Szeredi, Mauro Carvalho Chehab,
Pavel Machek, Oleg Nesterov
On Thursday, 12 July 2007 12:30, Rafael J. Wysocki wrote:
> On Thursday, 12 July 2007 01:02, Oleg Nesterov wrote:
> > On 07/12, Rafael J. Wysocki wrote:
> > >
> > > +#define wait_event_freezable(wq, condition) \
> > > +({ \
> > > + int __ret; \
> > > + do { \
> > > + __ret = wait_event_interruptible(wq, \
> > > + (condition) || freezing(current)); \
> > > + try_to_freeze(); \
> > > + } while (!(condition)); \
> > > + __ret; \
> > > +})
> >
> > This is still not right in general, I believe.
> >
> > It will spin if the caller is signalled. You changed the freezer, but
> > kthread can use signals anyway.
> >
> > It returns only when "condition" is true, so the "__ret" is meaningless.
>
> Yes, this is a mistake. The second one is also wrong. :-(
>
> Please see the fixed patch below.
Ugh, there still is a mistake in there, sorry.
[--snip--]
> +
> +#define wait_event_freezable(wq, condition) \
> +({ \
> + int __ret; \
> + do { \
> + __ret = wait_event_interruptible(wq, \
> + (condition) || freezing(current)); \
> + if (!__ret && !freezing(current)) \
There should be (__ret && !freezing(current)) here (ie. if we've been woken up
by a signal and this is not the freezing signal, break out of the loop.
Below is the corrected version.
Greetings,
Rafael
---
From: Rafael J. Wysocki <rjw@sisk.pl>
Introduce freezer-friendly wrappers around wait_event_interruptible() and
wait_event_interruptible_timeout(), originally defined in <linux/wait.h>, to
be used in freezable kernel threads. Make some of the freezable kernel threads
use them.
This is necessary for the freezer to stop sending signals to kernel threads,
which is implemented in the next patch.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/input/gameport/gameport.c | 3 --
drivers/input/serio/serio.c | 3 --
drivers/input/touchscreen/ucb1400_ts.c | 3 --
drivers/media/dvb/dvb-core/dvb_frontend.c | 3 +-
drivers/usb/core/hub.c | 3 --
drivers/usb/storage/usb.c | 5 ---
include/linux/freezer.h | 42 ++++++++++++++++++++++++++++--
7 files changed, 47 insertions(+), 15 deletions(-)
Index: linux-2.6.22-rc6-mm1/include/linux/freezer.h
===================================================================
--- linux-2.6.22-rc6-mm1.orig/include/linux/freezer.h
+++ linux-2.6.22-rc6-mm1/include/linux/freezer.h
@@ -4,6 +4,7 @@
#define FREEZER_H_INCLUDED
#include <linux/sched.h>
+#include <linux/wait.h>
#ifdef CONFIG_PM
/*
@@ -126,7 +127,37 @@ static inline void set_freezable(void)
current->flags &= ~PF_NOFREEZE;
}
-#else
+/*
+ * Freezer-friendly wrappers around wait_event_interruptible() and
+ * wait_event_interruptible_timeout(), originally defined in <linux/wait.h>
+ */
+
+#define wait_event_freezable(wq, condition) \
+({ \
+ int __ret; \
+ do { \
+ __ret = wait_event_interruptible(wq, \
+ (condition) || freezing(current)); \
+ if (__ret && !freezing(current)) \
+ break; \
+ else if (!(condition)) \
+ __ret = -ERESTARTSYS; \
+ } while (try_to_freeze()); \
+ __ret; \
+})
+
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ do { \
+ __ret = wait_event_interruptible_timeout(wq, \
+ (condition) || freezing(current), \
+ __ret); \
+ } while (try_to_freeze()); \
+ __ret; \
+})
+#else /* CONFIG_PM */
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) {}
@@ -143,6 +174,13 @@ static inline void freezer_do_not_count(
static inline void freezer_count(void) {}
static inline int freezer_should_skip(struct task_struct *p) { return 0; }
static inline void set_freezable(void) {}
-#endif
+
+#define wait_event_freezable(wq, condition) \
+ wait_event_interruptible(wq, condition)
+
+#define wait_event_freezable_timeout(wq, condition, timeout) \
+ wait_event_interruptible_timeout(wq, condition, timeout)
+
+#endif /* CONFIG_PM */
#endif /* FREEZER_H_INCLUDED */
Index: linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/gameport/gameport.c
+++ linux-2.6.22-rc6-mm1/drivers/input/gameport/gameport.c
@@ -448,9 +448,8 @@ static int gameport_thread(void *nothing
set_freezable();
do {
gameport_handle_event();
- wait_event_interruptible(gameport_wait,
+ wait_event_freezable(gameport_wait,
kthread_should_stop() || !list_empty(&gameport_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "gameport: kgameportd exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/serio/serio.c
+++ linux-2.6.22-rc6-mm1/drivers/input/serio/serio.c
@@ -387,9 +387,8 @@ static int serio_thread(void *nothing)
set_freezable();
do {
serio_handle_event();
- wait_event_interruptible(serio_wait,
+ wait_event_freezable(serio_wait,
kthread_should_stop() || !list_empty(&serio_event_list));
- try_to_freeze();
} while (!kthread_should_stop());
printk(KERN_DEBUG "serio: kseriod exiting\n");
Index: linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/input/touchscreen/ucb1400_ts.c
+++ linux-2.6.22-rc6-mm1/drivers/input/touchscreen/ucb1400_ts.c
@@ -334,10 +334,9 @@ static int ucb1400_ts_thread(void *_ucb)
timeout = msecs_to_jiffies(10);
}
- wait_event_interruptible_timeout(ucb->ts_wait,
+ wait_event_freezable_timeout(ucb->ts_wait,
ucb->irq_pending || ucb->ts_restart || kthread_should_stop(),
timeout);
- try_to_freeze();
}
/* Send the "pen off" if we are stopping with the pen still active */
Index: linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/media/dvb/dvb-core/dvb_frontend.c
+++ linux-2.6.22-rc6-mm1/drivers/media/dvb/dvb-core/dvb_frontend.c
@@ -528,7 +528,8 @@ static int dvb_frontend_thread(void *dat
up(&fepriv->sem); /* is locked when we enter the thread... */
restart:
timeout = wait_event_interruptible_timeout(fepriv->wait_queue,
- dvb_frontend_should_wakeup(fe) || kthread_should_stop(),
+ dvb_frontend_should_wakeup(fe) || kthread_should_stop()
+ || freezing(current),
fepriv->delay);
if (kthread_should_stop() || dvb_frontend_is_exiting(fe)) {
Index: linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/core/hub.c
+++ linux-2.6.22-rc6-mm1/drivers/usb/core/hub.c
@@ -2729,10 +2729,9 @@ static int hub_thread(void *__unused)
set_freezable();
do {
hub_events();
- wait_event_interruptible(khubd_wait,
+ wait_event_freezable(khubd_wait,
!list_empty(&hub_event_list) ||
kthread_should_stop());
- try_to_freeze();
} while (!kthread_should_stop() || !list_empty(&hub_event_list));
pr_debug("%s: khubd exiting\n", usbcore_name);
Index: linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.22-rc6-mm1.orig/drivers/usb/storage/usb.c
+++ linux-2.6.22-rc6-mm1/drivers/usb/storage/usb.c
@@ -913,12 +913,9 @@ static int usb_stor_scan_thread(void * _
if (delay_use > 0) {
printk(KERN_DEBUG "usb-storage: waiting for device "
"to settle before scanning\n");
-retry:
- wait_event_interruptible_timeout(us->delay_wait,
+ wait_event_freezable_timeout(us->delay_wait,
test_bit(US_FLIDX_DISCONNECTING, &us->flags),
delay_use * HZ);
- if (try_to_freeze())
- goto retry;
}
/* If the device is still connected, perform the scanning */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently
2007-07-11 22:17 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Rafael J. Wysocki
2007-07-12 10:38 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently (updated) Rafael J. Wysocki
@ 2007-07-13 23:43 ` Pavel Machek
2007-07-14 9:20 ` Rafael J. Wysocki
1 sibling, 1 reply; 20+ messages in thread
From: Pavel Machek @ 2007-07-13 23:43 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
Hi!
> There is the problem with try_to_freeze_tasks() that it always loops until the
> timeout expires, even if it is certain to fail much earlier. Namely, if there
> are uninterruptible tasks waiting for some frozen tasks to let them continue,
> try_to_freeze_tasks() will certainly fail and it shouldn't waste time in that
> cases.
>
> To detect such situations, we can check if the number of tasks that haven't
> frozen yet changes between subsequent iterations of the main loop in
> try_to_freeze_tasks(). If this number hasn't been changing for sufficiently
> long time (say, 250 ms), then most probably some uninterruptible tasks are
> blocked by some frozen tasks and we should break out of this stalemate. Thus,
> it seems reasonable to thaw the tasks that have already been frozen without
> clearing the freeze requests of the tasks that are refusing to freeze. This
> way, if these tasks are really blocked by the frozen ones, they will get extra
> chance to freeze themselves after we have thawed the other tasks and before we
> request those tasks to freeze again. Next, the freezing loop can be repeated
> and so on, until all tasks are frozen or the timeout expires.
I still don't quite like this patch... it is kind of trick we should
not have to play. Can we at least get confirmation it helps in the
FUSE case?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently
2007-07-13 23:43 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Pavel Machek
@ 2007-07-14 9:20 ` Rafael J. Wysocki
0 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2007-07-14 9:20 UTC (permalink / raw)
To: Pavel Machek; +Cc: Matthew Garrett, pm list, Oleg Nesterov, Miklos Szeredi
On Saturday, 14 July 2007 01:43, Pavel Machek wrote:
> Hi!
>
> > There is the problem with try_to_freeze_tasks() that it always loops until the
> > timeout expires, even if it is certain to fail much earlier. Namely, if there
> > are uninterruptible tasks waiting for some frozen tasks to let them continue,
> > try_to_freeze_tasks() will certainly fail and it shouldn't waste time in that
> > cases.
> >
> > To detect such situations, we can check if the number of tasks that haven't
> > frozen yet changes between subsequent iterations of the main loop in
> > try_to_freeze_tasks(). If this number hasn't been changing for sufficiently
> > long time (say, 250 ms), then most probably some uninterruptible tasks are
> > blocked by some frozen tasks and we should break out of this stalemate. Thus,
> > it seems reasonable to thaw the tasks that have already been frozen without
> > clearing the freeze requests of the tasks that are refusing to freeze. This
> > way, if these tasks are really blocked by the frozen ones, they will get extra
> > chance to freeze themselves after we have thawed the other tasks and before we
> > request those tasks to freeze again. Next, the freezing loop can be repeated
> > and so on, until all tasks are frozen or the timeout expires.
>
> I still don't quite like this patch... it is kind of trick we should
> not have to play. Can we at least get confirmation it helps in the
> FUSE case?
Well, anyone having problems with FUSE vs the freezer please check. :-)
I think I'll send the series to Andrew without this patch. Would that be OK?
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2007-07-14 9:20 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-11 22:06 [RFC][PATCH -mm 0/7] Freezer update (updated) Rafael J. Wysocki
2007-07-11 22:08 ` [RFC][PATCH -mm 1/7] Freezer: Document relationship with memory shrinking Rafael J. Wysocki
2007-07-11 22:10 ` [RFC][PATCH -mm 2/7] Freezer: Do not sync filesystems from freeze_processes Rafael J. Wysocki
2007-07-11 22:42 ` Pavel Machek
2007-07-11 22:12 ` [RFC][PATCH -mm 3/7] Freezer: Prevent new tasks from inheriting TIF_FREEZE set Rafael J. Wysocki
2007-07-11 22:25 ` Nigel Cunningham
2007-07-11 22:13 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros Rafael J. Wysocki
2007-07-11 22:56 ` Pavel Machek
2007-07-12 10:33 ` Rafael J. Wysocki
2007-07-11 23:02 ` Oleg Nesterov
2007-07-12 10:30 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated) Rafael J. Wysocki
2007-07-12 12:23 ` [RFC][PATCH -mm 4/7] Freezer: Introduce freezer-firendly waiting macros (updated 2x) Rafael J. Wysocki
2007-07-11 22:14 ` [RFC][PATCH -mm 5/7] Freezer: Do not send signals to kernel threads (updated) Rafael J. Wysocki
2007-07-11 22:16 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose Rafael J. Wysocki
2007-07-11 22:58 ` Pavel Machek
2007-07-12 10:39 ` [RFC][PATCH -mm 6/7] Freezer: Be more verbose (updated) Rafael J. Wysocki
2007-07-11 22:17 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Rafael J. Wysocki
2007-07-12 10:38 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently (updated) Rafael J. Wysocki
2007-07-13 23:43 ` [RFC][PATCH -mm 7/7] Freezer: Use freezing timeout more efficiently Pavel Machek
2007-07-14 9:20 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox