[PATCH -mm 0/6] Freezer changes

[PATCH -mm 0/6] Freezer changes

[Rafael J. Wysocki]

Hi,

The following series of patches contains modifications of the task freezer that
harden it and prepare it to be used in the CPU hotplug.

Greetings,
Rafael

[PATCH -mm 1/6] Freezer: Read PF_BORROWED_MM in a nonracy way

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/power/process.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -8,6 +8,7 @@
 
 #undef DEBUG
 
+#include <linux/sched.h>
 #include <linux/smp_lock.h>
 #include <linux/interrupt.h>
 #include <linux/suspend.h>
@@ -92,7 +93,12 @@ static void cancel_freezing(struct task_
 
 static inline int is_user_space(struct task_struct *p)
 {
-	return p->mm && !(p->flags & PF_BORROWED_MM);
+	int ret;
+
+	task_lock(p);
+	ret = p->mm && !(p->flags & PF_BORROWED_MM);
+	task_unlock(p);
+	return ret;
 }
 
 static unsigned int try_to_freeze_tasks(int freeze_user_space)

[PATCH -mm 2/6] Freezer: Fix memory ordering in refrigerator

[Rafael J. Wysocki]

From: Oleg Nesterov <oleg@tv-sign.ru>

refrigerator() can miss a wakeup, "wait event" loop needs a proper memory
ordering.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/power/process.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -47,8 +47,10 @@ void refrigerator(void)
 	recalc_sigpending(); /* We sent fake signal, clean it up */
 	spin_unlock_irq(&current->sighand->siglock);
 
-	while (frozen(current)) {
-		current->state = TASK_UNINTERRUPTIBLE;
+	for (;;) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		if (!frozen(current))
+			break;
 		schedule();
 	}
 	pr_debug("%s left refrigerator\n", current->comm);

[PATCH -mm 3/6] Freezer: Close theoretical race between refrigerator and thaw_tasks

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

If the freezing of tasks fails and a task is preempted in refrigerator() before
calling frozen_process(), then thaw_tasks() may run before this task is frozen.
In that case the task will freeze and no one will thaw it.

To fix this race we can call freezing(current) in refrigerator() along with
frozen_process(current) under the task_lock() which also should be taken in
the error path of try_to_freeze_tasks() as well as in thaw_process().  Moreover,
if thaw_process() additionally clears TIF_FREEZE for tasks that are not frozen,
we can be sure that all tasks are thawed and there are no pending "freeze"
requests after thaw_tasks() has run.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/freezer.h |    4 ++++
 kernel/power/process.c  |   12 +++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -40,11 +40,15 @@ static inline void do_not_freeze(struct 
  */
 static inline int thaw_process(struct task_struct *p)
 {
+	task_lock(p);
 	if (frozen(p)) {
 		p->flags &= ~PF_FROZEN;
+		task_unlock(p);
 		wake_up_process(p);
 		return 1;
 	}
+	clear_tsk_thread_flag(p, TIF_FREEZE);
+	task_unlock(p);
 	return 0;
 }
 
Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -39,10 +39,18 @@ void refrigerator(void)
 	/* Hmm, should we be allowed to suspend when there are realtime
 	   processes around? */
 	long save;
+
+	task_lock(current);
+	if (freezing(current)) {
+		frozen_process(current);
+		task_unlock(current);
+	} else {
+		task_unlock(current);
+		return;
+	}
 	save = current->state;
 	pr_debug("%s entered refrigerator\n", current->comm);
 
-	frozen_process(current);
 	spin_lock_irq(&current->sighand->siglock);
 	recalc_sigpending(); /* We sent fake signal, clean it up */
 	spin_unlock_irq(&current->sighand->siglock);
@@ -159,10 +167,12 @@ static unsigned int try_to_freeze_tasks(
 			if (is_user_space(p) == !freeze_user_space)
 				continue;
 
+			task_lock(p);
 			if (freezeable(p) && !frozen(p))
 				printk(KERN_ERR " %s\n", p->comm);
 
 			cancel_freezing(p);
+			task_unlock(p);
 		} while_each_thread(g, p);
 		read_unlock(&tasklist_lock);
 	}

[PATCH -mm 4/6] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Rafael J. Wysocki]

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
required.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/rcutorture.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/kernel/rcutorture.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/rcutorture.c	2007-02-22 23:51:54.000000000 +0100
+++ linux-2.6.20-mm2/kernel/rcutorture.c	2007-02-22 23:55:12.000000000 +0100
@@ -46,6 +46,7 @@
 #include <linux/byteorder/swabb.h>
 #include <linux/stat.h>
 #include <linux/srcu.h>
+#include <linux/freezer.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
@@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
 
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
 	set_user_nice(current, 19);
-	current->flags |= PF_NOFREEZE;
 
 	do {
 		schedule_timeout_uninterruptible(1);
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
 		}
 		rcu_torture_current_version++;
 		oldbatch = cur_ops->completed();
+		try_to_freeze();
 	} while (!kthread_should_stop() && !fullstop);
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
 	while (!kthread_should_stop())

[PATCH -mm 5/6] Freezer: Remove PF_NOFREEZE from bluetooth threads

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Remove PF_NOFREEZE from the bluetooth threads, adding try_to_freeze() calls as
required.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/bnep/core.c   |    6 ++++--
 net/bluetooth/cmtp/core.c   |    4 +++-
 net/bluetooth/hidp/core.c   |    4 +++-
 net/bluetooth/rfcomm/core.c |    4 +++-
 4 files changed, 13 insertions(+), 5 deletions(-)

Index: linux-2.6.20-mm2/net/bluetooth/bnep/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/bnep/core.c	2007-02-23 00:26:58.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/bnep/core.c	2007-02-23 00:30:47.000000000 +0100
@@ -39,6 +39,7 @@
 #include <linux/errno.h>
 #include <linux/smp_lock.h>
 #include <linux/net.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/socket.h>
