Skip to contentSkip to conversation

Per kthread freezer flags

PATCH In review Nigel Cunningham 5 participants 24 messages Jul 28, 2004 – Jul 31, 2004

Subsystems (13)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Per kthread freezer flags

Hi Andrew et al.

At the moment, all kthreads have PF_NOFREEZE set, meaning that they're not refrigerated during a suspend. This isn't right for some threads. They should be frozen while suspending. The attached patch implements per-kthread freezer flags. It does this by adding a new parameter to the create_workqueue call and its siblings. The new parameter contains the process flags relevant to suspending to be set. At the moment, this only means PF_FREEZE, but when I send the freezer improvements, a PF_SYNCTHREAD flag will also be valid here. The new parameter is passed down through the calls and applied (after masking invalid bits) once the thread is created.

Pavel has seen the code and requested that I send it.

Regards,

Nigel

drivers/acpi/osl.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/acpi/osl.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/acpi/osl.c
@@ -81,7 +81,7 @@
 		return AE_NULL_ENTRY;
 	}
 #endif
-	kacpid_wq = create_singlethread_workqueue("kacpid");
+	kacpid_wq = create_singlethread_workqueue("kacpid", 0);
 	BUG_ON(!kacpid_wq);
 
 	return AE_OK;
drivers/block/ll_rw_blk.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/block/ll_rw_blk.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/block/ll_rw_blk.c
@@ -2982,7 +2982,7 @@
 
 int __init blk_dev_init(void)
 {
-	kblockd_workqueue = create_workqueue("kblockd");
+	kblockd_workqueue = create_workqueue("kblockd", PF_NOFREEZE);
 	if (!kblockd_workqueue)
 		panic("Failed to create kblockd\n");
 
drivers/block/pktcdvd.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/block/pktcdvd.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/block/pktcdvd.c
@@ -2372,7 +2372,7 @@
 
 	pkt_init_queue(pd);
 
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;
drivers/md/dm-crypt.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/md/dm-crypt.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/dm-crypt.c
@@ -758,7 +758,7 @@
 	if (!_crypt_io_pool)
 		return -ENOMEM;
 
-	_kcryptd_workqueue = create_workqueue("kcryptd");
+	_kcryptd_workqueue = create_workqueue("kcryptd", PF_NOFREEZE);
 	if (!_kcryptd_workqueue) {
 		r = -ENOMEM;
 		DMERR(PFX "couldn't create kcryptd");
drivers/md/dm-raid1.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/md/dm-raid1.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/dm-raid1.c
@@ -1238,7 +1238,7 @@
 	if (r)
 		return r;
 
-	_kmirrord_wq = create_workqueue("kmirrord");
+	_kmirrord_wq = create_workqueue("kmirrord", PF_NOFREEZE);
 	if (!_kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		dm_dirty_log_exit();
drivers/md/kcopyd.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/md/kcopyd.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/kcopyd.c
@@ -609,7 +609,7 @@
 		return r;
 	}
 
-	_kcopyd_wq = create_singlethread_workqueue("kcopyd");
+	_kcopyd_wq = create_singlethread_workqueue("kcopyd", 0);
 	if (!_kcopyd_wq) {
 		jobs_exit();
 		up(&kcopyd_init_lock);
drivers/net/wan/sdlamain.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/net/wan/sdlamain.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/net/wan/sdlamain.c
@@ -240,7 +240,7 @@
 	printk(KERN_INFO "%s v%u.%u %s\n",
 		fullname, DRV_VERSION, DRV_RELEASE, copyright);
 
-	wanpipe_wq = create_workqueue("wanpipe_wq");
+	wanpipe_wq = create_workqueue("wanpipe_wq", 0);
 	if (!wanpipe_wq)
 		return -ENOMEM;
 
drivers/s390/cio/device.c +4 -3
diff -ruN linux-2.6.8-rc1-mm1/drivers/s390/cio/device.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/s390/cio/device.c
@@ -151,15 +151,16 @@
 	init_waitqueue_head(&ccw_device_init_wq);
 	atomic_set(&ccw_device_init_count, 0);
 
-	ccw_device_work = create_singlethread_workqueue("cio");
+	ccw_device_work = create_singlethread_workqueue("cio", 0);
 	if (!ccw_device_work)
 		return -ENOMEM; /* FIXME: better errno ? */
-	ccw_device_notify_work = create_singlethread_workqueue("cio_notify");
+	ccw_device_notify_work = create_singlethread_workqueue("cio_notify",
+			0);
 	if (!ccw_device_notify_work) {
 		ret = -ENOMEM; /* FIXME: better errno ? */
 		goto out_err;
 	}
-	slow_path_wq = create_singlethread_workqueue("kslowcrw");
+	slow_path_wq = create_singlethread_workqueue("kslowcrw", 0);
 	if (!slow_path_wq) {
 		ret = -ENOMEM; /* FIXME: better errno ? */
 		goto out_err;
drivers/scsi/libata-core.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/drivers/scsi/libata-core.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/scsi/libata-core.c
@@ -3438,7 +3438,7 @@
 
 static int __init ata_init(void)
 {
-	ata_wq = create_workqueue("ata");
+	ata_wq = create_workqueue("ata", PF_NOFREEZE);
 	if (!ata_wq)
 		return -ENOMEM;
 
fs/aio.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/fs/aio.c linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/aio.c
@@ -69,7 +69,7 @@
 	kioctx_cachep = kmem_cache_create("kioctx", sizeof(struct kioctx),
 				0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
 
-	aio_wq = create_workqueue("aio");
+	aio_wq = create_workqueue("aio", PF_NOFREEZE);
 
 	pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page));
 
fs/reiserfs/journal.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/fs/reiserfs/journal.c linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/reiserfs/journal.c
@@ -2483,7 +2483,7 @@
 
   reiserfs_mounted_fs_count++ ;
   if (reiserfs_mounted_fs_count <= 1)
-    commit_wq = create_workqueue("reiserfs");
+    commit_wq = create_workqueue("reiserfs", 0);
 
   INIT_WORK(&journal->j_work, flush_async_commits, p_s_sb);
   return 0 ;
fs/xfs/linux-2.6/xfs_buf.c +2 -2
diff -ruN linux-2.6.8-rc1-mm1/fs/xfs/linux-2.6/xfs_buf.c linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/xfs/linux-2.6/xfs_buf.c
@@ -1746,11 +1746,11 @@
 {
 	int		rval;
 
-	pagebuf_logio_workqueue = create_workqueue("xfslogd");
+	pagebuf_logio_workqueue = create_workqueue("xfslogd", 0);
 	if (!pagebuf_logio_workqueue)
 		return -ENOMEM;
 
-	pagebuf_dataio_workqueue = create_workqueue("xfsdatad");
+	pagebuf_dataio_workqueue = create_workqueue("xfsdatad", 0);
 	if (!pagebuf_dataio_workqueue) {
 		destroy_workqueue(pagebuf_logio_workqueue);
 		return -ENOMEM;
include/linux/kthread.h +7 -2
diff -ruN linux-2.6.8-rc1-mm1/include/linux/kthread.h linux-2.6.8-rc1-mm1-kthread_refrigerator/include/linux/kthread.h
@@ -25,20 +25,25 @@
  */
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
+				   unsigned long freezer_flags,
 				   const char namefmt[], ...);
 
 /**
  * kthread_run: create and wake a thread.
  * @threadfn: the function to run until signal_pending(current).
  * @data: data ptr for @threadfn.
+ * @freezer_flags: process flags that should be used for freezing.
+ * 	PF_NOFREEZE if also needed for writing the image.
+ * 	0 otherwise.
  * @namefmt: printf-style name for the thread.
  *
  * Description: Convenient wrapper for kthread_create() followed by
  * wake_up_process().  Returns the kthread, or ERR_PTR(-ENOMEM). */
-#define kthread_run(threadfn, data, namefmt, ...)			   \
+#define kthread_run(threadfn, data, freezer_flags, namefmt, ...)	   \
 ({									   \
 	struct task_struct *__k						   \
-		= kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
+		= kthread_create(threadfn, data, freezer_flags, 	   \
+			namefmt, ## __VA_ARGS__);			   \
 	if (!IS_ERR(__k))						   \
 		wake_up_process(__k);					   \
 	__k;								   \
include/linux/workqueue.h +4 -3
diff -ruN linux-2.6.8-rc1-mm1/include/linux/workqueue.h linux-2.6.8-rc1-mm1-kthread_refrigerator/include/linux/workqueue.h
@@ -51,9 +51,10 @@
 	} while (0)
 
 extern struct workqueue_struct *__create_workqueue(const char *name,
-						    int singlethread);
-#define create_workqueue(name) __create_workqueue((name), 0)
-#define create_singlethread_workqueue(name) __create_workqueue((name), 1)
+						    int singlethread,
+						    unsigned long freezer_flag);
+#define create_workqueue(name, flags) __create_workqueue((name), 0, flags)
+#define create_singlethread_workqueue(name, flags) __create_workqueue((name), 1, flags)
 
 extern void destroy_workqueue(struct workqueue_struct *wq);
 
kernel/kmod.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/kernel/kmod.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/kmod.c
@@ -274,7 +274,7 @@
 
 static __init int usermodehelper_init(void)
 {
-	khelper_wq = create_singlethread_workqueue("khelper");
+	khelper_wq = create_singlethread_workqueue("khelper", 0);
 	BUG_ON(!khelper_wq);
 	return 0;
 }
kernel/kthread.c +8 -0
diff -ruN linux-2.6.8-rc1-mm1/kernel/kthread.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/kthread.c
@@ -19,6 +19,7 @@
 	/* Information passed to kthread() from keventd. */
 	int (*threadfn)(void *data);
 	void *data;
+	unsigned long freezer_flags;
 	struct completion started;
 
 	/* Result passed back to kthread_create() from keventd. */
@@ -65,6 +66,7 @@
 	void *data;
 	sigset_t blocked;
 	int ret = -EINTR;
+	unsigned long flags_used;
 
 	kthread_exit_files();
 
@@ -80,6 +82,10 @@
 	/* By default we can run anywhere, unlike keventd. */
 	set_cpus_allowed(current, CPU_MASK_ALL);
 
+	/* Validate and set our freezer flags */
+	flags_used = create->freezer_flags & PF_NOFREEZE;
+	current->flags |= flags_used;
+
 	/* OK, tell user we're spawned, wait for stop or wakeup */
 	__set_current_state(TASK_INTERRUPTIBLE);
 	complete(&create->started);
@@ -115,6 +121,7 @@
 
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
+				   unsigned long freezer_flags,
 				   const char namefmt[],
 				   ...)
 {
@@ -123,6 +130,7 @@
 
 	create.threadfn = threadfn;
 	create.data = data;
+	create.freezer_flags = freezer_flags;
 	init_completion(&create.started);
 	init_completion(&create.done);
 
kernel/sched.c +2 -1
diff -ruN linux-2.6.8-rc1-mm1/kernel/sched.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/sched.c
@@ -3550,7 +3550,8 @@
 
 	switch (action) {
 	case CPU_UP_PREPARE:
-		p = kthread_create(migration_thread, hcpu, "migration/%d",cpu);
+		p = kthread_create(migration_thread, hcpu, 0,
+				"migration/%d",cpu);
 		if (IS_ERR(p))
 			return NOTIFY_BAD;
 		p->flags |= PF_NOFREEZE;
kernel/softirq.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/kernel/softirq.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/softirq.c
@@ -425,7 +425,7 @@
 	case CPU_UP_PREPARE:
 		BUG_ON(per_cpu(tasklet_vec, hotcpu).list);
 		BUG_ON(per_cpu(tasklet_hi_vec, hotcpu).list);
-		p = kthread_create(ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu);
+		p = kthread_create(ksoftirqd, hcpu, 0, "ksoftirqd/%d", hotcpu);
 		if (IS_ERR(p)) {
 			printk("ksoftirqd for %i failed\n", hotcpu);
 			return NOTIFY_BAD;
kernel/stop_machine.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/kernel/stop_machine.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/stop_machine.c
@@ -174,7 +174,7 @@
 	if (cpu == NR_CPUS)
 		cpu = smp_processor_id();
 
-	p = kthread_create(do_stop, &smdata, "kstopmachine");
+	p = kthread_create(do_stop, &smdata, 0, "kstopmachine");
 	if (!IS_ERR(p)) {
 		kthread_bind(p, cpu);
 		wake_up_process(p);
kernel/workqueue.c +14 -9
diff -ruN linux-2.6.8-rc1-mm1/kernel/workqueue.c linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/workqueue.c
@@ -25,6 +25,7 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/kthread.h>
+#include <linux/suspend.h>
 
 /*
  * The per-CPU workqueue (if single thread, we always use cpu 0's).
@@ -186,8 +187,6 @@
 	struct k_sigaction sa;
 	sigset_t blocked;
 
-	current->flags |= PF_NOFREEZE;
-
 	set_user_nice(current, -10);
 
 	/* Block and flush all signals */
@@ -208,6 +207,8 @@
 			schedule();
 		else
 			__set_current_state(TASK_RUNNING);
+		if (current->flags & PF_FREEZE)
+			refrigerator(PF_FREEZE);
 		remove_wait_queue(&cwq->more_work, &wait);
 
 		if (!list_empty(&cwq->worklist))
@@ -277,7 +278,8 @@
 }
 
 static struct task_struct *create_workqueue_thread(struct workqueue_struct *wq,
-						   int cpu)
+						   int cpu,
+						   unsigned long freezer_flags)
 {
 	struct cpu_workqueue_struct *cwq = wq->cpu_wq + cpu;
 	struct task_struct *p;
@@ -292,9 +294,11 @@
 	init_waitqueue_head(&cwq->work_done);
 
 	if (is_single_threaded(wq))
-		p = kthread_create(worker_thread, cwq, "%s", wq->name);
+		p = kthread_create(worker_thread, cwq, freezer_flags, 
+				"%s", wq->name);
 	else
-		p = kthread_create(worker_thread, cwq, "%s/%d", wq->name, cpu);
+		p = kthread_create(worker_thread, cwq, freezer_flags,
+				"%s/%d", wq->name, cpu);
 	if (IS_ERR(p))
 		return NULL;
 	cwq->thread = p;
@@ -302,7 +306,8 @@
 }
 
 struct workqueue_struct *__create_workqueue(const char *name,
-					    int singlethread)
+					    int singlethread,
+					    unsigned long freezer_flags)
 {
 	int cpu, destroy = 0;
 	struct workqueue_struct *wq;
@@ -320,7 +325,7 @@
 	lock_cpu_hotplug();
 	if (singlethread) {
 		INIT_LIST_HEAD(&wq->list);
-		p = create_workqueue_thread(wq, 0);
+		p = create_workqueue_thread(wq, 0, freezer_flags);
 		if (!p)
 			destroy = 1;
 		else
@@ -330,7 +335,7 @@
 		list_add(&wq->list, &workqueues);
 		spin_unlock(&workqueue_lock);
 		for_each_online_cpu(cpu) {
-			p = create_workqueue_thread(wq, cpu);
+			p = create_workqueue_thread(wq, cpu, freezer_flags);
 			if (p) {
 				kthread_bind(p, cpu);
 				wake_up_process(p);
@@ -493,7 +498,7 @@
 void init_workqueues(void)
 {
 	hotcpu_notifier(workqueue_cpu_callback, 0);
-	keventd_wq = create_workqueue("events");
+	keventd_wq = create_workqueue("events", 0);
 	BUG_ON(!keventd_wq);
 }
 
mm/pdflush.c +1 -1
diff -ruN linux-2.6.8-rc1-mm1/mm/pdflush.c linux-2.6.8-rc1-mm1-kthread_refrigerator/mm/pdflush.c
@@ -215,7 +215,7 @@
 
 static void start_one_pdflush_thread(void)
 {
-	kthread_run(pdflush, NULL, "pdflush");
+	kthread_run(pdflush, NULL, 0, "pdflush");
 }
 
 static int __init pdflush_init(void)
#1 lore raw
Quoted 22 lines · Nigel Cunningham

At the moment, all kthreads have PF_NOFREEZE set, meaning that they're not refrigerated during a suspend. This isn't right for some threads. They should be frozen while suspending. The attached patch implements per-kthread freezer flags. It does this by adding a new parameter to the create_workqueue call and its siblings. The new parameter contains the process flags relevant to suspending to be set. At the moment, this only means PF_FREEZE, but when I send the freezer improvements, a PF_SYNCTHREAD flag will also be valid here. The new parameter is passed down through the calls and applied (after masking invalid bits) once the thread is created.

Pavel has seen the code and requested that I send it.

Regards,

Nigel

diff -ruN linux-2.6.8-rc1-mm1/drivers/acpi/osl.c linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/acpi/osl.c

drivers/acpi/osl.c line 84

Quoted 5 lines
drivers/acpi/osl.c +1 -1
@@ -81,7 +81,7 @@
 		return AE_NULL_ENTRY;
 	}
 #endif
-	kacpid_wq = create_singlethread_workqueue("kacpid");
+	kacpid_wq = create_singlethread_workqueue("kacpid", 0);

hm. In some ways I'd prefer to see new create_singlethread_workqueue_freezer(char *) or whatever, rather than adding an extra argument. That's neater, smaller code and forward-compatible.

But then again, the advantage of breaking the build for unconverted code is that it makes people think about what their threads should be doing, so let's go your way.

The one concern I'd have is that $RANDOM_KERNEL_DEVELOPER probably doesn't have a clue whether or not his kernel thread should be setting PF_NOFREEZE. What are the guidelines here?

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

#2 lore raw
Quoted 5 lines · Andrew Morton

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

BTW, the "Add missing refrigerator support" breaks ACPI S3 and S4 support for me (2.6.8-rc2-bk7) and my laptop (NEC Chrom@). When resuming, my 3c59x CardBus NIC is not powered up forcing me to eject it, then plug it again.

#3 lore raw

Hi.

Quoted 8 lines · Felipe Alfaro Solana
Quoted 5 lines · Andrew Morton

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

Quoted 4 lines

BTW, the "Add missing refrigerator support" breaks ACPI S3 and S4 support for me (2.6.8-rc2-bk7) and my laptop (NEC Chrom@). When resuming, my 3c59x CardBus NIC is not powered up forcing me to eject it, then plug it again.

Hmm. I'll take a look. Those patches are straight out of current suspend2 and have been in there for some time. Perhaps I just don't have any users who have that config.

Regards,

Nigel

#4 lore raw
In reply to Felipe Alfaro Solana

Hi again.

Quoted 8 lines · Felipe Alfaro Solana
Quoted 5 lines · Andrew Morton

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

Quoted 4 lines

BTW, the "Add missing refrigerator support" breaks ACPI S3 and S4 support for me (2.6.8-rc2-bk7) and my laptop (NEC Chrom@). When resuming, my 3c59x CardBus NIC is not powered up forcing me to eject it, then plug it again.

Looking again at the patch, I'm not sure which diff would be relevant to you. When the card is running, do you have a kIrDAd thread?

Regards,

Nigel

#5 lore raw
Quoted 11 lines · Nigel Cunningham

Hi again.

Quoted 8 lines · Felipe Alfaro Solana
Quoted 5 lines · Andrew Morton

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

Quoted 7 lines
Quoted 4 lines

BTW, the "Add missing refrigerator support" breaks ACPI S3 and S4 support for me (2.6.8-rc2-bk7) and my laptop (NEC Chrom@). When resuming, my 3c59x CardBus NIC is not powered up forcing me to eject it, then plug it again.

Looking again at the patch, I'm not sure which diff would be relevant to you. When the card is running, do you have a kIrDAd thread?

kirdad? No... That sounds like Infrared which my laptop does not have. Here is a digest of ps -axf:

  PID TTY      STAT   TIME COMMAND
    1 ?        S      0:00 init [5]
    2 ?        S<     0:03 [irqd/0]
    3 ?        S<     0:00 [events/0]
    4 ?        S<     0:00  \_ [khelper]
    5 ?        S<     0:00  \_ [kacpid]
   22 ?        S<     0:00  \_ [kblockd/0]
   32 ?        S      0:00  \_ [pdflush]
   33 ?        S      0:00  \_ [pdflush]
   35 ?        S<     0:00  \_ [aio/0]
   36 ?        S<     0:00  \_ [xfslogd/0]
   37 ?        S<     0:00  \_ [xfsdatad/0]
   34 ?        S      0:00 [kswapd0]
   38 ?        S      0:00 [xfsbufd]
  120 ?        S      0:00 [kseriod]
  125 ?        S      0:00 [xfssyncd]
  273 ?        Ss     0:00 minilogd
  286 ?        S      0:00 [xfssyncd]
  287 ?        S      0:00 [xfssyncd]
  567 ?        S      0:00 [khubd]
  871 ?        S      0:00 [pccardd]
  877 ?        S      0:00 [pccardd]

Anything else? :-)

#6 lore raw

Hi.

Quoted 1 line · Felipe Alfaro Solana

kirdad? No... That sounds like Infrared which my laptop does not have.

Did to me too. I was clutching at straws. :>

Quoted 16 lines

Here is a digest of ps -axf:

  PID TTY      STAT   TIME COMMAND
    1 ?        S      0:00 init [5]
    2 ?        S<     0:03 [irqd/0]
    3 ?        S<     0:00 [events/0]
    4 ?        S<     0:00  \_ [khelper]
    5 ?        S<     0:00  \_ [kacpid]
   22 ?        S<     0:00  \_ [kblockd/0]
   32 ?        S      0:00  \_ [pdflush]
   33 ?        S      0:00  \_ [pdflush]
   35 ?        S<     0:00  \_ [aio/0]
   36 ?        S<     0:00  \_ [xfslogd/0]
   37 ?        S<     0:00  \_ [xfsdatad/0]
   34 ?        S      0:00 [kswapd0]
   38 ?        S      0:00 [xfsbufd]
Quoted 8 lines
  120 ?        S      0:00 [kseriod]
  125 ?        S      0:00 [xfssyncd]
  273 ?        Ss     0:00 minilogd
  286 ?        S      0:00 [xfssyncd]
  287 ?        S      0:00 [xfssyncd]
  567 ?        S      0:00 [khubd]
  871 ?        S      0:00 [pccardd]
  877 ?        S      0:00 [pccardd]

It doesn't look like I've touched any of those threads. I have doubts about irqd/0 (is that kirqd reworked?), so you might try making setting PF_NOFREEZE and seeing if it makes a difference. I haven't done the switch to rc2-mm1 yet, so haven't gotten to those issues.

Nigel

#7 lore raw
Quoted 32 lines · Nigel Cunningham

Hi.

Quoted 1 line · Felipe Alfaro Solana

kirdad? No... That sounds like Infrared which my laptop does not have.

Did to me too. I was clutching at straws. :>

Quoted 16 lines

Here is a digest of ps -axf:

  PID TTY      STAT   TIME COMMAND
    1 ?        S      0:00 init [5]
    2 ?        S<     0:03 [irqd/0]
    3 ?        S<     0:00 [events/0]
    4 ?        S<     0:00  \_ [khelper]
    5 ?        S<     0:00  \_ [kacpid]
   22 ?        S<     0:00  \_ [kblockd/0]
   32 ?        S      0:00  \_ [pdflush]
   33 ?        S      0:00  \_ [pdflush]
   35 ?        S<     0:00  \_ [aio/0]
   36 ?        S<     0:00  \_ [xfslogd/0]
   37 ?        S<     0:00  \_ [xfsdatad/0]
   34 ?        S      0:00 [kswapd0]
   38 ?        S      0:00 [xfsbufd]
Quoted 8 lines
  120 ?        S      0:00 [kseriod]
  125 ?        S      0:00 [xfssyncd]
  273 ?        Ss     0:00 minilogd
  286 ?        S      0:00 [xfssyncd]
  287 ?        S      0:00 [xfssyncd]
  567 ?        S      0:00 [khubd]
  871 ?        S      0:00 [pccardd]
  877 ?        S      0:00 [pccardd]
Quoted 4 lines

It doesn't look like I've touched any of those threads. I have doubts about irqd/0 (is that kirqd reworked?), so you might try making setting PF_NOFREEZE and seeing if it makes a difference. I haven't done the switch to rc2-mm1 yet, so haven't gotten to those issues.

kirqd is voluntary-preempt patch by Ingo Molnar. I have also applied several other patches, like Con's Staircase scheduler policy and some latency fixes.

#8 lore raw

Hi.

Quoted 8 lines · Felipe Alfaro Solana
Quoted 4 lines

It doesn't look like I've touched any of those threads. I have doubts about irqd/0 (is that kirqd reworked?), so you might try making setting PF_NOFREEZE and seeing if it makes a difference. I haven't done the switch to rc2-mm1 yet, so haven't gotten to those issues.

kirqd is voluntary-preempt patch by Ingo Molnar. I have also applied several other patches, like Con's Staircase scheduler policy and some latency fixes.

Okay. So, just to make sure I understand you correctly, suspending works fine with all of these other patches added and adding the extra refrigerator calls breaks it. Are you at all able to narrow it down to a particular change?

Regards,

Nigel

#9 lore raw
Quoted 8 lines · Nigel Cunningham

Hi.

Quoted 5 lines · Felipe Alfaro Solana
Quoted 4 lines

It doesn't look like I've touched any of those threads. I have doubts about irqd/0 (is that kirqd reworked?), so you might try making setting PF_NOFREEZE and seeing if it makes a difference. I haven't done the switch to rc2-mm1 yet, so haven't gotten to those issues.

Quoted 8 lines
Quoted 3 lines

kirqd is voluntary-preempt patch by Ingo Molnar. I have also applied several other patches, like Con's Staircase scheduler policy and some latency fixes.

Okay. So, just to make sure I understand you correctly, suspending works fine with all of these other patches added and adding the extra refrigerator calls breaks it. Are you at all able to narrow it down to a particular change?

Exactly! I'm currently running a highly patched kernel based on 2.6.8- rc2-bk7 plus Con's work and Ingo's voluntary preempt. They work fine when suspending to memory (S3) and to disk (S4 via swsusp), but adding your kthread freezer flags to the mix keeps my CardBus NIC from being recognized when resuming from S3: I need to unplug it, then plug it to make it functional again.

However, I'm not sure what causes this behavior.

#10 lore raw

Hi.

Quoted 5 lines · Felipe Alfaro Solana
Quoted 4 lines

Okay. So, just to make sure I understand you correctly, suspending works fine with all of these other patches added and adding the extra refrigerator calls breaks it. Are you at all able to narrow it down to a particular change?

Quoted 8 lines

Exactly! I'm currently running a highly patched kernel based on 2.6.8- rc2-bk7 plus Con's work and Ingo's voluntary preempt. They work fine when suspending to memory (S3) and to disk (S4 via swsusp), but adding your kthread freezer flags to the mix keeps my CardBus NIC from being recognized when resuming from S3: I need to unplug it, then plug it to make it functional again.

However, I'm not sure what causes this behavior.

Could you please try reversing each of the changes in my patch until it starts working? The NFS ones could be done all at once - they should be irrelevant to you anyway IIRC.

Regards,

Nigel

#11 lore raw
Quoted 15 lines · Nigel Cunningham

Hi.

Quoted 12 lines · Felipe Alfaro Solana
Quoted 4 lines

Okay. So, just to make sure I understand you correctly, suspending works fine with all of these other patches added and adding the extra refrigerator calls breaks it. Are you at all able to narrow it down to a particular change?

Exactly! I'm currently running a highly patched kernel based on 2.6.8- rc2-bk7 plus Con's work and Ingo's voluntary preempt. They work fine when suspending to memory (S3) and to disk (S4 via swsusp), but adding your kthread freezer flags to the mix keeps my CardBus NIC from being recognized when resuming from S3: I need to unplug it, then plug it to make it functional again.

Quoted 5 lines
Quoted 1 line

However, I'm not sure what causes this behavior.

Could you please try reversing each of the changes in my patch until it starts working? The NFS ones could be done all at once - they should be irrelevant to you anyway IIRC.

That's what I'm doing right now ;-)
Please, give me a few hours to post the result.

#12 lore raw
In reply to Nigel Cunningham
Quoted 32 lines · Nigel Cunningham

Hi.

Quoted 1 line · Felipe Alfaro Solana

kirdad? No... That sounds like Infrared which my laptop does not have.

Did to me too. I was clutching at straws. :>

Quoted 16 lines

Here is a digest of ps -axf:

  PID TTY      STAT   TIME COMMAND
    1 ?        S      0:00 init [5]
    2 ?        S<     0:03 [irqd/0]
    3 ?        S<     0:00 [events/0]
    4 ?        S<     0:00  \_ [khelper]
    5 ?        S<     0:00  \_ [kacpid]
   22 ?        S<     0:00  \_ [kblockd/0]
   32 ?        S      0:00  \_ [pdflush]
   33 ?        S      0:00  \_ [pdflush]
   35 ?        S<     0:00  \_ [aio/0]
   36 ?        S<     0:00  \_ [xfslogd/0]
   37 ?        S<     0:00  \_ [xfsdatad/0]
   34 ?        S      0:00 [kswapd0]
   38 ?        S      0:00 [xfsbufd]
Quoted 8 lines
  120 ?        S      0:00 [kseriod]
  125 ?        S      0:00 [xfssyncd]
  273 ?        Ss     0:00 minilogd
  286 ?        S      0:00 [xfssyncd]
  287 ?        S      0:00 [xfssyncd]
  567 ?        S      0:00 [khubd]
  871 ?        S      0:00 [pccardd]
  877 ?        S      0:00 [pccardd]
Quoted 4 lines

It doesn't look like I've touched any of those threads. I have doubts about irqd/0 (is that kirqd reworked?), so you might try making setting PF_NOFREEZE and seeing if it makes a difference. I haven't done the switch to rc2-mm1 yet, so haven't gotten to those issues.

Well, I've tried the kthread freezer patch against 2.6.8-rc2-mm1 and it works fine. However, with kthread freezer applied, suspending and resuming is much slower (around 5 seconds slower). Thus, I guess all my problems must be related to some specific patch I'm applying against the current -bk tree.

I'll keep investigating this issue, but I think voluntary-preempt might have some strange interactions with these kthread changes.

#13 lore raw

Hi.

Quoted 5 lines · Felipe Alfaro Solana

Well, I've tried the kthread freezer patch against 2.6.8-rc2-mm1 and it works fine. However, with kthread freezer applied, suspending and resuming is much slower (around 5 seconds slower). Thus, I guess all my problems must be related to some specific patch I'm applying against the current -bk tree.

Okay. That might be due to the treatment of ksoftirqd, which Pavel and I have agreed needs changing.

Quoted 2 lines

I'll keep investigating this issue, but I think voluntary-preempt might have some strange interactions with these kthread changes.

Me too, when I get a chance. I'm running rc2-mm1 now, and it seems okay. Especially since I turned off USB 2 support in the BIOS :>

Nigel

#14 lore raw
In reply to Andrew Morton

Hi.

Quoted 5 lines · Andrew Morton

hm. In some ways I'd prefer to see new create_singlethread_workqueue_freezer(char *) or whatever, rather than adding an extra argument. That's neater, smaller code and forward-compatible.

Quoted 7 lines

But then again, the advantage of breaking the build for unconverted code is that it makes people think about what their threads should be doing, so let's go your way.

The one concern I'd have is that $RANDOM_KERNEL_DEVELOPER probably doesn't have a clue whether or not his kernel thread should be setting PF_NOFREEZE. What are the guidelines here?

If a process might be/is needed to get the image to the storage device, it should be NOFREEZE. (Thus, USB threads might be NOFREEZE to allow for USB storage, likewise for SCSI). When I get an NFS writer done, the threads for network cards could probably be made NOFREEZE too. Things like kjournald don't need to run during suspend (even if we're writing to a swapfile in an ext3 partition), so can be 0 for now (SYNCTHREAD later).

Quoted 5 lines

wrt your "Add missing refrigerator support" patch: I'll suck that up, but be aware that there's a big i2o patch in -mm which basically rips out the driver which you just fixed up. Perhaps you can send Markus Lidel <[email protected]> and I a fix for that version of the driver sometime?

I've just started following your mm patches, and will do updates for last nights release shortly.

Regards,

Nigel

#15 lore raw
In reply to Nigel Cunningham

Hi!

Quoted 2 lines

At the moment, all kthreads have PF_NOFREEZE set, meaning that they're not refrigerated during a suspend. This isn't right for some threads.

Looks good, but see comments below.

drivers/block/pktcdvd.c line 2378

Quoted 11 lines
drivers/block/pktcdvd.c +1 -1
@@ -2372,7 +2372,7 @@
 
 	pkt_init_queue(pd);
 
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;

What if someone does swapon /dev/pktdvd0?

drivers/block/pktcdvd.c line 1244

Quoted 10 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/dm-raid1.c 2004-07-28 16:48:44.000000000 +1000

drivers/block/pktcdvd.c +1 -1
@@ -1238,7 +1238,7 @@
 	if (r)
 		return r;
 
-	_kmirrord_wq = create_workqueue("kmirrord");
+	_kmirrord_wq = create_workqueue("kmirrord", PF_NOFREEZE);
 	if (!_kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		dm_dirty_log_exit();

I'm not 100% certain what kmirrord does, but we certainly do not want raid array to be reconstructed while suspending.

linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/aio.c

fs/aio.c line 75

Quoted 11 lines
fs/aio.c +1 -1
@@ -69,7 +69,7 @@
 	kioctx_cachep = kmem_cache_create("kioctx", sizeof(struct kioctx),
 				0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
 
-	aio_wq = create_workqueue("aio");
+	aio_wq = create_workqueue("aio", PF_NOFREEZE);
 
 	pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page));
 

Are you sure? Unless swsusp itself uses aio, we want this to freeze.

linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/sched.c 2004-07-28 16:43:48.000000000 +1000

fs/aio.c line 3557

Quoted 10 lines
fs/aio.c +2 -1
@@ -3550,7 +3550,8 @@
 
 	switch (action) {
 	case CPU_UP_PREPARE:
-		p = kthread_create(migration_thread, hcpu, "migration/%d",cpu);
+		p = kthread_create(migration_thread, hcpu, 0,
+				"migration/%d",cpu);
 		if (IS_ERR(p))
 			return NOTIFY_BAD;
 		p->flags |= PF_NOFREEZE;

Ugh, creating thread normally only to add PF_NOFREEZE 2 lines later looks bad.

fs/aio.c line 431

Quoted 10 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/softirq.c 2004-07-28 16:43:48.000000000 +1000

fs/aio.c +1 -1
@@ -425,7 +425,7 @@
 	case CPU_UP_PREPARE:
 		BUG_ON(per_cpu(tasklet_vec, hotcpu).list);
 		BUG_ON(per_cpu(tasklet_hi_vec, hotcpu).list);
-		p = kthread_create(ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu);
+		p = kthread_create(ksoftirqd, hcpu, 0, "ksoftirqd/%d", hotcpu);
 		if (IS_ERR(p)) {
 			printk("ksoftirqd for %i failed\n", hotcpu);
 			return NOTIFY_BAD;

I guess softinterrupts may be neccessary for suspend... Random drivers may use them, right?

Pavel

Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         
#16 lore raw

Hi.

Quoted 12 lines · Pavel Machek

Hi!

Quoted 2 lines

At the moment, all kthreads have PF_NOFREEZE set, meaning that they're not refrigerated during a suspend. This isn't right for some threads.

Looks good, but see comments below.

Quoted 6 lines
drivers/block/pktcdvd.c +0 -0
@@ -2372,7 +2372,7 @@
 
 	pkt_init_queue(pd);
 
Quoted 7 lines

drivers/block/pktcdvd.c line 2378

Quoted 5 lines
drivers/block/pktcdvd.c +1 -1
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;

What if someone does swapon /dev/pktdvd0?

Sorry. That's my ignorance. I thought the packet writer was only for writing :>

Quoted 5 lines
Quoted 5 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/dm-raid1.c 2004-07-28 16:48:44.000000000 +1000

drivers/block/pktcdvd.c +0 -0
@@ -1238,7 +1238,7 @@
 	if (r)
 		return r;
 
Quoted 9 lines

drivers/block/pktcdvd.c line 1244

Quoted 5 lines
drivers/block/pktcdvd.c +1 -1
-	_kmirrord_wq = create_workqueue("kmirrord");
+	_kmirrord_wq = create_workqueue("kmirrord", PF_NOFREEZE);
 	if (!_kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		dm_dirty_log_exit();

I'm not 100% certain what kmirrord does, but we certainly do not want raid array to be reconstructed while suspending.

Mmm. Again, I plead picking it based on what I thought the code did. Can we get an author to say which it should be?

Quoted 7 lines

linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/aio.c

Quoted 6 lines
fs/aio.c +0 -0
@@ -69,7 +69,7 @@
 	kioctx_cachep = kmem_cache_create("kioctx", sizeof(struct kioctx),
 				0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
 
Quoted 7 lines

fs/aio.c line 75

Quoted 5 lines
fs/aio.c +1 -1
-	aio_wq = create_workqueue("aio");
+	aio_wq = create_workqueue("aio", PF_NOFREEZE);
 
 	pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page));
 

Are you sure? Unless swsusp itself uses aio, we want this to freeze.

I think it was needed to get the writes happening. Even if its wrong, it shouldn't matter as the only I/O pending should be what we're doing.

Quoted 14 lines

linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/sched.c 2004-07-28 16:43:48.000000000 +1000

fs/aio.c line 3557

Quoted 10 lines
fs/aio.c +2 -1
@@ -3550,7 +3550,8 @@
 
 	switch (action) {
 	case CPU_UP_PREPARE:
-		p = kthread_create(migration_thread, hcpu, "migration/%d",cpu);
+		p = kthread_create(migration_thread, hcpu, 0,
+				"migration/%d",cpu);
 		if (IS_ERR(p))
 			return NOTIFY_BAD;
 		p->flags |= PF_NOFREEZE;

Ugh, creating thread normally only to add PF_NOFREEZE 2 lines later looks bad.

Yes. Tunnel vision! Humble apologies.

Quoted 5 lines
Quoted 5 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/softirq.c 2004-07-28 16:43:48.000000000 +1000

fs/aio.c +0 -0
@@ -425,7 +425,7 @@
 	case CPU_UP_PREPARE:
 		BUG_ON(per_cpu(tasklet_vec, hotcpu).list);
 		BUG_ON(per_cpu(tasklet_hi_vec, hotcpu).list);
Quoted 8 lines

fs/aio.c line 431

Quoted 5 lines
fs/aio.c +1 -1
-		p = kthread_create(ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu);
+		p = kthread_create(ksoftirqd, hcpu, 0, "ksoftirqd/%d", hotcpu);
 		if (IS_ERR(p)) {
 			printk("ksoftirqd for %i failed\n", hotcpu);
 			return NOTIFY_BAD;

I guess softinterrupts may be neccessary for suspend... Random drivers may use them, right?

I made this change at least a month ago and no one using suspend2 has had any problems since, so perhaps not. Then again, with the voluntary preemption (from what I've seen of comments about it) this would be a definite yes.

Nigel

#17 lore raw

Hi!

Quoted 10 lines
Quoted 7 lines

drivers/block/pktcdvd.c line 2378

Quoted 5 lines
drivers/block/pktcdvd.c +1 -1
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;

What if someone does swapon /dev/pktdvd0?

Sorry. That's my ignorance. I thought the packet writer was only for writing :>

Well, swapon /dev/pktdvd would be *very* bad idea as optical drives are very slow, but PF_NOFREEZE is more correct here.

Quoted 15 lines
Quoted 5 lines
Quoted 5 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/drivers/md/dm-raid1.c 2004-07-28 16:48:44.000000000 +1000

drivers/md/dm-raid1.c +0 -0
@@ -1238,7 +1238,7 @@
 	if (r)
 		return r;
 
Quoted 9 lines

drivers/md/dm-raid1.c line 1244

Quoted 5 lines
drivers/md/dm-raid1.c +1 -1
-	_kmirrord_wq = create_workqueue("kmirrord");
+	_kmirrord_wq = create_workqueue("kmirrord", PF_NOFREEZE);
 	if (!_kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		dm_dirty_log_exit();

I'm not 100% certain what kmirrord does, but we certainly do not want raid array to be reconstructed while suspending.

Quoted 2 lines

Mmm. Again, I plead picking it based on what I thought the code did. Can we get an author to say which it should be?

I'd make it stop to be safe, and see what happens ;-).

Quoted 10 lines
Quoted 10 lines

linux-2.6.8-rc1-mm1-kthread_refrigerator/fs/aio.c

Quoted 9 lines
fs/aio.c +1 -1
@@ -69,7 +69,7 @@
 	kioctx_cachep = kmem_cache_create("kioctx", sizeof(struct kioctx),
 				0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
 
-	aio_wq = create_workqueue("aio");
+	aio_wq = create_workqueue("aio", PF_NOFREEZE);
 
Quoted 7 lines
Quoted 4 lines

fs/aio.c line 75

Quoted 2 lines
fs/aio.c +0 -0
 	pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page));
 

Are you sure? Unless swsusp itself uses aio, we want this to freeze.

I think it was needed to get the writes happening. Even if its wrong, it shouldn't matter as the only I/O pending should be what we're doing.

I'm little bit nervous about this one. I'd make it stop, and see if it breaks... From quick look at aio.c it really does *not* seem we need this.

Quoted 14 lines
Quoted 5 lines
Quoted 5 lines

+++ linux-2.6.8-rc1-mm1-kthread_refrigerator/kernel/softirq.c 2004-07-28 16:43:48.000000000 +1000

fs/aio.c +0 -0
@@ -425,7 +425,7 @@
 	case CPU_UP_PREPARE:
 		BUG_ON(per_cpu(tasklet_vec, hotcpu).list);
 		BUG_ON(per_cpu(tasklet_hi_vec, hotcpu).list);
Quoted 8 lines

fs/aio.c line 431

Quoted 5 lines
fs/aio.c +1 -1
-		p = kthread_create(ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu);
+		p = kthread_create(ksoftirqd, hcpu, 0, "ksoftirqd/%d", hotcpu);
 		if (IS_ERR(p)) {
 			printk("ksoftirqd for %i failed\n", hotcpu);
 			return NOTIFY_BAD;

I guess softinterrupts may be neccessary for suspend... Random drivers may use them, right?

Quoted 4 lines

I made this change at least a month ago and no one using suspend2 has had any problems since, so perhaps not. Then again, with the voluntary preemption (from what I've seen of comments about it) this would be a definite yes.

Ok.
Pavel

Signature · 2 lines
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
#18 lore raw

Hi.

Quoted 10 lines · Pavel Machek

Hi!

Quoted 8 lines
Quoted 7 lines

drivers/block/pktcdvd.c line 2378

Quoted 5 lines
drivers/block/pktcdvd.c +1 -1
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;

What if someone does swapon /dev/pktdvd0?

Quoted 5 lines
Quoted 2 lines

Sorry. That's my ignorance. I thought the packet writer was only for writing :>

Well, swapon /dev/pktdvd would be *very* bad idea as optical drives are very slow, but PF_NOFREEZE is more correct here.

Okay. I'll do a new patch for Andrew for this and the following corrections.

[...]

Quoted 10 lines
Quoted 8 lines
Quoted 3 lines

I guess softinterrupts may be neccessary for suspend... Random drivers may use them, right?

I made this change at least a month ago and no one using suspend2 has had any problems since, so perhaps not. Then again, with the voluntary preemption (from what I've seen of comments about it) this would be a definite yes.

Ok.

Just in case I wasn't clear, by 'a definite yes', I mean you're absolutely right - it should be NOFREEZE.

Regards,

Nigel

#19 lore raw

Re: [Patch] Per kthread freezer flags (Version 2)

In reply to Pavel Machek

Okay.. how does this look?

I applied your changes and fixed a couple of typos I noticed. I also added support for the hcvs thread, which is new since rc1-mm1. (This is against rc2-mm1).

Regards,

Nigel

drivers/acpi/osl.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/acpi/osl.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/acpi/osl.c
@@ -81,7 +81,7 @@
 		return AE_NULL_ENTRY;
 	}
 #endif
-	kacpid_wq = create_singlethread_workqueue("kacpid");
+	kacpid_wq = create_singlethread_workqueue("kacpid", 0);
 	BUG_ON(!kacpid_wq);
 
 	return AE_OK;
drivers/block/ll_rw_blk.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/block/ll_rw_blk.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/block/ll_rw_blk.c
@@ -2998,7 +2998,7 @@
 
 int __init blk_dev_init(void)
 {
-	kblockd_workqueue = create_workqueue("kblockd");
+	kblockd_workqueue = create_workqueue("kblockd", PF_NOFREEZE);
 	if (!kblockd_workqueue)
 		panic("Failed to create kblockd\n");
 
drivers/block/pktcdvd.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/block/pktcdvd.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/block/pktcdvd.c
@@ -2350,7 +2350,7 @@
 
 	pkt_init_queue(pd);
 
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, PF_NOFREEZE, "%s", pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;
drivers/char/hvcs.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/char/hvcs.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/char/hvcs.c
@@ -1218,7 +1218,7 @@
 	hvcs_pi_lock = SPIN_LOCK_UNLOCKED;
 	hvcs_pi_buff = kmalloc(PAGE_SIZE, GFP_KERNEL);
 
-	hvcs_task = kthread_run(khvcsd, NULL, "khvcsd");
+	hvcs_task = kthread_run(khvcsd, NULL, PF_NOFREEZE, "khvcsd");
 	if (IS_ERR(hvcs_task)) {
 		printk("khvcsd creation failed.  Driver not loaded.\n");
 		kfree(hvcs_pi_buff);
drivers/md/dm-crypt.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/md/dm-crypt.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/md/dm-crypt.c
@@ -758,7 +758,7 @@
 	if (!_crypt_io_pool)
 		return -ENOMEM;
 
-	_kcryptd_workqueue = create_workqueue("kcryptd");
+	_kcryptd_workqueue = create_workqueue("kcryptd", PF_NOFREEZE);
 	if (!_kcryptd_workqueue) {
 		r = -ENOMEM;
 		DMERR(PFX "couldn't create kcryptd");
drivers/md/dm-raid1.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/md/dm-raid1.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/md/dm-raid1.c
@@ -1238,7 +1238,7 @@
 	if (r)
 		return r;
 
-	_kmirrord_wq = create_workqueue("kmirrord");
+	_kmirrord_wq = create_workqueue("kmirrord", 0);
 	if (!_kmirrord_wq) {
 		DMERR("couldn't start kmirrord");
 		dm_dirty_log_exit();
drivers/md/kcopyd.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/md/kcopyd.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/md/kcopyd.c
@@ -609,7 +609,7 @@
 		return r;
 	}
 
-	_kcopyd_wq = create_singlethread_workqueue("kcopyd");
+	_kcopyd_wq = create_singlethread_workqueue("kcopyd", 0);
 	if (!_kcopyd_wq) {
 		jobs_exit();
 		up(&kcopyd_init_lock);
drivers/net/wan/sdlamain.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/net/wan/sdlamain.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/net/wan/sdlamain.c
@@ -240,7 +240,7 @@
 	printk(KERN_INFO "%s v%u.%u %s\n",
 		fullname, DRV_VERSION, DRV_RELEASE, copyright);
 
-	wanpipe_wq = create_workqueue("wanpipe_wq");
+	wanpipe_wq = create_workqueue("wanpipe_wq", 0);
 	if (!wanpipe_wq)
 		return -ENOMEM;
 
drivers/s390/cio/device.c +4 -3
diff -ruN linux-2.6.8-rc2-mm1/drivers/s390/cio/device.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/s390/cio/device.c
@@ -151,15 +151,16 @@
 	init_waitqueue_head(&ccw_device_init_wq);
 	atomic_set(&ccw_device_init_count, 0);
 
-	ccw_device_work = create_singlethread_workqueue("cio");
+	ccw_device_work = create_singlethread_workqueue("cio", 0);
 	if (!ccw_device_work)
 		return -ENOMEM; /* FIXME: better errno ? */
-	ccw_device_notify_work = create_singlethread_workqueue("cio_notify");
+	ccw_device_notify_work = create_singlethread_workqueue("cio_notify",
+			0);
 	if (!ccw_device_notify_work) {
 		ret = -ENOMEM; /* FIXME: better errno ? */
 		goto out_err;
 	}
-	slow_path_wq = create_singlethread_workqueue("kslowcrw");
+	slow_path_wq = create_singlethread_workqueue("kslowcrw", 0);
 	if (!slow_path_wq) {
 		ret = -ENOMEM; /* FIXME: better errno ? */
 		goto out_err;
drivers/scsi/libata-core.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/drivers/scsi/libata-core.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/drivers/scsi/libata-core.c
@@ -3440,7 +3440,7 @@
 
 static int __init ata_init(void)
 {
-	ata_wq = create_workqueue("ata");
+	ata_wq = create_workqueue("ata", PF_NOFREEZE);
 	if (!ata_wq)
 		return -ENOMEM;
 
fs/aio.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/fs/aio.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/fs/aio.c
@@ -69,7 +69,7 @@
 	kioctx_cachep = kmem_cache_create("kioctx", sizeof(struct kioctx),
 				0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
 
-	aio_wq = create_workqueue("aio");
+	aio_wq = create_workqueue("aio", 0);
 
 	pr_debug("aio_setup: sizeof(struct page) = %d\n", (int)sizeof(struct page));
 
fs/reiserfs/journal.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/fs/reiserfs/journal.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/fs/reiserfs/journal.c
@@ -2483,7 +2483,7 @@
 
   reiserfs_mounted_fs_count++ ;
   if (reiserfs_mounted_fs_count <= 1)
-    commit_wq = create_workqueue("reiserfs");
+    commit_wq = create_workqueue("reiserfs", 0);
 
   INIT_WORK(&journal->j_work, flush_async_commits, p_s_sb);
   return 0 ;
fs/xfs/linux-2.6/xfs_buf.c +2 -2
diff -ruN linux-2.6.8-rc2-mm1/fs/xfs/linux-2.6/xfs_buf.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/fs/xfs/linux-2.6/xfs_buf.c
@@ -1746,11 +1746,11 @@
 {
 	int		rval;
 
-	pagebuf_logio_workqueue = create_workqueue("xfslogd");
+	pagebuf_logio_workqueue = create_workqueue("xfslogd", 0);
 	if (!pagebuf_logio_workqueue)
 		return -ENOMEM;
 
-	pagebuf_dataio_workqueue = create_workqueue("xfsdatad");
+	pagebuf_dataio_workqueue = create_workqueue("xfsdatad", 0);
 	if (!pagebuf_dataio_workqueue) {
 		destroy_workqueue(pagebuf_logio_workqueue);
 		return -ENOMEM;
include/linux/kthread.h +7 -2
diff -ruN linux-2.6.8-rc2-mm1/include/linux/kthread.h linux-2.6.8-rc2-mm1-kthread_refrigerator_support/include/linux/kthread.h
@@ -25,20 +25,25 @@
  */
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
+				   unsigned long freezer_flags,
 				   const char namefmt[], ...);
 
 /**
  * kthread_run: create and wake a thread.
  * @threadfn: the function to run until signal_pending(current).
  * @data: data ptr for @threadfn.
+ * @freezer_flags: process flags that should be used for freezing.
+ * 	PF_NOFREEZE if also needed for writing the image.
+ * 	0 otherwise.
  * @namefmt: printf-style name for the thread.
  *
  * Description: Convenient wrapper for kthread_create() followed by
  * wake_up_process().  Returns the kthread, or ERR_PTR(-ENOMEM). */
-#define kthread_run(threadfn, data, namefmt, ...)			   \
+#define kthread_run(threadfn, data, freezer_flags, namefmt, ...)	   \
 ({									   \
 	struct task_struct *__k						   \
-		= kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \
+		= kthread_create(threadfn, data, freezer_flags, 	   \
+			namefmt, ## __VA_ARGS__);			   \
 	if (!IS_ERR(__k))						   \
 		wake_up_process(__k);					   \
 	__k;								   \
include/linux/workqueue.h +4 -3
diff -ruN linux-2.6.8-rc2-mm1/include/linux/workqueue.h linux-2.6.8-rc2-mm1-kthread_refrigerator_support/include/linux/workqueue.h
@@ -51,9 +51,10 @@
 	} while (0)
 
 extern struct workqueue_struct *__create_workqueue(const char *name,
-						    int singlethread);
-#define create_workqueue(name) __create_workqueue((name), 0)
-#define create_singlethread_workqueue(name) __create_workqueue((name), 1)
+						    int singlethread,
+						    unsigned long freezer_flag);
+#define create_workqueue(name, flags) __create_workqueue((name), 0, flags)
+#define create_singlethread_workqueue(name, flags) __create_workqueue((name), 1, flags)
 
 extern void destroy_workqueue(struct workqueue_struct *wq);
 
kernel/kmod.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/kernel/kmod.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/kmod.c
@@ -274,6 +274,6 @@
 
 void __init usermodehelper_init(void)
 {
-	khelper_wq = create_singlethread_workqueue("khelper");
+	khelper_wq = create_singlethread_workqueue("khelper", 0);
 	BUG_ON(!khelper_wq);
 }
kernel/kthread.c +8 -0
diff -ruN linux-2.6.8-rc2-mm1/kernel/kthread.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/kthread.c
@@ -19,6 +19,7 @@
 	/* Information passed to kthread() from keventd. */
 	int (*threadfn)(void *data);
 	void *data;
+	unsigned long freezer_flags;
 	struct completion started;
 
 	/* Result passed back to kthread_create() from keventd. */
@@ -65,6 +66,7 @@
 	void *data;
 	sigset_t blocked;
 	int ret = -EINTR;
+	unsigned long flags_used;
 
 	kthread_exit_files();
 
@@ -80,6 +82,10 @@
 	/* By default we can run anywhere, unlike keventd. */
 	set_cpus_allowed(current, CPU_MASK_ALL);
 
+	/* Validate and set our freezer flags */
+	flags_used = create->freezer_flags & PF_NOFREEZE;
+	current->flags |= flags_used;
+
 	/* OK, tell user we're spawned, wait for stop or wakeup */
 	__set_current_state(TASK_INTERRUPTIBLE);
 	complete(&create->started);
@@ -115,6 +121,7 @@
 
 struct task_struct *kthread_create(int (*threadfn)(void *data),
 				   void *data,
+				   unsigned long freezer_flags,
 				   const char namefmt[],
 				   ...)
 {
@@ -123,6 +130,7 @@
 
 	create.threadfn = threadfn;
 	create.data = data;
+	create.freezer_flags = freezer_flags;
 	init_completion(&create.started);
 	init_completion(&create.done);
 
kernel/sched.c +2 -2
diff -ruN linux-2.6.8-rc2-mm1/kernel/sched.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/sched.c
@@ -3549,10 +3549,10 @@
 
 	switch (action) {
 	case CPU_UP_PREPARE:
-		p = kthread_create(migration_thread, hcpu, "migration/%d",cpu);
+		p = kthread_create(migration_thread, hcpu, 0,
+				"migration/%d",cpu);
 		if (IS_ERR(p))
 			return NOTIFY_BAD;
-		p->flags |= PF_NOFREEZE;
 		kthread_bind(p, cpu);
 		/* Must be high prio: stop_machine expects to yield to it. */
 		rq = task_rq_lock(p, &flags);
kernel/softirq.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/kernel/softirq.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/softirq.c
@@ -425,7 +425,7 @@
 	case CPU_UP_PREPARE:
 		BUG_ON(per_cpu(tasklet_vec, hotcpu).list);
 		BUG_ON(per_cpu(tasklet_hi_vec, hotcpu).list);
-		p = kthread_create(ksoftirqd, hcpu, "ksoftirqd/%d", hotcpu);
+		p = kthread_create(ksoftirqd, hcpu, PF_NOFREEZE, "ksoftirqd/%d", hotcpu);
 		if (IS_ERR(p)) {
 			printk("ksoftirqd for %i failed\n", hotcpu);
 			return NOTIFY_BAD;
kernel/stop_machine.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/kernel/stop_machine.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/stop_machine.c
@@ -174,7 +174,7 @@
 	if (cpu == NR_CPUS)
 		cpu = smp_processor_id();
 
-	p = kthread_create(do_stop, &smdata, "kstopmachine");
+	p = kthread_create(do_stop, &smdata, 0, "kstopmachine");
 	if (!IS_ERR(p)) {
 		kthread_bind(p, cpu);
 		wake_up_process(p);
kernel/workqueue.c +14 -9
diff -ruN linux-2.6.8-rc2-mm1/kernel/workqueue.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/kernel/workqueue.c
@@ -25,6 +25,7 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/kthread.h>
+#include <linux/suspend.h>
 
 /*
  * The per-CPU workqueue (if single thread, we always use cpu 0's).
@@ -186,8 +187,6 @@
 	struct k_sigaction sa;
 	sigset_t blocked;
 
-	current->flags |= PF_NOFREEZE;
-
 	set_user_nice(current, -10);
 
 	/* Block and flush all signals */
@@ -208,6 +207,8 @@
 			schedule();
 		else
 			__set_current_state(TASK_RUNNING);
+		if (current->flags & PF_FREEZE)
+			refrigerator(PF_FREEZE);
 		remove_wait_queue(&cwq->more_work, &wait);
 
 		if (!list_empty(&cwq->worklist))
@@ -277,7 +278,8 @@
 }
 
 static struct task_struct *create_workqueue_thread(struct workqueue_struct *wq,
-						   int cpu)
+						   int cpu,
+						   unsigned long freezer_flags)
 {
 	struct cpu_workqueue_struct *cwq = wq->cpu_wq + cpu;
 	struct task_struct *p;
@@ -292,9 +294,11 @@
 	init_waitqueue_head(&cwq->work_done);
 
 	if (is_single_threaded(wq))
-		p = kthread_create(worker_thread, cwq, "%s", wq->name);
+		p = kthread_create(worker_thread, cwq, freezer_flags, 
+				"%s", wq->name);
 	else
-		p = kthread_create(worker_thread, cwq, "%s/%d", wq->name, cpu);
+		p = kthread_create(worker_thread, cwq, freezer_flags,
+				"%s/%d", wq->name, cpu);
 	if (IS_ERR(p))
 		return NULL;
 	cwq->thread = p;
@@ -302,7 +306,8 @@
 }
 
 struct workqueue_struct *__create_workqueue(const char *name,
-					    int singlethread)
+					    int singlethread,
+					    unsigned long freezer_flags)
 {
 	int cpu, destroy = 0;
 	struct workqueue_struct *wq;
@@ -320,7 +325,7 @@
 	lock_cpu_hotplug();
 	if (singlethread) {
 		INIT_LIST_HEAD(&wq->list);
-		p = create_workqueue_thread(wq, 0);
+		p = create_workqueue_thread(wq, 0, freezer_flags);
 		if (!p)
 			destroy = 1;
 		else
@@ -330,7 +335,7 @@
 		list_add(&wq->list, &workqueues);
 		spin_unlock(&workqueue_lock);
 		for_each_online_cpu(cpu) {
-			p = create_workqueue_thread(wq, cpu);
+			p = create_workqueue_thread(wq, cpu, freezer_flags);
 			if (p) {
 				kthread_bind(p, cpu);
 				wake_up_process(p);
@@ -513,7 +518,7 @@
 void init_workqueues(void)
 {
 	hotcpu_notifier(workqueue_cpu_callback, 0);
-	keventd_wq = create_workqueue("events");
+	keventd_wq = create_workqueue("events", 0);
 	BUG_ON(!keventd_wq);
 }
 
mm/pdflush.c +1 -1
diff -ruN linux-2.6.8-rc2-mm1/mm/pdflush.c linux-2.6.8-rc2-mm1-kthread_refrigerator_support/mm/pdflush.c
@@ -215,7 +215,7 @@
 
 static void start_one_pdflush_thread(void)
 {
-	kthread_run(pdflush, NULL, "pdflush");
+	kthread_run(pdflush, NULL, 0, "pdflush");
 }
 
 static int __init pdflush_init(void)
#20 lore raw

Re: [Patch] Per kthread freezer flags (Version 2)

Hi!

Quoted 5 lines

Okay.. how does this look?

I applied your changes and fixed a couple of typos I noticed. I also added support for the hcvs thread, which is new since rc1-mm1. (This is against rc2-mm1).

Looks okay, thanks.
Pavel

Signature · 2 lines
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
#21 lore raw

Re: [Patch] Per kthread freezer flags (Version 2)

In reply to Nigel Cunningham
Quoted 5 lines · Nigel Cunningham

Okay.. how does this look?

I applied your changes and fixed a couple of typos I noticed. I also added support for the hcvs thread, which is new since rc1-mm1. (This is against rc2-mm1).

I'm still suffering from the NIC-won't-come-out-from-S3 (I need to unplug my CardBus NIC, then plug it again after resuming from S3) when applying this patch against 2.6.8-rc2-mm1... I'll keep reverting every hunk of the patch till I find the culprit.

#22 lore raw
In reply to Pavel Machek
Quoted 12 lines · Pavel Machek

Hi!

Quoted 10 lines
Quoted 7 lines

drivers/block/pktcdvd.c line 2378

Quoted 5 lines
drivers/block/pktcdvd.c +1 -1
-	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", pd->name);
+	pd->cdrw.thread = kthread_run(kcdrwd, pd, "%s", 0, pd->name);
 	if (IS_ERR(pd->cdrw.thread)) {
 		printk("pktcdvd: can't start kernel thread\n");
 		ret = -ENOMEM;

What if someone does swapon /dev/pktdvd0?

Sorry. That's my ignorance. I thought the packet writer was only for writing :>

pktcdvd devices are standard writable block devices, so you can use them for anything you want, including swapping.

Quoted 2 lines

Well, swapon /dev/pktdvd would be *very* bad idea as optical drives are very slow,

Agreed. Seek times before writes are in the range of 500-1000ms on my drives. This will probably make your machine unresponsive for a *very* long time when you start to swap to an optical disc.

It should work though, unless you wear out your disc before it's done swapping, in which case nasty things will probably happen. I don't think I/O errors on a swap device can be handled in a sane way by the kernel.

Quoted 1 line

but PF_NOFREEZE is more correct here.

Yes. What about this code in the main loop in kcdrwd?

			/* make swsusp happy with our thread */
			if (current->flags & PF_FREEZE)
				refrigerator(PF_FREEZE);

Should it still be there when the task is marked as PF_NOFREEZE?

Signature · 2 lines
Peter Osterlund - [email protected]
http://w1.894.telia.com/~u89404340
#23 lore raw

Hi.

Quoted 7 lines · Peter Osterlund

Yes. What about this code in the main loop in kcdrwd?

			/* make swsusp happy with our thread */
			if (current->flags & PF_FREEZE)
				refrigerator(PF_FREEZE);

Should it still be there when the task is marked as PF_NOFREEZE?

No, it's not needed if the thread is NOFREEZE.

Regards,

Nigel