[PATCH -mm 0/5] Freezer update

[PATCH -mm 0/5] Freezer update

[Rafael J. Wysocki]

Hi,

The patches in these series update the freezer to eliminate some existing
shortcomings, so please consider 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

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

[PATCH -mm 1/5] Freezer: Document relationship with memory shrinking

[Rafael J. Wysocki]

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

[PATCH -mm 2/5] Freezer: Do not sync filesystems from freeze_processes

[Rafael J. Wysocki]

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 path sync filesystems independently of the freezer.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 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
+++ linux-2.6.22-rc6-mm1/kernel/power/disk.c
@@ -312,6 +312,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
+++ linux-2.6.22-rc6-mm1/kernel/power/main.c
@@ -20,6 +20,7 @@
 #include <linux/resume-trace.h>
 #include <linux/freezer.h>
 #include <linux/vmstat.h>
+#include <linux/syscalls.h>
 
 #include "power.h"
 
@@ -228,9 +229,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
+++ linux-2.6.22-rc6-mm1/kernel/power/process.c
@@ -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
+++ linux-2.6.22-rc6-mm1/kernel/power/user.c
@@ -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();

[PATCH -mm 3/5] Freezer: Prevent new tasks from inheriting TIF_FREEZE set

[Rafael J. Wysocki]

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)

[PATCH -mm 4/5] Freezer: Introduce freezer-firendly waiting macros

[Rafael J. Wysocki]

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>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 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 */

[PATCH -mm 5/5] Freezer: Do not send signals to kernel threads

[Rafael J. Wysocki]

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

Re: [PATCH -mm 1/5] Freezer: Document relationship with memory shrinking

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi.

On Wednesday 18 July 2007 17:30:44 Rafael J. Wysocki wrote:
> 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>

Acked-by: Nigel Cunningham <nigel@nigel.suspend2.net>

-- 
See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH -mm 3/5] Freezer: Prevent new tasks from inheriting TIF_FREEZE set

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

Hi.

On Wednesday 18 July 2007 17:33:31 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>
Acked-by: Nigel Cunningham <nigel@nigel.suspend2.net>

Regards,

Nigel

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH -mm 0/5] Freezer update

[Nigel Cunningham]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

Hi.

On Wednesday 18 July 2007 17:29:30 Rafael J. Wysocki wrote:
> Hi,
> 
> The patches in these series update the freezer to eliminate some existing
> shortcomings, so please consider 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

I've sent acks for a couple, but not all.

No ack for the sys_sync removal because I think this is just a work around for 
fuse issues, and fuse should be fixed (getting fuse tasks frozen 
post-sys_sync makes far more sense to me).

No ack for the other patches because I haven't put the time into considering 
them.

Regards,

Nigel
-- 
See http://www.tuxonice.net for Howtos, FAQs, mailing
lists, wiki and bugzilla info.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH -mm 0/5] Freezer update

[Andrew Morton]

On Wed, 18 Jul 2007 09:29:30 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> The patches in these series update the freezer to eliminate some existing
> shortcomings,

argh.

I have a backlog of maybe 300 patches here which I am cheerfully ignoring
while concentrating on preventing 2.6.23 from being less of a disaster than
it has already been.

Please, stop writing patches.  Maybe do something to help get 2.6.23 off
its back.  Like, go review some of the code which people are cheerfully
merging five minutes after having written it.

> so please consider them as 2.6.23 material.

The door for new 2.6.23 material shut two weeks ago.  Here, at least.

Later, OK?

Re: [PATCH -mm 0/5] Freezer update

[Rafael J. Wysocki]

On Wednesday, 18 July 2007 10:03, Andrew Morton wrote:
> On Wed, 18 Jul 2007 09:29:30 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > The patches in these series update the freezer to eliminate some existing
> > shortcomings,
> 
> argh.
> 
> I have a backlog of maybe 300 patches here which I am cheerfully ignoring
> while concentrating on preventing 2.6.23 from being less of a disaster than
> it has already been.
> 
> Please, stop writing patches.  Maybe do something to help get 2.6.23 off
> its back.  Like, go review some of the code which people are cheerfully
> merging five minutes after having written it.
> 
> > so please consider them as 2.6.23 material.
> 
> The door for new 2.6.23 material shut two weeks ago.  Here, at least.
> 
> Later, OK?

OK, and sorry for the trouble.

Re: [PATCH -mm 0/5] Freezer update

[Josh Boyer]

On 7/18/07, Andrew Morton <akpm@linux-foundation.org> wrote:

> > so please consider them as 2.6.23 material.
>
> The door for new 2.6.23 material shut two weeks ago.  Here, at least.

So do you have a general guideline as to when you'd like patches sent
to you for the next merge window?  The releases are fairly
predictable, so perhaps something like one of the later -rc candidates
for the upcoming release?

E.g. patches for 2.6.24 should be to you by 2.6.23-rc<N>?

Of course, this is assuming they go through you.  I'm not trying to
complicate things.  Just trying to get an understanding of how you'd
like it to work.

josh

Re: [PATCH -mm 0/5] Freezer update

[Andrew Morton]

On Thu, 19 Jul 2007 11:30:54 -0500 "Josh Boyer" <jwboyer@gmail.com> wrote:

> On 7/18/07, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > > so please consider them as 2.6.23 material.
> >
> > The door for new 2.6.23 material shut two weeks ago.  Here, at least.
> 
> So do you have a general guideline as to when you'd like patches sent
> to you for the next merge window?  The releases are fairly
> predictable, so perhaps something like one of the later -rc candidates
> for the upcoming release?
> 
> E.g. patches for 2.6.24 should be to you by 2.6.23-rc<N>?

That sounds about right.  But nothing is cast in stone here: one needs
to take into account the urgency/desirability of the change, the perceived
benefit and risk, etc.

I do prefer that material has been out in at least one -mm release before
it goes mainline.

Re: [PATCH -mm 0/5] Freezer update

[Pavel Machek]

Hi!

> > > The patches in these series update the freezer to eliminate some existing
> > > shortcomings,
> > 
> > argh.
> > 
> > I have a backlog of maybe 300 patches here which I am cheerfully ignoring
> > while concentrating on preventing 2.6.23 from being less of a disaster than
> > it has already been.
> > 
> > Please, stop writing patches.  Maybe do something to help get 2.6.23 off
> > its back.  Like, go review some of the code which people are cheerfully
> > merging five minutes after having written it.
> > 
> > > so please consider them as 2.6.23 material.
> > 
> > The door for new 2.6.23 material shut two weeks ago.  Here, at least.
> > 
> > Later, OK?
> 
> OK, and sorry for the trouble.

Actually, 1 and 2 from that series seem like .23 material to me. 1 is
(harmless docs update, and 2 is trivial bugfix for fuse.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html