@@ -473,11 +474,12 @@ static int bnep_session(void *arg)
 
 	daemonize("kbnepd %s", dev->name);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(sk->sk_sleep, &wait);
 	while (!atomic_read(&s->killed)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		// RX
Index: linux-2.6.20-mm2/net/bluetooth/cmtp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/cmtp/core.c	2007-02-23 00:26:58.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/cmtp/core.c	2007-02-23 00:31:01.000000000 +0100
@@ -34,6 +34,7 @@
 #include <linux/ioctl.h>
 #include <linux/file.h>
 #include <linux/init.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/isdn/capilli.h>
@@ -287,11 +288,12 @@ static int cmtp_session(void *arg)
 
 	daemonize("kcmtpd_ctr_%d", session->num);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(sk->sk_sleep, &wait);
 	while (!atomic_read(&session->terminate)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		if (sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/hidp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/hidp/core.c	2007-02-23 00:26:58.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/hidp/core.c	2007-02-23 00:31:17.000000000 +0100
@@ -35,6 +35,7 @@
 #include <linux/file.h>
 #include <linux/init.h>
 #include <linux/wait.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/input.h>
@@ -547,13 +548,14 @@ static int hidp_session(void *arg)
 
 	daemonize("khidpd_%04x%04x", vendor, product);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&ctrl_wait, current);
 	init_waitqueue_entry(&intr_wait, current);
 	add_wait_queue(ctrl_sk->sk_sleep, &ctrl_wait);
 	add_wait_queue(intr_sk->sk_sleep, &intr_wait);
 	while (!atomic_read(&session->terminate)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		if (ctrl_sk->sk_state != BT_CONNECTED || intr_sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/rfcomm/core.c	2007-02-23 00:26:58.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c	2007-02-23 00:31:43.000000000 +0100
@@ -37,6 +37,7 @@
 #include <linux/device.h>
 #include <linux/net.h>
 #include <linux/mutex.h>
+#include <linux/freezer.h>
 
 #include <net/sock.h>
 #include <asm/uaccess.h>
@@ -1851,6 +1852,8 @@ static void rfcomm_worker(void)
 	BT_DBG("");
 
 	while (!atomic_read(&terminate)) {
+		try_to_freeze();
+
 		if (!test_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event)) {
 			/* No pending events. Let's sleep.
 			 * Incoming connections and data will wake us up. */
@@ -1937,7 +1940,6 @@ static int rfcomm_run(void *unused)
 
 	daemonize("krfcommd");
 	set_user_nice(current, -10);
-	current->flags |= PF_NOFREEZE;
 
 	BT_DBG("");
 

[PATCH -mm 6/6] Freezer: Add try_to_freeze calls to all kernel threads

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Add try_to_freeze() calls to the remaining kernel threads that do not call
try_to_freeze() already, although they set PF_NOFREEZE.

In the future we are going to replace PF_NOFREEZE with a set of flags that will
be set to indicate in which situations the task should not be frozen (for
example, there can be a task that should be frozen for the CPU hotplugging and
should not be frozen for the system suspend).  For this reason every kernel
thread should be able to freeze itself (ie. call try_to_freeze()), so that it
can be frozen whenever necessary.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 arch/i386/kernel/apm.c              |    2 ++
 drivers/block/loop.c                |    2 ++
 drivers/char/apm-emulation.c        |    3 +++
 drivers/ieee1394/ieee1394_core.c    |    3 +++
 drivers/md/md.c                     |    2 ++
 drivers/mmc/card/queue.c            |    3 +++
 drivers/mtd/mtd_blkdevs.c           |    3 +++
 drivers/scsi/libsas/sas_scsi_host.c |    3 +++
 drivers/scsi/scsi_error.c           |    3 +++
 drivers/usb/storage/usb.c           |    2 ++
 kernel/softirq.c                    |    2 ++
 kernel/softlockup.c                 |    2 ++
 kernel/workqueue.c                  |    3 +--
 13 files changed, 31 insertions(+), 2 deletions(-)

Index: linux-2.6.20-mm2/arch/i386/kernel/apm.c
===================================================================
--- linux-2.6.20-mm2.orig/arch/i386/kernel/apm.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/arch/i386/kernel/apm.c	2007-02-23 00:34:25.000000000 +0100
@@ -227,6 +227,7 @@
 #include <linux/dmi.h>
 #include <linux/suspend.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -1402,6 +1403,7 @@ static void apm_mainloop(void)
 	add_wait_queue(&apm_waitqueue, &wait);
 	set_current_state(TASK_INTERRUPTIBLE);
 	for (;;) {
+		try_to_freeze();
 		schedule_timeout(APM_CHECK_TIMEOUT);
 		if (kthread_should_stop())
 			break;
Index: linux-2.6.20-mm2/drivers/md/md.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/md/md.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/md/md.c	2007-02-23 00:34:25.000000000 +0100
@@ -4513,6 +4513,8 @@ static int md_thread(void * arg)
 			 || kthread_should_stop(),
 			 thread->timeout);
 
+		try_to_freeze();
+
 		clear_bit(THREAD_WAKEUP, &thread->flags);
 
 		thread->run(thread->mddev);
Index: linux-2.6.20-mm2/drivers/mmc/card/queue.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mmc/card/queue.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mmc/card/queue.c	2007-02-23 00:34:25.000000000 +0100
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/blkdev.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
@@ -71,6 +72,8 @@ static int mmc_queue_thread(void *d)
 	do {
 		struct request *req = NULL;
 
+		try_to_freeze();
+
 		spin_lock_irq(q->queue_lock);
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!blk_queue_plugged(q))
Index: linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mtd/mtd_blkdevs.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c	2007-02-23 00:34:25.000000000 +0100
@@ -20,6 +20,7 @@
 #include <linux/hdreg.h>
 #include <linux/init.h>
 #include <linux/mutex.h>
+#include <linux/freezer.h>
 #include <asm/uaccess.h>
 
 static LIST_HEAD(blktrans_majors);
@@ -113,6 +114,8 @@ static int mtd_blktrans_thread(void *arg
 			schedule();
 			remove_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
 
+			try_to_freeze();
+
 			spin_lock_irq(rq->queue_lock);
 
 			continue;
Index: linux-2.6.20-mm2/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/usb/storage/usb.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/usb/storage/usb.c	2007-02-23 00:34:25.000000000 +0100
@@ -304,6 +304,8 @@ static int usb_stor_control_thread(void 
 	current->flags |= PF_NOFREEZE;
 
 	for(;;) {
+		try_to_freeze();
+
 		US_DEBUGP("*** thread sleeping.\n");
 		if(down_interruptible(&us->sema))
 			break;
Index: linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/ieee1394/ieee1394_core.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c	2007-02-23 00:34:25.000000000 +0100
@@ -35,6 +35,7 @@
 #include <linux/kthread.h>
 #include <linux/preempt.h>
 #include <linux/time.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 #include <asm/byteorder.h>
@@ -1081,6 +1082,8 @@ static int hpsbpkt_thread(void *__hi)
 			complete_routine(complete_data);
 		}
 
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!skb_peek(&hpsbpkt_queue))
 			schedule();
Index: linux-2.6.20-mm2/drivers/char/apm-emulation.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/char/apm-emulation.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/char/apm-emulation.c	2007-02-23 00:34:25.000000000 +0100
@@ -27,6 +27,7 @@
 #include <linux/completion.h>
 #include <linux/kthread.h>
 #include <linux/delay.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 
@@ -539,6 +540,8 @@ static int kapmd(void *arg)
 		apm_event_t event;
 		int ret;
 
+		try_to_freeze();
+
 		wait_event_interruptible(kapmd_wait,
 				!queue_empty(&kapmd_queue) || kthread_should_stop());
 
Index: linux-2.6.20-mm2/drivers/block/loop.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/block/loop.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/block/loop.c	2007-02-23 00:34:25.000000000 +0100
@@ -74,6 +74,7 @@
 #include <linux/highmem.h>
 #include <linux/gfp.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <asm/uaccess.h>
 
@@ -580,6 +581,7 @@ static int loop_thread(void *data)
 	set_user_nice(current, -20);
 
 	while (!kthread_should_stop() || lo->lo_bio) {
+		try_to_freeze();
 
 		wait_event_interruptible(lo->lo_event,
 				lo->lo_bio || kthread_should_stop());
Index: linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/libsas/sas_scsi_host.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c	2007-02-23 00:34:25.000000000 +0100
@@ -39,6 +39,7 @@
 #include <linux/err.h>
 #include <linux/blkdev.h>
 #include <linux/scatterlist.h>
+#include <linux/freezer.h>
 
 /* ---------- SCSI Host glue ---------- */
 
@@ -875,6 +876,8 @@ static int sas_queue_thread(void *_sas_h
 	complete(&queue_th_comp);
 
 	while (1) {
+		try_to_freeze();
+
 		down_interruptible(&core->queue_thread_sema);
 		sas_queue(sas_ha);
 		if (core->queue_thread_kill)
Index: linux-2.6.20-mm2/drivers/scsi/scsi_error.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/scsi_error.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/scsi_error.c	2007-02-23 00:34:25.000000000 +0100
@@ -24,6 +24,7 @@
 #include <linux/interrupt.h>
 #include <linux/blkdev.h>
 #include <linux/delay.h>
+#include <linux/freezer.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1536,6 +1537,8 @@ int scsi_error_handler(void *data)
 	 */
 	set_current_state(TASK_INTERRUPTIBLE);
 	while (!kthread_should_stop()) {
+		try_to_freeze();
+
 		if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
 		    shost->host_failed != shost->host_busy) {
 			SCSI_LOG_ERROR_RECOVERY(1,
Index: linux-2.6.20-mm2/kernel/softlockup.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softlockup.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softlockup.c	2007-02-23 00:34:25.000000000 +0100
@@ -13,6 +13,7 @@
 #include <linux/kthread.h>
 #include <linux/notifier.h>
 #include <linux/module.h>
+#include <linux/freezer.h>
 
 static DEFINE_SPINLOCK(print_lock);
 
@@ -93,6 +94,7 @@ static int watchdog(void * __bind_cpu)
 	 * debug-printout triggers in softlockup_tick().
 	 */
 	while (!kthread_should_stop()) {
+		try_to_freeze();
 		set_current_state(TASK_INTERRUPTIBLE);
 		touch_softlockup_watchdog();
 		schedule();
Index: linux-2.6.20-mm2/kernel/softirq.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softirq.c	2007-02-22 23:48:52.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softirq.c	2007-02-23 00:34:25.000000000 +0100
@@ -18,6 +18,7 @@
 #include <linux/rcupdate.h>
 #include <linux/smp.h>
 #include <linux/tick.h>
+#include <linux/freezer.h>
 
 #include <asm/irq.h>
 /*
@@ -494,6 +495,7 @@ static int ksoftirqd(void * __bind_cpu)
 	set_current_state(TASK_INTERRUPTIBLE);
 
 	while (!kthread_should_stop()) {
+		try_to_freeze();
 		preempt_disable();
 		if (!local_softirq_pending()) {
 			preempt_enable_no_resched();

Re: [PATCH -mm 2/6] Freezer: Fix memory ordering in refrigerator

[Oleg Nesterov]

On 02/26, Rafael J. Wysocki wrote:
>
> From: Oleg Nesterov <oleg@tv-sign.ru>
> 
> refrigerator() can miss a wakeup, "wait event" loop needs a proper memory
> ordering.
> 
> Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  kernel/power/process.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.20-mm2/kernel/power/process.c
> ===================================================================
> --- linux-2.6.20-mm2.orig/kernel/power/process.c
> +++ linux-2.6.20-mm2/kernel/power/process.c
> @@ -47,8 +47,10 @@ void refrigerator(void)
>  	recalc_sigpending(); /* We sent fake signal, clean it up */
>  	spin_unlock_irq(&current->sighand->siglock);
>  
> -	while (frozen(current)) {
> -		current->state = TASK_UNINTERRUPTIBLE;
> +	for (;;) {
> +		set_current_state(TASK_UNINTERRUPTIBLE);
> +		if (!frozen(current))
> +			break;
>  		schedule();
>  	}
>  	pr_debug("%s left refrigerator\n", current->comm);

This one is already in -mm tree.

Oleg.

[PATCH -mm 0/7] Freezer changes (take 2)

[Rafael J. Wysocki]

Hi,

The following series of patches contains modifications of the task freezer that
harden it and prepare it to be used in the CPU hotplug.

Greetings,
Rafael

[PATCH -mm 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/power/process.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c	2007-02-25 14:02:55.000000000 +0100
+++ linux-2.6.20-mm2/kernel/power/process.c	2007-02-25 14:03:51.000000000 +0100
@@ -8,6 +8,7 @@
 
 #undef DEBUG
 
+#include <linux/sched.h>
 #include <linux/smp_lock.h>
 #include <linux/interrupt.h>
 #include <linux/suspend.h>
@@ -87,7 +88,12 @@ static void cancel_freezing(struct task_
 
 static inline int is_user_space(struct task_struct *p)
 {
-	return p->mm && !(p->flags & PF_BORROWED_MM);
+	int ret;
+
+	task_lock(p);
+	ret = p->mm && !(p->flags & PF_BORROWED_MM);
+	task_unlock(p);
+	return ret;
 }
 
 static unsigned int try_to_freeze_tasks(int freeze_user_space)

[PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

If the freezing of tasks fails and a task is preempted in refrigerator() before
calling frozen_process(), then thaw_tasks() may run before this task is frozen.
In that case the task will freeze and no one will thaw it.

To fix this race we can call freezing(current) in refrigerator() along with
frozen_process(current) under the task_lock() which also should be taken in
the error path of try_to_freeze_tasks() as well as in thaw_process().  Moreover,
if thaw_process() additionally clears TIF_FREEZE for tasks that are not frozen,
we can be sure that all tasks are thawed and there are no pending "freeze"
requests after thaw_tasks() has run.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/freezer.h |    4 ++++
 kernel/power/process.c  |   12 +++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -40,11 +40,15 @@ static inline void do_not_freeze(struct 
  */
 static inline int thaw_process(struct task_struct *p)
 {
+	task_lock(p);
 	if (frozen(p)) {
 		p->flags &= ~PF_FROZEN;
+		task_unlock(p);
 		wake_up_process(p);
 		return 1;
 	}
+	clear_tsk_thread_flag(p, TIF_FREEZE);
+	task_unlock(p);
 	return 0;
 }
 
Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -39,10 +39,18 @@ void refrigerator(void)
 	/* Hmm, should we be allowed to suspend when there are realtime
 	   processes around? */
 	long save;
+
+	task_lock(current);
+	if (freezing(current)) {
+		frozen_process(current);
+		task_unlock(current);
+	} else {
+		task_unlock(current);
+		return;
+	}
 	save = current->state;
 	pr_debug("%s entered refrigerator\n", current->comm);
 
-	frozen_process(current);
 	spin_lock_irq(&current->sighand->siglock);
 	recalc_sigpending(); /* We sent fake signal, clean it up */
 	spin_unlock_irq(&current->sighand->siglock);
@@ -159,10 +167,12 @@ static unsigned int try_to_freeze_tasks(
 			if (is_user_space(p) == !freeze_user_space)
 				continue;
 
+			task_lock(p);
 			if (freezeable(p) && !frozen(p))
 				printk(KERN_ERR " %s\n", p->comm);
 
 			cancel_freezing(p);
+			task_unlock(p);
 		} while_each_thread(g, p);
 		read_unlock(&tasklist_lock);
 	}

[PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Rafael J. Wysocki]

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
required.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/rcutorture.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.20-mm2/kernel/rcutorture.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/rcutorture.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/rcutorture.c	2007-02-25 12:49:23.000000000 +0100
@@ -46,6 +46,7 @@
 #include <linux/byteorder/swabb.h>
 #include <linux/stat.h>
 #include <linux/srcu.h>
+#include <linux/freezer.h>
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
@@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
 
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
 	set_user_nice(current, 19);
-	current->flags |= PF_NOFREEZE;
 
 	do {
 		schedule_timeout_uninterruptible(1);
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
 		}
 		rcu_torture_current_version++;
 		oldbatch = cur_ops->completed();
+		try_to_freeze();
 	} while (!kthread_should_stop() && !fullstop);
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
 	while (!kthread_should_stop())

[PATCH -mm 4/7] Freezer: Remove PF_NOFREEZE from bluetooth threads

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Remove PF_NOFREEZE from the bluetooth threads, adding try_to_freeze() calls as
required.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/bnep/core.c   |    4 +++-
 net/bluetooth/cmtp/core.c   |    4 +++-
 net/bluetooth/hidp/core.c   |    4 +++-
 net/bluetooth/rfcomm/core.c |    4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

Index: linux-2.6.20-mm2/net/bluetooth/bnep/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/bnep/core.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/bnep/core.c	2007-02-25 12:49:27.000000000 +0100
@@ -39,6 +39,7 @@
 #include <linux/errno.h>
 #include <linux/smp_lock.h>
 #include <linux/net.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/socket.h>
@@ -473,11 +474,12 @@ static int bnep_session(void *arg)
 
 	daemonize("kbnepd %s", dev->name);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(sk->sk_sleep, &wait);
 	while (!atomic_read(&s->killed)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		// RX
Index: linux-2.6.20-mm2/net/bluetooth/cmtp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/cmtp/core.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/cmtp/core.c	2007-02-25 12:49:27.000000000 +0100
@@ -34,6 +34,7 @@
 #include <linux/ioctl.h>
 #include <linux/file.h>
 #include <linux/init.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/isdn/capilli.h>
@@ -287,11 +288,12 @@ static int cmtp_session(void *arg)
 
 	daemonize("kcmtpd_ctr_%d", session->num);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(sk->sk_sleep, &wait);
 	while (!atomic_read(&session->terminate)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		if (sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/hidp/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/hidp/core.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/hidp/core.c	2007-02-25 12:49:27.000000000 +0100
@@ -35,6 +35,7 @@
 #include <linux/file.h>
 #include <linux/init.h>
 #include <linux/wait.h>
+#include <linux/freezer.h>
 #include <net/sock.h>
 
 #include <linux/input.h>
@@ -547,13 +548,14 @@ static int hidp_session(void *arg)
 
 	daemonize("khidpd_%04x%04x", vendor, product);
 	set_user_nice(current, -15);
-	current->flags |= PF_NOFREEZE;
 
 	init_waitqueue_entry(&ctrl_wait, current);
 	init_waitqueue_entry(&intr_wait, current);
 	add_wait_queue(ctrl_sk->sk_sleep, &ctrl_wait);
 	add_wait_queue(intr_sk->sk_sleep, &intr_wait);
 	while (!atomic_read(&session->terminate)) {
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 
 		if (ctrl_sk->sk_state != BT_CONNECTED || intr_sk->sk_state != BT_CONNECTED)
Index: linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c
===================================================================
--- linux-2.6.20-mm2.orig/net/bluetooth/rfcomm/core.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/net/bluetooth/rfcomm/core.c	2007-02-25 12:49:27.000000000 +0100
@@ -37,6 +37,7 @@
 #include <linux/device.h>
 #include <linux/net.h>
 #include <linux/mutex.h>
+#include <linux/freezer.h>
 
 #include <net/sock.h>
 #include <asm/uaccess.h>
@@ -1851,6 +1852,8 @@ static void rfcomm_worker(void)
 	BT_DBG("");
 
 	while (!atomic_read(&terminate)) {
+		try_to_freeze();
+
 		if (!test_bit(RFCOMM_SCHED_WAKEUP, &rfcomm_event)) {
 			/* No pending events. Let's sleep.
 			 * Incoming connections and data will wake us up. */
@@ -1937,7 +1940,6 @@ static int rfcomm_run(void *unused)
 
 	daemonize("krfcommd");
 	set_user_nice(current, -10);
-	current->flags |= PF_NOFREEZE;
 
 	BT_DBG("");
 

[PATCH -mm 5/7] Freezer: Add try_to_freeze calls to all kernel threads

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Add try_to_freeze() calls to the remaining kernel threads that do not call
try_to_freeze() already, although they set PF_NOFREEZE.

In the future we are going to replace PF_NOFREEZE with a set of flags that will
be set to indicate in which situations the task should not be frozen (for
example, there can be a task that should be frozen for the CPU hotplugging and
should not be frozen for the system suspend).  For this reason every kernel
thread should be able to freeze itself (ie. call try_to_freeze()), so that it
can be frozen whenever necessary.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 arch/i386/kernel/apm.c              |    2 ++
 drivers/block/loop.c                |    2 ++
 drivers/char/apm-emulation.c        |    3 +++
 drivers/ieee1394/ieee1394_core.c    |    3 +++
 drivers/md/md.c                     |    2 ++
 drivers/mmc/card/queue.c            |    3 +++
 drivers/mtd/mtd_blkdevs.c           |    3 +++
 drivers/scsi/libsas/sas_scsi_host.c |    3 +++
 drivers/scsi/scsi_error.c           |    3 +++
 drivers/usb/storage/usb.c           |    2 ++
 kernel/softirq.c                    |    2 ++
 kernel/softlockup.c                 |    2 ++
 12 files changed, 30 insertions(+)

Index: linux-2.6.20-mm2/arch/i386/kernel/apm.c
===================================================================
--- linux-2.6.20-mm2.orig/arch/i386/kernel/apm.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/arch/i386/kernel/apm.c	2007-02-25 12:49:30.000000000 +0100
@@ -227,6 +227,7 @@
 #include <linux/dmi.h>
 #include <linux/suspend.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 #include <asm/uaccess.h>
@@ -1402,6 +1403,7 @@ static void apm_mainloop(void)
 	add_wait_queue(&apm_waitqueue, &wait);
 	set_current_state(TASK_INTERRUPTIBLE);
 	for (;;) {
+		try_to_freeze();
 		schedule_timeout(APM_CHECK_TIMEOUT);
 		if (kthread_should_stop())
 			break;
Index: linux-2.6.20-mm2/drivers/md/md.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/md/md.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/md/md.c	2007-02-25 12:49:30.000000000 +0100
@@ -4513,6 +4513,8 @@ static int md_thread(void * arg)
 			 || kthread_should_stop(),
 			 thread->timeout);
 
+		try_to_freeze();
+
 		clear_bit(THREAD_WAKEUP, &thread->flags);
 
 		thread->run(thread->mddev);
Index: linux-2.6.20-mm2/drivers/mmc/card/queue.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mmc/card/queue.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mmc/card/queue.c	2007-02-25 12:49:30.000000000 +0100
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/blkdev.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <linux/mmc/card.h>
 #include <linux/mmc/host.h>
@@ -71,6 +72,8 @@ static int mmc_queue_thread(void *d)
 	do {
 		struct request *req = NULL;
 
+		try_to_freeze();
+
 		spin_lock_irq(q->queue_lock);
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!blk_queue_plugged(q))
Index: linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/mtd/mtd_blkdevs.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/mtd/mtd_blkdevs.c	2007-02-25 12:49:30.000000000 +0100
@@ -20,6 +20,7 @@
 #include <linux/hdreg.h>
 #include <linux/init.h>
 #include <linux/mutex.h>
+#include <linux/freezer.h>
 #include <asm/uaccess.h>
 
 static LIST_HEAD(blktrans_majors);
@@ -113,6 +114,8 @@ static int mtd_blktrans_thread(void *arg
 			schedule();
 			remove_wait_queue(&tr->blkcore_priv->thread_wq, &wait);
 
+			try_to_freeze();
+
 			spin_lock_irq(rq->queue_lock);
 
 			continue;
Index: linux-2.6.20-mm2/drivers/usb/storage/usb.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/usb/storage/usb.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/usb/storage/usb.c	2007-02-25 12:49:30.000000000 +0100
@@ -304,6 +304,8 @@ static int usb_stor_control_thread(void 
 	current->flags |= PF_NOFREEZE;
 
 	for(;;) {
+		try_to_freeze();
+
 		US_DEBUGP("*** thread sleeping.\n");
 		if(down_interruptible(&us->sema))
 			break;
Index: linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/ieee1394/ieee1394_core.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/ieee1394/ieee1394_core.c	2007-02-25 12:49:30.000000000 +0100
@@ -35,6 +35,7 @@
 #include <linux/kthread.h>
 #include <linux/preempt.h>
 #include <linux/time.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 #include <asm/byteorder.h>
@@ -1081,6 +1082,8 @@ static int hpsbpkt_thread(void *__hi)
 			complete_routine(complete_data);
 		}
 
+		try_to_freeze();
+
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!skb_peek(&hpsbpkt_queue))
 			schedule();
Index: linux-2.6.20-mm2/drivers/char/apm-emulation.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/char/apm-emulation.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/char/apm-emulation.c	2007-02-25 12:49:30.000000000 +0100
@@ -27,6 +27,7 @@
 #include <linux/completion.h>
 #include <linux/kthread.h>
 #include <linux/delay.h>
+#include <linux/freezer.h>
 
 #include <asm/system.h>
 
@@ -539,6 +540,8 @@ static int kapmd(void *arg)
 		apm_event_t event;
 		int ret;
 
+		try_to_freeze();
+
 		wait_event_interruptible(kapmd_wait,
 				!queue_empty(&kapmd_queue) || kthread_should_stop());
 
Index: linux-2.6.20-mm2/drivers/block/loop.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/block/loop.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/block/loop.c	2007-02-25 12:49:30.000000000 +0100
@@ -74,6 +74,7 @@
 #include <linux/highmem.h>
 #include <linux/gfp.h>
 #include <linux/kthread.h>
+#include <linux/freezer.h>
 
 #include <asm/uaccess.h>
 
@@ -580,6 +581,7 @@ static int loop_thread(void *data)
 	set_user_nice(current, -20);
 
 	while (!kthread_should_stop() || lo->lo_bio) {
+		try_to_freeze();
 
 		wait_event_interruptible(lo->lo_event,
 				lo->lo_bio || kthread_should_stop());
Index: linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/libsas/sas_scsi_host.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/libsas/sas_scsi_host.c	2007-02-25 12:49:30.000000000 +0100
@@ -39,6 +39,7 @@
 #include <linux/err.h>
 #include <linux/blkdev.h>
 #include <linux/scatterlist.h>
+#include <linux/freezer.h>
 
 /* ---------- SCSI Host glue ---------- */
 
@@ -875,6 +876,8 @@ static int sas_queue_thread(void *_sas_h
 	complete(&queue_th_comp);
 
 	while (1) {
+		try_to_freeze();
+
 		down_interruptible(&core->queue_thread_sema);
 		sas_queue(sas_ha);
 		if (core->queue_thread_kill)
Index: linux-2.6.20-mm2/drivers/scsi/scsi_error.c
===================================================================
--- linux-2.6.20-mm2.orig/drivers/scsi/scsi_error.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/drivers/scsi/scsi_error.c	2007-02-25 12:49:30.000000000 +0100
@@ -24,6 +24,7 @@
 #include <linux/interrupt.h>
 #include <linux/blkdev.h>
 #include <linux/delay.h>
+#include <linux/freezer.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1536,6 +1537,8 @@ int scsi_error_handler(void *data)
 	 */
 	set_current_state(TASK_INTERRUPTIBLE);
 	while (!kthread_should_stop()) {
+		try_to_freeze();
+
 		if ((shost->host_failed == 0 && shost->host_eh_scheduled == 0) ||
 		    shost->host_failed != shost->host_busy) {
 			SCSI_LOG_ERROR_RECOVERY(1,
Index: linux-2.6.20-mm2/kernel/softlockup.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softlockup.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softlockup.c	2007-02-25 12:49:30.000000000 +0100
@@ -13,6 +13,7 @@
 #include <linux/kthread.h>
 #include <linux/notifier.h>
 #include <linux/module.h>
+#include <linux/freezer.h>
 
 static DEFINE_SPINLOCK(print_lock);
 
@@ -93,6 +94,7 @@ static int watchdog(void * __bind_cpu)
 	 * debug-printout triggers in softlockup_tick().
 	 */
 	while (!kthread_should_stop()) {
+		try_to_freeze();
 		set_current_state(TASK_INTERRUPTIBLE);
 		touch_softlockup_watchdog();
 		schedule();
Index: linux-2.6.20-mm2/kernel/softirq.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/softirq.c	2007-02-25 12:07:15.000000000 +0100
+++ linux-2.6.20-mm2/kernel/softirq.c	2007-02-25 12:49:30.000000000 +0100
@@ -18,6 +18,7 @@
 #include <linux/rcupdate.h>
 #include <linux/smp.h>
 #include <linux/tick.h>
+#include <linux/freezer.h>
 
 #include <asm/irq.h>
 /*
@@ -494,6 +495,7 @@ static int ksoftirqd(void * __bind_cpu)
 	set_current_state(TASK_INTERRUPTIBLE);
 
 	while (!kthread_should_stop()) {
+		try_to_freeze();
 		preempt_disable();
 		if (!local_softirq_pending()) {
 			preempt_enable_no_resched();

[PATCH -mm 6/7] Freezer: Fix vfork problem

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Currently try_to_freeze_tasks() has to wait until all of the vforked processes
exit and for this reason every user can make it fail.  To fix this problem
we can introduce the additional process flag PF_FREEZER_SKIP to be used by
tasks that do not want to be counted as freezable by the freezer and want to
have TIF_FREEZE set nevertheless.  Then, this flag can be set by tasks using
sys_vfork() before they call wait_for_completion(&vfork) and cleared after they
have woken up.  After clearing it, the tasks should call try_to_freeze() as
soon as possible.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 include/linux/freezer.h |   48 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/sched.h   |    1 +
 kernel/fork.c           |    3 +++
 kernel/power/process.c  |   27 ++++++++-------------------
 4 files changed, 58 insertions(+), 21 deletions(-)

Index: linux-2.6.20-mm2/include/linux/sched.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/sched.h
+++ linux-2.6.20-mm2/include/linux/sched.h
@@ -1189,6 +1189,7 @@ static inline void put_task_struct(struc
 #define PF_SPREAD_SLAB	0x02000000	/* Spread some slab caches over cpuset */
 #define PF_MEMPOLICY	0x10000000	/* Non-default NUMA mempolicy */
 #define PF_MUTEX_TESTER	0x20000000	/* Thread belongs to the rt mutex tester */
+#define PF_FREEZER_SKIP	0x40000000	/* Freezer should not count it as freezeable */
 
 /*
  * Only the _current_ task can read/write to tsk->flags, but other
Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -75,7 +75,49 @@ static inline int try_to_freeze(void)
 		return 0;
 }
 
-extern void thaw_some_processes(int all);
+/*
+ * The PF_FREEZER_SKIP flag should be set by a vfork parent right before it
+ * calls wait_for_completion(&vfork) and reset right after it returns from this
+ * function.  Next, the parent should call try_to_freeze() to freeze itself
+ * appropriately in case the child has exited before the freezing of tasks is
+ * complete.  However, we don't want kernel threads to be frozen in unexpected
+ * places, so we allow them to block freeze_processes() instead or to set
+ * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork
+ * parents.  Fortunately, in the ____call_usermodehelper() case the parent won't
+ * really block freeze_processes(), since ____call_usermodehelper() (the child)
+ * does a little before exec/exit and it can't be frozen before waking up the
+ * parent.
+ */
+
+/*
+ * If the current task is a user space one, tell the freezer not to count it as
+ * freezable.
+ */
+static inline void freezer_do_not_count(void)
+{
+	if (current->mm)
+		current->flags |= PF_FREEZER_SKIP;
+}
+
+/*
+ * If the current task is a user space one, tell the freezer to count it as
+ * freezable again and try to freeze it.
+ */
+static inline void freezer_count(void)
+{
+	if (current->mm) {
+		current->flags &= ~PF_FREEZER_SKIP;
+		try_to_freeze();
+	}
+}
+
+/*
+ * Check if the task should be counted as freezeable by the freezer
+ */
+static inline int freezer_should_skip(struct task_struct *p)
+{
+	return !!(p->flags & PF_FREEZER_SKIP);
+}
 
 #else
 static inline int frozen(struct task_struct *p) { return 0; }
@@ -90,5 +132,7 @@ static inline void thaw_processes(void) 
 
 static inline int try_to_freeze(void) { return 0; }
 
-
+static inline void freezer_do_not_count(void) {}
+static inline void freezer_count(void) {}
+static inline int freezer_should_skip(struct task_struct *p) { return 0; }
 #endif
Index: linux-2.6.20-mm2/kernel/fork.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/fork.c
+++ linux-2.6.20-mm2/kernel/fork.c
@@ -50,6 +50,7 @@
 #include <linux/taskstats_kern.h>
 #include <linux/random.h>
 #include <linux/ptrace.h>
+#include <linux/freezer.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1393,7 +1394,9 @@ long do_fork(unsigned long clone_flags,
 		tracehook_report_clone_complete(clone_flags, nr, p);
 
 		if (clone_flags & CLONE_VFORK) {
+			freezer_do_not_count();
 			wait_for_completion(&vfork);
+			freezer_count();
 			tracehook_report_vfork_done(p, nr);
 		}
 	} else {
Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -127,22 +127,12 @@ static unsigned int try_to_freeze_tasks(
 				cancel_freezing(p);
 				continue;
 			}
-			if (is_user_space(p)) {
-				if (!freeze_user_space)
-					continue;
-
-				/* Freeze the task unless there is a vfork
-				 * completion pending
-				 */
-				if (!p->vfork_done)
-					freeze_process(p);
-			} else {
-				if (freeze_user_space)
-					continue;
+			if (is_user_space(p) == !freeze_user_space)
+				continue;
 
-				freeze_process(p);
-			}
-			todo++;
+			freeze_process(p);
+			if (!freezer_should_skip(p))
+				todo++;
 		} while_each_thread(g, p);
 		read_unlock(&tasklist_lock);
 		yield();			/* Yield is okay here */
@@ -168,7 +158,8 @@ static unsigned int try_to_freeze_tasks(
 				continue;
 
 			task_lock(p);
-			if (freezeable(p) && !frozen(p))
+			if (freezeable(p) && !frozen(p) &&
+			    !freezer_should_skip(p))
 				printk(KERN_ERR " %s\n", p->comm);
 
 			cancel_freezing(p);
@@ -217,9 +208,7 @@ static void thaw_tasks(int thaw_user_spa
 		if (is_user_space(p) == !thaw_user_space)
 			continue;
 
-		if (!thaw_process(p))
-			printk(KERN_WARNING " Strange, %s not stopped\n",
-				p->comm );
+		thaw_process(p);
 	} while_each_thread(g, p);
 	read_unlock(&tasklist_lock);
 }

[PATCH -mm 7/7] Freezer: Take kernel_execve into consideration

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Kernel threads can become userland processes by calling kernel_execve().  In
particular, this may happen right after try_to_freeze_tasks(FREEZER_USER_SPACE)
has returned, so try_to_freeze_tasks() needs to take userspace processes
into consideration even if it is called with FREEZER_KERNEL_THREADS.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 kernel/power/process.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -127,7 +127,7 @@ static unsigned int try_to_freeze_tasks(
 				cancel_freezing(p);
 				continue;
 			}
-			if (is_user_space(p) == !freeze_user_space)
+			if (freeze_user_space && !is_user_space(p))
 				continue;
 
 			freeze_process(p);
@@ -154,7 +154,7 @@ static unsigned int try_to_freeze_tasks(
 				TIMEOUT / HZ, todo);
 		read_lock(&tasklist_lock);
 		do_each_thread(g, p) {
-			if (is_user_space(p) == !freeze_user_space)
+			if (freeze_user_space && !is_user_space(p))
 				continue;
 
 			task_lock(p);

Re: [PATCH -mm 2/7] Freezer: Close theoretical race between refrigerator and thaw_tasks

[Pavel Machek]

On Thu 2007-03-01 16:08:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> If the freezing of tasks fails and a task is preempted in refrigerator() before
> calling frozen_process(), then thaw_tasks() may run before this task is frozen.
> In that case the task will freeze and no one will thaw it.
> 
> To fix this race we can call freezing(current) in refrigerator() along with
> frozen_process(current) under the task_lock() which also should be taken in
> the error path of try_to_freeze_tasks() as well as in thaw_process().  Moreover,
> if thaw_process() additionally clears TIF_FREEZE for tasks that are not frozen,
> we can be sure that all tasks are thawed and there are no pending "freeze"
> requests after thaw_tasks() has run.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ack.
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Anton Blanchard]

Hi,

> Remove PF_NOFREEZE from the rcutorture thread, adding a
> try_to_freeze() call as required.

...

> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
>  		}
>  		rcu_torture_current_version++;
>  		oldbatch = cur_ops->completed();
> +		try_to_freeze();
>  	} while (!kthread_should_stop() && !fullstop);
>  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
>  	while (!kthread_should_stop())

I wonder if it makes sense to embed try_to_freeze() into the kthread
API somewhere. Short of that we should document the try_to_freeze()
requirement in the kthread documentation... Unfortunately I cant find
any kthread docs in Documentation/ :)

Anton

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Rafael J. Wysocki]

On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
> 
> Hi,
> 
> > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > try_to_freeze() call as required.
> 
> ...
> 
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> >  		}
> >  		rcu_torture_current_version++;
> >  		oldbatch = cur_ops->completed();
> > +		try_to_freeze();
> >  	} while (!kthread_should_stop() && !fullstop);
> >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> >  	while (!kthread_should_stop())
> 
> I wonder if it makes sense to embed try_to_freeze() into the kthread
> API somewhere. Short of that we should document the try_to_freeze()
> requirement in the kthread documentation... Unfortunately I cant find
> any kthread docs in Documentation/ :)

Well, the patch is from Paul, so I think he'll be able to comment. :-)

Greetings,
Rafael

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Gautham R Shenoy]

> From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
> required.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  kernel/rcutorture.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.20-mm2/kernel/rcutorture.c
> ===================================================================
> --- linux-2.6.20-mm2.orig/kernel/rcutorture.c	2007-02-25 12:07:15.000000000 +0100
> +++ linux-2.6.20-mm2/kernel/rcutorture.c	2007-02-25 12:49:23.000000000 +0100
> @@ -46,6 +46,7 @@
>  #include <linux/byteorder/swabb.h>
>  #include <linux/stat.h>
>  #include <linux/srcu.h>
> +#include <linux/freezer.h>
> 
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
> @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
> 
>  	VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
>  	set_user_nice(current, 19);
> -	current->flags |= PF_NOFREEZE;
> 
>  	do {
>  		schedule_timeout_uninterruptible(1);
> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
>  		}
>  		rcu_torture_current_version++;
>  		oldbatch = cur_ops->completed();
> +		try_to_freeze();
>  	} while (!kthread_should_stop() && !fullstop);
>  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
>  	while (!kthread_should_stop())

Paul, 
Any reasons for not try_to_freeze()'ing the fakewriter and the reader
threads?? (Ok, I admit, I haven't looked into the code for the reason
which might be obvious.)


thanks
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Paul E. McKenney]

On Thu, Mar 01, 2007 at 08:54:25PM +0100, Rafael J. Wysocki wrote:
> On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
> > 
> > Hi,
> > 
> > > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > > try_to_freeze() call as required.
> > 
> > ...
> > 
> > > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > >  		}
> > >  		rcu_torture_current_version++;
> > >  		oldbatch = cur_ops->completed();
> > > +		try_to_freeze();
> > >  	} while (!kthread_should_stop() && !fullstop);
> > >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > >  	while (!kthread_should_stop())
> > 
> > I wonder if it makes sense to embed try_to_freeze() into the kthread
> > API somewhere. Short of that we should document the try_to_freeze()
> > requirement in the kthread documentation... Unfortunately I cant find
> > any kthread docs in Documentation/ :)
> 
> Well, the patch is from Paul, so I think he'll be able to comment. :-)

We certainly either need to embed try_to_freeze() into kthread_should_stop()
or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
components of this patch.  ;-)

One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:

	int kthread_should_stop(void)
	{
		if (kthread_stop_info.k == current)
			return 1;
		try_to_freeze();
		return 0;
	}

Does this seem reasonable?  It certainly would cut down some of the
code required for freezing -- and reduce the potential for bugs.

							Thanx, Paul

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Paul E. McKenney]

On Fri, Mar 02, 2007 at 12:27:30PM +0530, Gautham R Shenoy wrote:
> > From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as
> > required.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > Acked-by: Pavel Machek <pavel@ucw.cz>
> > ---
> >  kernel/rcutorture.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6.20-mm2/kernel/rcutorture.c
> > ===================================================================
> > --- linux-2.6.20-mm2.orig/kernel/rcutorture.c	2007-02-25 12:07:15.000000000 +0100
> > +++ linux-2.6.20-mm2/kernel/rcutorture.c	2007-02-25 12:49:23.000000000 +0100
> > @@ -46,6 +46,7 @@
> >  #include <linux/byteorder/swabb.h>
> >  #include <linux/stat.h>
> >  #include <linux/srcu.h>
> > +#include <linux/freezer.h>
> > 
> >  MODULE_LICENSE("GPL");
> >  MODULE_AUTHOR("Paul E. McKenney <paulmck@us.ibm.com> and "
> > @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
> > 
> >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
> >  	set_user_nice(current, 19);
> > -	current->flags |= PF_NOFREEZE;
> > 
> >  	do {
> >  		schedule_timeout_uninterruptible(1);
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> >  		}
> >  		rcu_torture_current_version++;
> >  		oldbatch = cur_ops->completed();
> > +		try_to_freeze();
> >  	} while (!kthread_should_stop() && !fullstop);
> >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> >  	while (!kthread_should_stop())
> 
> Paul, 
> Any reasons for not try_to_freeze()'ing the fakewriter and the reader
> threads?? (Ok, I admit, I haven't looked into the code for the reason
> which might be obvious.)

None that I know of -- though I do like Anton's idea of burying a
try_to_freeze() in kthread_should_stop(), which would get rid of all
try_to_freeze() calls in rcutorture, and many other places as well.

						Thanx, Paul

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Anton Blanchard]

Hi Paul,

> We certainly either need to embed try_to_freeze() into kthread_should_stop()
> or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
> components of this patch.  ;-)
> 
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
> 
> 	int kthread_should_stop(void)
> 	{
> 		if (kthread_stop_info.k == current)
> 			return 1;
> 		try_to_freeze();
> 		return 0;
> 	}
> 
> Does this seem reasonable?  It certainly would cut down some of the
> code required for freezing -- and reduce the potential for bugs.

Thats what I was thinking too :) 

Anton

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Oleg Nesterov]

On 03/02, Paul E. McKenney wrote:
>
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
> 
> 	int kthread_should_stop(void)
> 	{
> 		if (kthread_stop_info.k == current)
> 			return 1;
> 		try_to_freeze();
> 		return 0;
> 	}

I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.

Perhaps we should introduce a new helper which does this.

Oleg.

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Paul E. McKenney]

On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> > 
> > 	int kthread_should_stop(void)
> > 	{
> > 		if (kthread_stop_info.k == current)
> > 			return 1;
> > 		try_to_freeze();
> > 		return 0;
> > 	}
> 
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
> 
> Perhaps we should introduce a new helper which does this.

Good point -- the return value from try_to_freeze() is lost if one uses
the above approach.  About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.

One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.

Is this what you had in mind?

							Thanx, Paul

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Rafael J. Wysocki]

On Saturday, 3 March 2007 00:33, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> > 
> > 	int kthread_should_stop(void)
> > 	{
> > 		if (kthread_stop_info.k == current)
> > 			return 1;
> > 		try_to_freeze();
> > 		return 0;
> > 	}
> 
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
> 
> Perhaps we should introduce a new helper which does this.

Agreed.

Rafael

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Oleg Nesterov]

On 03/02, Paul E. McKenney wrote:
>
> On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > as follows:
> > > 
> > > 	int kthread_should_stop(void)
> > > 	{
> > > 		if (kthread_stop_info.k == current)
> > > 			return 1;
> > > 		try_to_freeze();
> > > 		return 0;
> > > 	}
> > 
> > I think this is dangerous. For example, worker_thread() will probably
> > need some special actions after return from refrigerator. Also, a kernel
> > thread may check kthread_should_stop() in the place where try_to_freeze()
> > is not safe.
> > 
> > Perhaps we should introduce a new helper which does this.
> 
> Good point -- the return value from try_to_freeze() is lost if one uses
> the above approach.  About one third of the calls to try_to_freeze()
> in 2.6.20 pay attention to the return value.
> 
> One approach would be to have a kthread_should_stop_nofreeze() for those
> cases, and let the default be to try to freeze.

I personally think we should do the opposite, add kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously "might_sleep()".

Oleg.

Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread

[Srivatsa Vaddagiri]

On Sat, Mar 03, 2007 at 08:32:40PM +0300, Oleg Nesterov wrote:
> I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> or something. kthread_should_stop() is like signal_pending(), we can use
> it under spin_lock (and it is probably used this way by some out-of-tree
> driver). The new helper is obviously "might_sleep()".

Yes, I agree. It helps the caller explicitly know that this function
-can- freeze.

-- 
Regards,
vatsa

[PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Rafael J. Wysocki]

On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > On 03/02, Paul E. McKenney wrote:
> > > >
> > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > as follows:
> > > > 
> > > > 	int kthread_should_stop(void)
> > > > 	{
> > > > 		if (kthread_stop_info.k == current)
> > > > 			return 1;
> > > > 		try_to_freeze();
> > > > 		return 0;
> > > > 	}
> > > 
> > > I think this is dangerous. For example, worker_thread() will probably
> > > need some special actions after return from refrigerator. Also, a kernel
> > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > is not safe.
> > > 
> > > Perhaps we should introduce a new helper which does this.
> > 
> > Good point -- the return value from try_to_freeze() is lost if one uses
> > the above approach.  About one third of the calls to try_to_freeze()
> > in 2.6.20 pay attention to the return value.
> > 
> > One approach would be to have a kthread_should_stop_nofreeze() for those
> > cases, and let the default be to try to freeze.
> 
> I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> or something. kthread_should_stop() is like signal_pending(), we can use
> it under spin_lock (and it is probably used this way by some out-of-tree
> driver). The new helper is obviously "might_sleep()".

Something like this, perhaps:

 include/linux/kthread.h |    1 +
 kernel/kthread.c        |   16 ++++++++++++++++
 kernel/rcutorture.c     |    5 ++---
 3 files changed, 19 insertions(+), 3 deletions(-)

Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c	2007-03-08 21:58:48.000000000 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c	2007-03-11 18:32:59.000000000 +0100
@@ -13,6 +13,7 @@
 #include <linux/file.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/freezer.h>
 #include <asm/semaphore.h>
 
 /*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
 }
 EXPORT_SYMBOL(kthread_should_stop);
 
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+	might_sleep();
+	if (kthread_stop_info.k == current)
+		return 1;
+
+	try_to_freeze();
+	return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
+
 static void kthread_exit_files(void)
 {
 	struct fs_struct *fs;
Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
===================================================================
--- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h	2007-02-04 19:44:54.000000000 +0100
+++ linux-2.6.21-rc3-mm2/include/linux/kthread.h	2007-03-11 18:37:10.000000000 +0100
@@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
 void kthread_bind(struct task_struct *k, unsigned int cpu);
 int kthread_stop(struct task_struct *k);
 int kthread_should_stop(void);
+int kthread_should_stop_check_freeze(void);
 
 #endif /* _LINUX_KTHREAD_H */
Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
===================================================================
--- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c	2007-03-11 11:39:06.000000000 +0100
+++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c	2007-03-11 18:45:00.000000000 +0100
@@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
 		}
 		rcu_torture_current_version++;
 		oldbatch = cur_ops->completed();
-		try_to_freeze();
-	} while (!kthread_should_stop() && !fullstop);
+	} while (!kthread_should_stop_check_freeze() && !fullstop);
 	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
-	while (!kthread_should_stop())
+	while (!kthread_should_stop_check_freeze())
 		schedule_timeout_uninterruptible(1);
 	return 0;
 }

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Paul E. McKenney]

On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > > 
> > > > > 	int kthread_should_stop(void)
> > > > > 	{
> > > > > 		if (kthread_stop_info.k == current)
> > > > > 			return 1;
> > > > > 		try_to_freeze();
> > > > > 		return 0;
> > > > > 	}
> > > > 
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > > is not safe.
> > > > 
> > > > Perhaps we should introduce a new helper which does this.
> > > 
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach.  About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > > 
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> > 
> > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
> 
> Something like this, perhaps:

Looks good to me!  The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

>  include/linux/kthread.h |    1 +
>  kernel/kthread.c        |   16 ++++++++++++++++
>  kernel/rcutorture.c     |    5 ++---
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c	2007-03-08 21:58:48.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c	2007-03-11 18:32:59.000000000 +0100
> @@ -13,6 +13,7 @@
>  #include <linux/file.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/freezer.h>
>  #include <asm/semaphore.h>
> 
>  /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
>  }
>  EXPORT_SYMBOL(kthread_should_stop);
> 
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> +	might_sleep();
> +	if (kthread_stop_info.k == current)
> +		return 1;
> +
> +	try_to_freeze();
> +	return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);
> +
>  static void kthread_exit_files(void)
>  {
>  	struct fs_struct *fs;
> Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h	2007-02-04 19:44:54.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/include/linux/kthread.h	2007-03-11 18:37:10.000000000 +0100
> @@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
>  void kthread_bind(struct task_struct *k, unsigned int cpu);
>  int kthread_stop(struct task_struct *k);
>  int kthread_should_stop(void);
> +int kthread_should_stop_check_freeze(void);
> 
>  #endif /* _LINUX_KTHREAD_H */
> Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c	2007-03-11 11:39:06.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c	2007-03-11 18:45:00.000000000 +0100
> @@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
>  		}
>  		rcu_torture_current_version++;
>  		oldbatch = cur_ops->completed();
> -		try_to_freeze();
> -	} while (!kthread_should_stop() && !fullstop);
> +	} while (!kthread_should_stop_check_freeze() && !fullstop);
>  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> -	while (!kthread_should_stop())
> +	while (!kthread_should_stop_check_freeze())
>  		schedule_timeout_uninterruptible(1);
>  	return 0;
>  }

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Pavel Machek]

Hi!

> > > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > driver). The new helper is obviously "might_sleep()".
> > 
> > Something like this, perhaps:
> 
> Looks good to me!  The other kthread_should_stop() calls in
> rcutorture.c should also become kthread_should_top_check_freeze().
> 
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> >  include/linux/kthread.h |    1 +
> >  kernel/kthread.c        |   16 ++++++++++++++++
> >  kernel/rcutorture.c     |    5 ++---
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> > 
> > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > ===================================================================
> > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c	2007-03-08 21:58:48.000000000 +0100
> > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c	2007-03-11 18:32:59.000000000 +0100
> > @@ -13,6 +13,7 @@
> >  #include <linux/file.h>
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> > +#include <linux/freezer.h>
> >  #include <asm/semaphore.h>
> > 
> >  /*
> > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> >  }
> >  EXPORT_SYMBOL(kthread_should_stop);
> > 
> > +/**
> > + * kthread_should_stop_check_freeze - check if the thread should return now and
> > + * if not, check if there is a freezing request pending for it.
> > + */
> > +int kthread_should_stop_check_freeze(void)
> > +{
> > +	might_sleep();
> > +	if (kthread_stop_info.k == current)
> > +		return 1;
> > +
> > +	try_to_freeze();
> > +	return 0;
> > +}

Can we get better name for this function?

Why is it useful? Caller can do "try_to_freeze()" as well, no?

> >  		}
> >  		rcu_torture_current_version++;
> >  		oldbatch = cur_ops->completed();
> > -		try_to_freeze();
> > -	} while (!kthread_should_stop() && !fullstop);
> > +	} while (!kthread_should_stop_check_freeze() && !fullstop);
> >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > -	while (!kthread_should_stop())
> > +	while (!kthread_should_stop_check_freeze())
> >  		schedule_timeout_uninterruptible(1);
> >  	return 0;

Aha, I see, here it probably becomes handy.

Actually, no... I do not see it. Why don't you simply move first
try_to_freeze() to beggining of the loop and do

-   while (!kthread_should_stop()) {
             schedule_timeout_uninterruptible(1);
	     try_to_freeze()
}
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Rafael J. Wysocki]

Hi,

On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> Hi!
> 
> > > > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > > driver). The new helper is obviously "might_sleep()".
> > > 
> > > Something like this, perhaps:
> > 
> > Looks good to me!  The other kthread_should_stop() calls in
> > rcutorture.c should also become kthread_should_top_check_freeze().
> > 
> > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > >  include/linux/kthread.h |    1 +
> > >  kernel/kthread.c        |   16 ++++++++++++++++
> > >  kernel/rcutorture.c     |    5 ++---
> > >  3 files changed, 19 insertions(+), 3 deletions(-)
> > > 
> > > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > > ===================================================================
> > > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c	2007-03-08 21:58:48.000000000 +0100
> > > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c	2007-03-11 18:32:59.000000000 +0100
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/file.h>
> > >  #include <linux/module.h>
> > >  #include <linux/mutex.h>
> > > +#include <linux/freezer.h>
> > >  #include <asm/semaphore.h>
> > > 
> > >  /*
> > > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> > >  }
> > >  EXPORT_SYMBOL(kthread_should_stop);
> > > 
> > > +/**
> > > + * kthread_should_stop_check_freeze - check if the thread should return now and
> > > + * if not, check if there is a freezing request pending for it.
> > > + */
> > > +int kthread_should_stop_check_freeze(void)
> > > +{
> > > +	might_sleep();
> > > +	if (kthread_stop_info.k == current)
> > > +		return 1;
> > > +
> > > +	try_to_freeze();
> > > +	return 0;
> > > +}
> 
> Can we get better name for this function?

Well, I took the name from the Oleg's message.  Can you please suggest
something?

> Why is it useful?

Because we want to avoid repeating

while (!kthread_should_stop()) {
	try_to_freeze();
	...
}

in many places?

> Caller can do "try_to_freeze()" as well, no? 

Sure, it just eliminates one line of code.

> > >  		}
> > >  		rcu_torture_current_version++;
> > >  		oldbatch = cur_ops->completed();
> > > -		try_to_freeze();
> > > -	} while (!kthread_should_stop() && !fullstop);
> > > +	} while (!kthread_should_stop_check_freeze() && !fullstop);
> > >  	VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > > -	while (!kthread_should_stop())
> > > +	while (!kthread_should_stop_check_freeze())
> > >  		schedule_timeout_uninterruptible(1);
> > >  	return 0;
> 
> Aha, I see, here it probably becomes handy.
> 
> Actually, no... I do not see it. Why don't you simply move first
> try_to_freeze() to beggining of the loop and do
> 
> -   while (!kthread_should_stop()) {
>              schedule_timeout_uninterruptible(1);
> 	     try_to_freeze()
> }

Yes, but then the second loop will contain one more line of code.

Greetings,
Rafael

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Oleg Nesterov]

On 03/12, Rafael J. Wysocki wrote:
> 
> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> > 
> > Can we get better name for this function?
> 
> Well, I took the name from the Oleg's message.  Can you please suggest
> something?

Well, kthread_should_stop_check_freeze() is really awful, I agree :)
We need something better, but I can't suggest anything.

Oleg.

Re: [PATCH] kthread_should_stop_check_freeze

[Cedric Le Goater]

Oleg Nesterov wrote:
> On 03/12, Rafael J. Wysocki wrote:
>> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
>>> Can we get better name for this function?
>> Well, I took the name from the Oleg's message.  Can you please suggest
>> something?
> 
> Well, kthread_should_stop_check_freeze() is really awful, I agree :)
> We need something better, but I can't suggest anything.

not much better, but what about kthread_should_stop_or_freeze() ?

cheers,

C.

Re: [PATCH] kthread_should_stop_check_freeze

[Rafael J. Wysocki]

On Monday, 12 March 2007 14:24, Cedric Le Goater wrote:
> Oleg Nesterov wrote:
> > On 03/12, Rafael J. Wysocki wrote:
> >> On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> >>> Can we get better name for this function?
> >> Well, I took the name from the Oleg's message.  Can you please suggest
> >> something?
> > 
> > Well, kthread_should_stop_check_freeze() is really awful, I agree :)
> > We need something better, but I can't suggest anything.
> 
> not much better, but what about kthread_should_stop_or_freeze() ?

IMO this suggests that the thread will either stop or freeze itself, but the
freezing part is not unconditional.

I was thinking of kthread_should_stop_freezable() .

Greetings,
Rafael

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Pavel Machek]

Hi!

> > > Looks good to me!  The other kthread_should_stop() calls in
> > > rcutorture.c should also become
> > > kthread_should_top_check_freeze().

> > Why is it useful?
> 
> Because we want to avoid repeating
> 
> while (!kthread_should_stop()) {
> 	try_to_freeze();
> 	...
> }
> 
> in many places?

Do not do it, then. Confusion it causes is not worth saving one line
of code. 

You do less typing, but the resulting code is _less_ readable, not
more.

NAK.

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Anton Blanchard]

> Do not do it, then. Confusion it causes is not worth saving one line
> of code. 
> 
> You do less typing, but the resulting code is _less_ readable, not
> more.

Then please document it _clearly_ with the kthread code somewhere. The
reason I brought this up is I had no idea we had to put the freezer gunk
in all kernel thread loops and Ive been writing kernel threads for years.

Anton

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Paul E. McKenney]

On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > > Looks good to me!  The other kthread_should_stop() calls in
> > > > rcutorture.c should also become
> > > > kthread_should_top_check_freeze().
> 
> > > Why is it useful?
> > 
> > Because we want to avoid repeating
> > 
> > while (!kthread_should_stop()) {
> > 	try_to_freeze();
> > 	...
> > }
> > 
> > in many places?
> 
> Do not do it, then. Confusion it causes is not worth saving one line
> of code. 
> 
> You do less typing, but the resulting code is _less_ readable, not
> more.
> 
> NAK.

Another problem is people doing "kthread_should_stop()" and forgetting
about freezing.  Then we continue ending up with situations where we are
intermittently unable to freeze.  In the spirit of "Rusty Scale" interface
design, how do we make it difficult for people to misuse this interface?

						Thanx, Paul

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Srivatsa Vaddagiri]

On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote:
> Then please document it _clearly_ with the kthread code somewhere. 

Document as well in the kernel_thread() API, as I notice people still
use kernel_thread() some places (ex: rtasd.c in powerpc arch)?

> The reason I brought this up is I had no idea we had to put the freezer gunk
> in all kernel thread loops and Ive been writing kernel threads for years.

I noticed that in the Powerpc code (atleast for rtas kernel thread)
here:

	http://lkml.org/lkml/2007/1/9/61

That was not a serious problem perhaps because process freezer was mostly used
in software suspend and only those platforms supporting software suspend
had to worry abt it.

But now we intend to use process freezer for CPU hotplug as well, so all
platforms wanting to support CPU hotplug better support process freezer!

P.S : I believe kprobes is already using process freezer as well.

-- 
Regards,
vatsa

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Gautham R Shenoy]

On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > > 
> > > > > 	int kthread_should_stop(void)
> > > > > 	{
> > > > > 		if (kthread_stop_info.k == current)
> > > > > 			return 1;
> > > > > 		try_to_freeze();
> > > > > 		return 0;
> > > > > 	}
> > > > 
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > > is not safe.
> > > > 
> > > > Perhaps we should introduce a new helper which does this.
> > > 
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach.  About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > > 
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> > 
> > I personally think we should do the opposite, add kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
> 
> Something like this, perhaps:
> 
>  include/linux/kthread.h |    1 +
>  kernel/kthread.c        |   16 ++++++++++++++++
>  kernel/rcutorture.c     |    5 ++---
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===================================================================
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c	2007-03-08 21:58:48.000000000 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c	2007-03-11 18:32:59.000000000 +0100
> @@ -13,6 +13,7 @@
>  #include <linux/file.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/freezer.h>
>  #include <asm/semaphore.h>
> 
>  /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
>  }
>  EXPORT_SYMBOL(kthread_should_stop);
> 
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> +	might_sleep();
> +	if (kthread_stop_info.k == current)
> +		return 1;
> +
> +	try_to_freeze();
> +	return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);

I would prefer to have try_to_freeze() followed by the
kthread_stop_info.k check. Something like

if (try_to_freeze())
	/*some barrier ensuring all writes are completed */

if (kthread_stop_info.k == current)
		return 1;
return 0;

This would be helpful in situations (atleast for cpu-hotplug)
where we want to stop a frozen thread immediately after thawing it.
Something like

CPU_DEAD:
thaw_process(p);
kthread_stop(p);
p = NULL;

Is there a problem with this line of thinking ?

thanks and regards
gautham.
-- 
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Srivatsa Vaddagiri]

On Tue, Mar 13, 2007 at 10:57:16AM +0530, Gautham R Shenoy wrote:
> CPU_DEAD:
> thaw_process(p);
> kthread_stop(p);
> p = NULL;

This neednt guarantee that the thread will see the stop request before
it exits the kthread_should_stop_freeze() function. There will always
be races .. So the only safe way for a thread to know whether it is time
to exit is:

	while (!kthread_should_stop_freeze()) {
		 if (!cpu_online(home_cpu)) 
			goto wait_to_die;

		...

	}

wait_to_die:

	while (!kthread_should_stop()) {
		/* sleep */
	}


-- 
Regards,
vatsa

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Christoph Hellwig]

On Tue, Mar 13, 2007 at 08:44:11AM +0530, Srivatsa Vaddagiri wrote:
> On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote:
> > Then please document it _clearly_ with the kthread code somewhere. 
> 
> Document as well in the kernel_thread() API, as I notice people still
> use kernel_thread() some places (ex: rtasd.c in powerpc arch)?

They shouldn't use kernel_thread.

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Srivatsa Vaddagiri]

On Tue, Mar 13, 2007 at 09:16:29AM +0000, Christoph Hellwig wrote:
> > Document as well in the kernel_thread() API, as I notice people still
> > use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
> 
> They shouldn't use kernel_thread.

Hmm ..that needs to be documented as well then! I can easily count more
than dozen places where kernel_thread() is being used.

I agree though that kthread is a much cleaner interface to create/destroy 
threads.

-- 
Regards,
vatsa

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Christoph Hellwig]

On Tue, Mar 13, 2007 at 03:28:08PM +0530, Srivatsa Vaddagiri wrote:
> On Tue, Mar 13, 2007 at 09:16:29AM +0000, Christoph Hellwig wrote:
> > > Document as well in the kernel_thread() API, as I notice people still
> > > use kernel_thread() some places (ex: rtasd.c in powerpc arch)?
> > 
> > They shouldn't use kernel_thread.
> 
> Hmm ..that needs to be documented as well then! I can easily count more
> than dozen places where kernel_thread() is being used.
> 
> I agree though that kthread is a much cleaner interface to create/destroy 
> threads.

Well, it takes a lot of time to convert all the existing users.  But
I try to make sure to flame^H^H^H^H^Hcorrect everyone who tries to sneak
a new user of kernel_thread in.

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Pavel Machek]

Hi!

> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code. 
> > 
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
> 
> Then please document it _clearly_ with the kthread code somewhere. The
> reason I brought this up is I had no idea we had to put the freezer gunk
> in all kernel thread loops and Ive been writing kernel threads for years.

This is in Doc*/power/kernel_threads.txt:

KERNEL THREADS


Freezer

Upon entering a suspended state the system will freeze all
tasks. This is done by delivering pseudosignals. This affects
kernel threads, too. To successfully freeze a kernel thread
the thread has to check for the pseudosignal and enter the
refrigerator. Code to do this looks like this:

        do {
                hub_events();
                wait_event_interruptible(khubd_wait,
!list_empty(&hub_event_list));
                try_to_freeze();
        } while (!signal_pending(current));


...do you know about better place?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)

[Pavel Machek]

On Mon 2007-03-12 17:58:05, Paul E. McKenney wrote:
> On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Looks good to me!  The other kthread_should_stop() calls in
> > > > > rcutorture.c should also become
> > > > > kthread_should_top_check_freeze().
> > 
> > > > Why is it useful?
> > > 
> > > Because we want to avoid repeating
> > > 
> > > while (!kthread_should_stop()) {
> > > 	try_to_freeze();
> > > 	...
> > > }
> > > 
> > > in many places?
> > 
> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code. 
> > 
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
> > 
> > NAK.
> 
> Another problem is people doing "kthread_should_stop()" and forgetting
> about freezing.  Then we continue ending up with situations where weare
> intermittently unable to freeze.  In the spirit of "Rusty Scale" interface

No, in such case we end up with situation when it is impossible to
freeze, and that's very easy to debug.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html