[PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

This series is a followup to my proposal I brought up on Kernel Summit in 
Seoul. Noone seemed to had any principal objections, so let's have wider 
audience look into it.

In a nuthsell: freezing of kernel threads is horrible interface with 
unclear semantics and guarantees, and I am surprised it ever worked 
properly. Plus there are a lot of places that simply use it in a 
completely wrong way (which is not suprising, given the lack of defined 
semantics and requirements).

I've tested this over a series of suspend/resume cycles on several 
machines with at least ext4, btrfs and xfs, and it survived the testing 
without any harm.

Patch 1/3 	implements the actual change, and has a more detailed 
		explanation on "why?" and "how?" questions in the changelog

Patch 2/3	nukes all (hopefully) the calls to freezer from kthreads 
		in Linus' tree (as of 858e904bd71)

Patch 3/3	introduces WARN_ON() if anyone is trying to make use of 
		this again

Open questions / discussion points:

- is the way I am traversing list of superblocks backwards enough to 
  guarantee correct ordering? Especially: does this work as intended for 
  FUSE?

- should freezable workqueues be dealt with the same way? I haven't even 
  started to look into them in a serious way, but it seems like the 
  drivers that are making use of them would actually like to use proper
  PM callbacks instead

-- 
Jiri Kosina
SUSE Labs

[PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Freeze all filesystems during hibernation in favor of dropping kthread
freezing completely.

Kthread freezing has a history of not very well defined semantics.
Historically, it has been established to make sure that kthreads which are
helpers to writing out data to disk are frozen during hibernation at
well-defined state, so that it's guaranteed that they freeze only after all the
outstanding I/O made it to disk (userspace has already been frozen by that
time, so there is no new I/O being issued).

One of the issues with kthread freezer is that the places where try_to_freeze()
is called in kthreads is rather random / arbitrary. This stems mostly from
the fact that there is actually no good definition / list of requirements
that should be enforced on a frozen kthread (it's important to mention that
frozen kthread for example doesn't automatically imply that everything that
has been spawned asynchronously from it (such as timers) is stopped as well).

Basically the main argument why kthread freezer is not needed boils down to
this: the only facility that is needed during suspend: "no persistent fs
changes are allowed from now on".

	- this is something we get from fs freezing for free
	- Why do we issue all those try_to_freeze() calls in kthreads that
	  don't generate any I/O themselves at all?
	- Why do we issue all those try_to_freeze() calls in kthreads that
	  are actual I/O helpers? (if there is outstanding I/O, we *want*
	  it to happen before hibernating).

This patch removes freezing of kthreads during suspend, and issues a global
filesystem freeze instead.

We awoid freezing in-memory filesystems, because (a) they end up in the
image in a consistent state anyway (b) we will deadlock, as write to
/sys/power/state would never succeed.

The patch could have been made a bit nicer if generic iterate_supers()
could be used, but the locking (especially s_umount semantics) would have to
be redone, so that'd be something to postpone for later.
Also, the 'skip_virtual' flag is superfluous for now (as hibernation is the
only user of this facility for the time being), but I'd like to keep it
there to avoid further confusion regarding the fact that freeze_all_supers()
actually skips in-memory filesystems.

Based on prior work done by Rafael Wysocki.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 drivers/xen/manage.c     |  6 ----
 fs/super.c               | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/freezer.h  |  2 --
 include/linux/fs.h       |  2 ++
 kernel/power/hibernate.c |  5 ---
 kernel/power/power.h     | 20 +-----------
 kernel/power/process.c   | 63 +++++++++---------------------------
 kernel/power/user.c      |  9 ------
 8 files changed, 103 insertions(+), 88 deletions(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index e12bd36..d47716a 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -109,12 +109,6 @@ static void do_suspend(void)
 		goto out;
 	}
 
-	err = freeze_kernel_threads();
-	if (err) {
-		pr_err("%s: freeze kernel threads failed %d\n", __func__, err);
-		goto out_thaw;
-	}
-
 	err = dpm_suspend_start(PMSG_FREEZE);
 	if (err) {
 		pr_err("%s: dpm_suspend_start %d\n", __func__, err);
diff --git a/fs/super.c b/fs/super.c
index 954aeb8..b7cb50f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1373,3 +1373,87 @@ out:
 	return 0;
 }
 EXPORT_SYMBOL(thaw_super);
+
+static bool super_should_freeze(struct super_block *sb, bool skip_virtual)
+{
+	if (!sb->s_root)
+		return false;
+	if (!(sb->s_flags & MS_BORN))
+		return false;
+	/* Should we freeze virtual filesystems? */
+	if (sb->s_bdi == &noop_backing_dev_info && skip_virtual)
+		return false;
+	/* No need to freeze read-only filesystems */
+	if (sb->s_flags & MS_RDONLY)
+		return false;
+	return true;
+}
+
+/**
+ * freeze_all_supers -- iterate through all filesystems and freeze them
+ * @skip_virtual: should those with no backing device be skipped?
+ *
+ * Iterate over all superblocks and call freeze_super() for them. Some
+ * use-cases (such as freezer) might want to have to skip those which
+ * don't have any backing bdev.
+ *
+ */
+int freeze_all_supers(bool skip_virtual)
+{
+	struct super_block *sb, *p = NULL;
+	int error = 0;
+
+	spin_lock(&sb_lock);
+	/*
+	 * The list of super-blocks is iterated in a reverse order so that
+	 * inter-dependencies (such as loopback devices) are handled in
+	 * a non-deadlocking order
+	 */
+	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
+		if (hlist_unhashed(&sb->s_instances))
+			continue;
+		sb->s_count++;
+
+		spin_unlock(&sb_lock);
+		if (super_should_freeze(sb, skip_virtual)) {
+			error = freeze_super(sb);
+			if (error) {
+				spin_lock(&sb_lock);
+				break;
+			}
+		}
+		spin_lock(&sb_lock);
+
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+
+	return error;
+}
+
+void thaw_all_supers(void)
+{
+	struct super_block *sb, *p = NULL;
+
+	spin_lock(&sb_lock);
+	list_for_each_entry(sb, &super_blocks, s_list) {
+		if (hlist_unhashed(&sb->s_instances))
+			continue;
+		sb->s_count++;
+
+		spin_unlock(&sb_lock);
+		thaw_super(sb);
+		spin_lock(&sb_lock);
+
+		if (p)
+			__put_super(p);
+		p = sb;
+	}
+	if (p)
+		__put_super(p);
+	spin_unlock(&sb_lock);
+}
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 6b7fd9c..81335f6 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -43,9 +43,7 @@ extern void __thaw_task(struct task_struct *t);
 
 extern bool __refrigerator(bool check_kthr_stop);
 extern int freeze_processes(void);
-extern int freeze_kernel_threads(void);
 extern void thaw_processes(void);
-extern void thaw_kernel_threads(void);
 
 /*
  * DO NOT ADD ANY NEW CALLERS OF THIS FUNCTION
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 72d8a84..8ef2605 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2029,6 +2029,8 @@ extern int fd_statfs(int, struct kstatfs *);
 extern int vfs_ustat(dev_t, struct kstatfs *);
 extern int freeze_super(struct super_block *super);
 extern int thaw_super(struct super_block *super);
+extern int freeze_all_supers(bool);
+extern void thaw_all_supers(void);
 extern bool our_mnt(struct vfsmount *mnt);
 
 extern int current_umask(void);
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 690f78f..d5c36bb 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -348,10 +348,6 @@ int hibernation_snapshot(int platform_mode)
 	if (error)
 		goto Close;
 
-	error = freeze_kernel_threads();
-	if (error)
-		goto Cleanup;
-
 	if (hibernation_test(TEST_FREEZER)) {
 
 		/*
@@ -402,7 +398,6 @@ int hibernation_snapshot(int platform_mode)
 	return error;
 
  Thaw:
-	thaw_kernel_threads();
  Cleanup:
 	swsusp_free();
 	goto Close;
diff --git a/kernel/power/power.h b/kernel/power/power.h
index caadb56..4c3267f 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -224,25 +224,7 @@ extern int pm_test_level;
 #ifdef CONFIG_SUSPEND_FREEZER
 static inline int suspend_freeze_processes(void)
 {
-	int error;
-
-	error = freeze_processes();
-	/*
-	 * freeze_processes() automatically thaws every task if freezing
-	 * fails. So we need not do anything extra upon error.
-	 */
-	if (error)
-		return error;
-
-	error = freeze_kernel_threads();
-	/*
-	 * freeze_kernel_threads() thaws only kernel threads upon freezing
-	 * failure. So we have to thaw the userspace tasks ourselves.
-	 */
-	if (error)
-		thaw_processes();
-
-	return error;
+	return freeze_processes();
 }
 
 static inline void suspend_thaw_processes(void)
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 564f786..b1ad215 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -17,6 +17,7 @@
 #include <linux/delay.h>
 #include <linux/workqueue.h>
 #include <linux/kmod.h>
+#include <linux/fs.h>
 #include <trace/events/power.h>
 
 /* 
@@ -140,6 +141,17 @@ int freeze_processes(void)
 	pr_cont("\n");
 	BUG_ON(in_atomic());
 
+	pr_info("Freezing filesystems ... ");
+
+	error = freeze_all_supers(true);
+	if (error) {
+		pr_cont("failed.\n");
+		thaw_processes();
+		return error;
+	}
+
+	pr_cont("done.\n");
+
 	/*
 	 * Now that the whole userspace is frozen we need to disbale
 	 * the OOM killer to disallow any further interference with
@@ -148,35 +160,10 @@ int freeze_processes(void)
 	if (!error && !oom_killer_disable())
 		error = -EBUSY;
 
-	if (error)
+	if (error) {
 		thaw_processes();
-	return error;
-}
-
-/**
- * freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
- *
- * On success, returns 0.  On failure, -errno and only the kernel threads are
- * thawed, so as to give a chance to the caller to do additional cleanups
- * (if any) before thawing the userspace tasks. So, it is the responsibility
- * of the caller to thaw the userspace tasks, when the time is right.
- */
-int freeze_kernel_threads(void)
-{
-	int error;
-
-	pr_info("Freezing remaining freezable tasks ... ");
-
-	pm_nosig_freezing = true;
-	error = try_to_freeze_tasks(false);
-	if (!error)
-		pr_cont("done.");
-
-	pr_cont("\n");
-	BUG_ON(in_atomic());
-
-	if (error)
-		thaw_kernel_threads();
+		thaw_all_supers();
+	}
 	return error;
 }
 
@@ -197,6 +184,7 @@ void thaw_processes(void)
 
 	__usermodehelper_set_disable_depth(UMH_FREEZING);
 	thaw_workqueues();
+	thaw_all_supers();
 
 	read_lock(&tasklist_lock);
 	for_each_process_thread(g, p) {
@@ -216,22 +204,3 @@ void thaw_processes(void)
 	trace_suspend_resume(TPS("thaw_processes"), 0, false);
 }
 
-void thaw_kernel_threads(void)
-{
-	struct task_struct *g, *p;
-
-	pm_nosig_freezing = false;
-	pr_info("Restarting kernel threads ... ");
-
-	thaw_workqueues();
-
-	read_lock(&tasklist_lock);
-	for_each_process_thread(g, p) {
-		if (p->flags & (PF_KTHREAD | PF_WQ_WORKER))
-			__thaw_task(p);
-	}
-	read_unlock(&tasklist_lock);
-
-	schedule();
-	pr_cont("done.\n");
-}
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 526e891..75771c0 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -275,15 +275,6 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 		swsusp_free();
 		memset(&data->handle, 0, sizeof(struct snapshot_handle));
 		data->ready = false;
-		/*
-		 * It is necessary to thaw kernel threads here, because
-		 * SNAPSHOT_CREATE_IMAGE may be invoked directly after
-		 * SNAPSHOT_FREE.  In that case, if kernel threads were not
-		 * thawed, the preallocation of memory carried out by
-		 * hibernation_snapshot() might run into problems (i.e. it
-		 * might fail or even deadlock).
-		 */
-		thaw_kernel_threads();
 		break;
 
 	case SNAPSHOT_PREF_IMAGE_SIZE:
-- 
1.9.2

Re: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[kbuild test robot]

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

Hi Jiri,

[auto build test WARNING on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/PM-vfs-use-filesystem-freezing-instead-of-kthread-freezer/20151030-215223
config: x86_64-randconfig-x014-10300134 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   kernel/power/hibernate.c: In function 'hibernation_snapshot':
>> kernel/power/hibernate.c:401:2: warning: label 'Cleanup' defined but not used [-Wunused-label]
     Cleanup:
     ^

vim +/Cleanup +401 kernel/power/hibernate.c

64a473cb7 kernel/power/hibernate.c Rafael J. Wysocki 2009-07-08  385  		swsusp_free();
64a473cb7 kernel/power/hibernate.c Rafael J. Wysocki 2009-07-08  386  
91e7c75ba kernel/power/hibernate.c Rafael J. Wysocki 2011-05-17  387  	msg = in_suspend ? (error ? PMSG_RECOVER : PMSG_THAW) : PMSG_RESTORE;
91e7c75ba kernel/power/hibernate.c Rafael J. Wysocki 2011-05-17  388  	dpm_resume(msg);
c9e664f1f kernel/power/hibernate.c Rafael J. Wysocki 2010-12-03  389  
c9e664f1f kernel/power/hibernate.c Rafael J. Wysocki 2010-12-03  390  	if (error || !in_suspend)
c9e664f1f kernel/power/hibernate.c Rafael J. Wysocki 2010-12-03  391  		pm_restore_gfp_mask();
c9e664f1f kernel/power/hibernate.c Rafael J. Wysocki 2010-12-03  392  
7777fab98 kernel/power/disk.c      Rafael J. Wysocki 2007-07-19  393  	resume_console();
91e7c75ba kernel/power/hibernate.c Rafael J. Wysocki 2011-05-17  394  	dpm_complete(msg);
91e7c75ba kernel/power/hibernate.c Rafael J. Wysocki 2011-05-17  395  
caea99ef3 kernel/power/disk.c      Rafael J. Wysocki 2008-01-08  396   Close:
caea99ef3 kernel/power/disk.c      Rafael J. Wysocki 2008-01-08  397  	platform_end(platform_mode);
7777fab98 kernel/power/disk.c      Rafael J. Wysocki 2007-07-19  398  	return error;
d8f3de0d2 kernel/power/disk.c      Rafael J. Wysocki 2008-06-12  399  
51d6ff7ac kernel/power/hibernate.c Srivatsa S. Bhat  2012-02-04  400   Thaw:
bb58dd5d1 kernel/power/hibernate.c Rafael J. Wysocki 2011-11-22 @401   Cleanup:
bb58dd5d1 kernel/power/hibernate.c Rafael J. Wysocki 2011-11-22  402  	swsusp_free();
bb58dd5d1 kernel/power/hibernate.c Rafael J. Wysocki 2011-11-22  403  	goto Close;
7777fab98 kernel/power/disk.c      Rafael J. Wysocki 2007-07-19  404  }
7777fab98 kernel/power/disk.c      Rafael J. Wysocki 2007-07-19  405  
7777fab98 kernel/power/disk.c      Rafael J. Wysocki 2007-07-19  406  /**
f42a9813f kernel/power/hibernate.c Rafael J. Wysocki 2011-05-24  407   * resume_target_kernel - Restore system state from a hibernation image.
f42a9813f kernel/power/hibernate.c Rafael J. Wysocki 2011-05-24  408   * @platform_mode: Whether or not to use the platform driver.
f42a9813f kernel/power/hibernate.c Rafael J. Wysocki 2011-05-24  409   *

:::::: The code at line 401 was first introduced by commit
:::::: bb58dd5d1ffad6c2d21c69698ba766dad4ae54e6 PM / Hibernate: Do not leak memory in error/test code paths

:::::: TO: Rafael J. Wysocki <rjw@sisk.pl>
:::::: CC: Rafael J. Wysocki <rjw@sisk.pl>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22003 bytes --]

Re: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[Oliver Neukum]

On Fri, 2015-10-30 at 14:47 +0100, Jiri Kosina wrote:
> Basically the main argument why kthread freezer is not needed boils
> down to
> this: the only facility that is needed during suspend: "no persistent
> fs
> changes are allowed from now on".

Is that true? Drivers of character devices also may assume
that IO and suspend() wouldn't race (except in fairly clear exceptions)

	Regards
		Oliver

Re: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[Neil Brown]

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

On Sat, Oct 31 2015, Oliver Neukum wrote:

> On Fri, 2015-10-30 at 14:47 +0100, Jiri Kosina wrote:
>> Basically the main argument why kthread freezer is not needed boils
>> down to
>> this: the only facility that is needed during suspend: "no persistent
>> fs
>> changes are allowed from now on".
>
> Is that true? Drivers of character devices also may assume
> that IO and suspend() wouldn't race (except in fairly clear exceptions)

Anything that is a device can hook in to the suspend using the standard
call back and make sure no problematic races happen.

Filesystems are different partly because the "know" in memory some
details of what is persisting on storage, and partly because they aren't
devices.

Maybe filesystems should be devices .... though that probably wouldn't
help much.


I would suggest that "the only facility that is needed during suspend"
is really "well defined states and well defined transitions".
Filesystems need them as well as devices, and frozen kthreads might have
been a kludge to try to provide them.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[yalin wang]

> On Oct 30, 2015, at 21:47, Jiri Kosina <jikos@kernel.org> wrote:
> 
> From: Jiri Kosina <jkosina@suse.cz>
> 
> Freeze all filesystems during hibernation in favor of dropping kthread
> freezing completely.
> 
> Kthread freezing has a history of not very well defined semantics.
> Historically, it has been established to make sure that kthreads which are
> helpers to writing out data to disk are frozen during hibernation at
> well-defined state, so that it's guaranteed that they freeze only after all the
> outstanding I/O made it to disk (userspace has already been frozen by that
> time, so there is no new I/O being issued).
> 
> One of the issues with kthread freezer is that the places where try_to_freeze()
> is called in kthreads is rather random / arbitrary. This stems mostly from
> the fact that there is actually no good definition / list of requirements
> that should be enforced on a frozen kthread (it's important to mention that
> frozen kthread for example doesn't automatically imply that everything that
> has been spawned asynchronously from it (such as timers) is stopped as well).
> 
> Basically the main argument why kthread freezer is not needed boils down to
> this: the only facility that is needed during suspend: "no persistent fs
> changes are allowed from now on".
> 
> 	- this is something we get from fs freezing for free
> 	- Why do we issue all those try_to_freeze() calls in kthreads that
> 	  don't generate any I/O themselves at all?
> 	- Why do we issue all those try_to_freeze() calls in kthreads that
> 	  are actual I/O helpers? (if there is outstanding I/O, we *want*
> 	  it to happen before hibernating).
> 
> This patch removes freezing of kthreads during suspend, and issues a global
> filesystem freeze instead.
> 
> We awoid freezing in-memory filesystems, because (a) they end up in the
> image in a consistent state anyway (b) we will deadlock, as write to
> /sys/power/state would never succeed.
> 
> The patch could have been made a bit nicer if generic iterate_supers()
> could be used, but the locking (especially s_umount semantics) would have to
> be redone, so that'd be something to postpone for later.
> Also, the 'skip_virtual' flag is superfluous for now (as hibernation is the
> only user of this facility for the time being), but I'd like to keep it
> there to avoid further confusion regarding the fact that freeze_all_supers()
> actually skips in-memory filesystems.
> 
> Based on prior work done by Rafael Wysocki.
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/xen/manage.c     |  6 ----
> fs/super.c               | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/freezer.h  |  2 --
> include/linux/fs.h       |  2 ++
> kernel/power/hibernate.c |  5 ---
> kernel/power/power.h     | 20 +-----------
> kernel/power/process.c   | 63 +++++++++---------------------------
> kernel/power/user.c      |  9 ------
> 8 files changed, 103 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index e12bd36..d47716a 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -109,12 +109,6 @@ static void do_suspend(void)
> 		goto out;
> 	}
> 
> -	err = freeze_kernel_threads();
> -	if (err) {
> -		pr_err("%s: freeze kernel threads failed %d\n", __func__, err);
> -		goto out_thaw;
> -	}
> -
> 	err = dpm_suspend_start(PMSG_FREEZE);
> 	if (err) {
> 		pr_err("%s: dpm_suspend_start %d\n", __func__, err);
> diff --git a/fs/super.c b/fs/super.c
> index 954aeb8..b7cb50f 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1373,3 +1373,87 @@ out:
> 	return 0;
> }
> EXPORT_SYMBOL(thaw_super);
> +
> +static bool super_should_freeze(struct super_block *sb, bool skip_virtual)
> +{
> +	if (!sb->s_root)
> +		return false;
> +	if (!(sb->s_flags & MS_BORN))
> +		return false;
> +	/* Should we freeze virtual filesystems? */
> +	if (sb->s_bdi == &noop_backing_dev_info && skip_virtual)
> +		return false;
> +	/* No need to freeze read-only filesystems */
> +	if (sb->s_flags & MS_RDONLY)
> +		return false;
> +	return true;
> +}
> +
> +/**
> + * freeze_all_supers -- iterate through all filesystems and freeze them
> + * @skip_virtual: should those with no backing device be skipped?
> + *
> + * Iterate over all superblocks and call freeze_super() for them. Some
> + * use-cases (such as freezer) might want to have to skip those which
> + * don't have any backing bdev.
> + *
> + */
> +int freeze_all_supers(bool skip_virtual)
> +{
> +	struct super_block *sb, *p = NULL;
> +	int error = 0;
> +
> +	spin_lock(&sb_lock);
> +	/*
> +	 * The list of super-blocks is iterated in a reverse order so that
> +	 * inter-dependencies (such as loopback devices) are handled in
> +	 * a non-deadlocking order
> +	 */
> +	list_for_each_entry_reverse(sb, &super_blocks, s_list) {
> +		if (hlist_unhashed(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +
> +		spin_unlock(&sb_lock);
> +		if (super_should_freeze(sb, skip_virtual)) {
> +			error = freeze_super(sb);
> +			if (error) {
> +				spin_lock(&sb_lock);
> +				break;
> +			}
> +		}
> +		spin_lock(&sb_lock);
> +
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +
> +	return error;
> +}
> +
> +void thaw_all_supers(void)
> +{
> +	struct super_block *sb, *p = NULL;
> +
> +	spin_lock(&sb_lock);
> +	list_for_each_entry(sb, &super_blocks, s_list) {
> +		if (hlist_unhashed(&sb->s_instances))
> +			continue;
> +		sb->s_count++;
> +
> +		spin_unlock(&sb_lock);
> +		thaw_super(sb);
> +		spin_lock(&sb_lock);
> +
> +		if (p)
> +			__put_super(p);
> +		p = sb;
> +	}
> +	if (p)
> +		__put_super(p);
> +	spin_unlock(&sb_lock);
> +}
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 6b7fd9c..81335f6 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -43,9 +43,7 @@ extern void __thaw_task(struct task_struct *t);
> 
> extern bool __refrigerator(bool check_kthr_stop);
> extern int freeze_processes(void);
> -extern int freeze_kernel_threads(void);
> extern void thaw_processes(void);
> -extern void thaw_kernel_threads(void);
> 
> /*
>  * DO NOT ADD ANY NEW CALLERS OF THIS FUNCTION
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 72d8a84..8ef2605 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2029,6 +2029,8 @@ extern int fd_statfs(int, struct kstatfs *);
> extern int vfs_ustat(dev_t, struct kstatfs *);
> extern int freeze_super(struct super_block *super);
> extern int thaw_super(struct super_block *super);
> +extern int freeze_all_supers(bool);
> +extern void thaw_all_supers(void);
> extern bool our_mnt(struct vfsmount *mnt);
> 
> extern int current_umask(void);
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 690f78f..d5c36bb 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -348,10 +348,6 @@ int hibernation_snapshot(int platform_mode)
> 	if (error)
> 		goto Close;
> 
> -	error = freeze_kernel_threads();
> -	if (error)
> -		goto Cleanup;
> -
> 	if (hibernation_test(TEST_FREEZER)) {
> 
> 		/*
> @@ -402,7 +398,6 @@ int hibernation_snapshot(int platform_mode)
> 	return error;
> 
>  Thaw:
> -	thaw_kernel_threads();
>  Cleanup:
> 	swsusp_free();
> 	goto Close;
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index caadb56..4c3267f 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -224,25 +224,7 @@ extern int pm_test_level;
> #ifdef CONFIG_SUSPEND_FREEZER
> static inline int suspend_freeze_processes(void)
> {
> -	int error;
> -
> -	error = freeze_processes();
> -	/*
> -	 * freeze_processes() automatically thaws every task if freezing
> -	 * fails. So we need not do anything extra upon error.
> -	 */
> -	if (error)
> -		return error;
> -
> -	error = freeze_kernel_threads();
> -	/*
> -	 * freeze_kernel_threads() thaws only kernel threads upon freezing
> -	 * failure. So we have to thaw the userspace tasks ourselves.
> -	 */
> -	if (error)
> -		thaw_processes();
> -
> -	return error;
> +	return freeze_processes();
> }
> 
> static inline void suspend_thaw_processes(void)
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 564f786..b1ad215 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -17,6 +17,7 @@
> #include <linux/delay.h>
> #include <linux/workqueue.h>
> #include <linux/kmod.h>
> +#include <linux/fs.h>
> #include <trace/events/power.h>
> 
> /* 
> @@ -140,6 +141,17 @@ int freeze_processes(void)
> 	pr_cont("\n");
> 	BUG_ON(in_atomic());
> 
> +	pr_info("Freezing filesystems ... ");
> +
> +	error = freeze_all_supers(true);
> +	if (error) {
> +		pr_cont("failed.\n");
> +		thaw_processes();
> +		return error;
> +	}
> +
> +	pr_cont("done.\n");
> +
> 	/*
> 	 * Now that the whole userspace is frozen we need to disbale
> 	 * the OOM killer to disallow any further interference with
> @@ -148,35 +160,10 @@ int freeze_processes(void)
> 	if (!error && !oom_killer_disable())
> 		error = -EBUSY;
> 
> -	if (error)
> +	if (error) {
> 		thaw_processes();
> -	return error;
> -}
> -
> -/**
> - * freeze_kernel_threads - Make freezable kernel threads go to the refrigerator.
> - *
> - * On success, returns 0.  On failure, -errno and only the kernel threads are
> - * thawed, so as to give a chance to the caller to do additional cleanups
> - * (if any) before thawing the userspace tasks. So, it is the responsibility
> - * of the caller to thaw the userspace tasks, when the time is right.
> - */
> -int freeze_kernel_threads(void)
> -{
> -	int error;
> -
> -	pr_info("Freezing remaining freezable tasks ... ");
> -
> -	pm_nosig_freezing = true;
> -	error = try_to_freeze_tasks(false);
> -	if (!error)
> -		pr_cont("done.");
> -
> -	pr_cont("\n");
> -	BUG_ON(in_atomic());
> -
> -	if (error)
> -		thaw_kernel_threads();
> +		thaw_all_supers();
> +	}
> 	return error;
> }
> 
> @@ -197,6 +184,7 @@ void thaw_processes(void)
> 
> 	__usermodehelper_set_disable_depth(UMH_FREEZING);
> 	thaw_workqueues();
> +	thaw_all_supers();
> 
> 	read_lock(&tasklist_lock);
> 	for_each_process_thread(g, p) {
> @@ -216,22 +204,3 @@ void thaw_processes(void)
> 	trace_suspend_resume(TPS("thaw_processes"), 0, false);
> }
> 
> -void thaw_kernel_threads(void)
> -{
> -	struct task_struct *g, *p;
> -
> -	pm_nosig_freezing = false;
> -	pr_info("Restarting kernel threads ... ");
> -
> -	thaw_workqueues();
> -
> -	read_lock(&tasklist_lock);
> -	for_each_process_thread(g, p) {
> -		if (p->flags & (PF_KTHREAD | PF_WQ_WORKER))
> -			__thaw_task(p);
> -	}
> -	read_unlock(&tasklist_lock);
> -
> -	schedule();
> -	pr_cont("done.\n");
> -}
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index 526e891..75771c0 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -275,15 +275,6 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> 		swsusp_free();
> 		memset(&data->handle, 0, sizeof(struct snapshot_handle));
> 		data->ready = false;
> -		/*
> -		 * It is necessary to thaw kernel threads here, because
> -		 * SNAPSHOT_CREATE_IMAGE may be invoked directly after
> -		 * SNAPSHOT_FREE.  In that case, if kernel threads were not
> -		 * thawed, the preallocation of memory carried out by
> -		 * hibernation_snapshot() might run into problems (i.e. it
> -		 * might fail or even deadlock).
> -		 */
> -		thaw_kernel_threads();
> 		break;
> 
> 	case SNAPSHOT_PREF_IMAGE_SIZE:
> -- 
> 1.9.2
do you test your patch on kthread_bind()  kernel thread ?
if you remove freeze_kernel_threads() function,
this means lots of pthread will be running status during suspend .
will have problem for bind thread , these thread will be migrate to other 
CPU , will have problem running code on other CPU, another problems is 
that the cpu_allowed_ptr is changed and will never be restore to original CPU mask .
this is not unsafe .
for example, if there is a thread like this :

kthread_bind()
while (!pthread_should_stop())
{
	do_something();

	try_to_freeze();
}

if remove try_to_freeze() , this thread maybe migrate to other CPU if the thread is running .
freeze kernel thread can make sure lots of kernel thread are not in runq during suspend ,
then we can avoid task migration.

Thanks

Re: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing

[Jiri Kosina]

On Mon, 2 Nov 2015, yalin wang wrote:

> do you test your patch on kthread_bind()  kernel thread ?
> if you remove freeze_kernel_threads() function,
> this means lots of pthread will be running status during suspend .

pthreads? Again, userspace is frozen by that time.

> will have problem for bind thread , these thread will be migrate to other 
> CPU , will have problem running code on other CPU, another problems is 
> that the cpu_allowed_ptr is changed and will never be restore to original CPU mask .

Hmm, shouldn't that be fixed instead?

I mean, select_fallback_rq() will surely force a rq outside of the 
cpus_allowed if needed. I can imagine kthread bound to a particular CPU 
either wanting to keep itself affine to that CPU (and be scheduled out 
until CPU becomes online again), or it might want to actually be forced to 
another CPU and migrated back once "its own" CPU becomes available again.

But then yes, indeed, at the end of the day this is more or less what 
try_to_freeze() gives you. Fair enough, this might be one of cases where 
freezer for kthreads is actually useful (and would need to be documented 
to provide this semantics).

kthread CPU binding seems to be used by ~7 kthreads altogether in the 
kernel. Funily enough, quite some of them do not call try_to_freeze() at 
all. Which just demonstrates my point regarding how messy the current 
state of affairs is.

-- 
Jiri Kosina
SUSE Labs

[PATCH 2/3] freezer: get rid of the kthread freezer

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Now that hibernation performs fs freezing, we can get rid of kthread freezing
altogether.

This patch can even be used to demonstrate why the current kthread freezing is
an absolutely horrible API -- there are many kthreads that have gotten it wrong
since ever; they are often either not freezable, but issue the try_to_freeze()
call anyway or they set itself freezable, but never ever attempt to freeze
themselves.

Plus, it's being misused completely by all those kthreads which don't
participate in filesystem I/O at all.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 arch/powerpc/platforms/83xx/suspend.c            |  1 -
 arch/powerpc/platforms/powernv/opal.c            |  2 -
 arch/powerpc/platforms/ps3/device-init.c         |  1 -
 drivers/acpi/acpi_pad.c                          |  2 -
 drivers/android/binder.c                         |  4 +-
 drivers/block/pktcdvd.c                          |  4 --
 drivers/block/xen-blkback/blkback.c              |  2 -
 drivers/dma/dmatest.c                            |  6 +--
 drivers/macintosh/therm_adt746x.c                |  2 -
 drivers/macintosh/windfarm_core.c                |  2 -
 drivers/md/bcache/alloc.c                        |  1 -
 drivers/md/bcache/btree.c                        |  1 -
 drivers/md/bcache/writeback.c                    |  2 -
 drivers/md/dm-log-writes.c                       | 12 +++---
 drivers/media/dvb-core/dvb_frontend.c            |  4 --
 drivers/media/i2c/msp3400-driver.c               |  1 -
 drivers/media/i2c/msp3400-kthreads.c             |  3 --
 drivers/media/i2c/tvaudio.c                      |  2 -
 drivers/media/pci/cx88/cx88-tvaudio.c            |  2 -
 drivers/media/pci/pt1/pt1.c                      |  2 -
 drivers/media/pci/pt3/pt3.c                      |  1 -
 drivers/media/pci/saa7134/saa7134-tvaudio.c      |  8 ----
 drivers/media/pci/saa7164/saa7164-core.c         |  3 --
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c   |  2 -
 drivers/media/pci/solo6x10/solo6x10-v4l2.c       |  2 -
 drivers/media/platform/vivid/vivid-kthread-cap.c |  3 --
 drivers/media/platform/vivid/vivid-kthread-out.c |  3 --
 drivers/media/platform/vivid/vivid-sdr-cap.c     |  3 --
 drivers/media/usb/pvrusb2/pvrusb2-dvb.c          |  4 --
 drivers/media/v4l2-core/videobuf-dvb.c           |  2 -
 drivers/media/v4l2-core/videobuf2-core.c         |  3 --
 drivers/mfd/ucb1x00-ts.c                         |  1 -
 drivers/mtd/ubi/wl.c                             |  4 --
 drivers/net/irda/stir4200.c                      |  2 -
 drivers/net/wireless/airo.c                      |  2 -
 drivers/pcmcia/cs.c                              |  2 -
 drivers/platform/x86/thinkpad_acpi.c             |  2 -
 drivers/pnp/pnpbios/core.c                       |  4 --
 drivers/power/tps65090-charger.c                 |  3 --
 drivers/staging/android/ion/ion_heap.c           |  2 +-
 drivers/thermal/intel_powerclamp.c               |  2 -
 drivers/tty/hvc/hvc_console.c                    |  2 -
 drivers/usb/atm/ueagle-atm.c                     |  2 -
 drivers/usb/gadget/function/f_mass_storage.c     | 25 +++++-------
 drivers/uwb/uwbd.c                               |  1 -
 drivers/video/fbdev/ps3fb.c                      |  2 -
 drivers/video/fbdev/pxafb.c                      |  4 --
 drivers/virtio/virtio_balloon.c                  |  3 --
 drivers/w1/w1.c                                  |  1 -
 fs/btrfs/disk-io.c                               | 16 ++++----
 fs/cifs/connect.c                                |  5 ---
 fs/ecryptfs/kthread.c                            |  3 +-
 fs/ext4/super.c                                  |  2 -
 fs/f2fs/gc.c                                     |  9 ++---
 fs/gfs2/log.c                                    |  2 -
 fs/gfs2/quota.c                                  |  2 -
 fs/jbd2/journal.c                                |  3 --
 fs/jffs2/background.c                            |  4 --
 fs/jfs/jfs_logmgr.c                              |  1 -
 fs/jfs/jfs_txnmgr.c                              | 28 +++++--------
 fs/lockd/svc.c                                   |  3 --
 fs/nfs/callback.c                                |  6 ---
 fs/nfsd/nfssvc.c                                 |  2 -
 fs/nilfs2/segment.c                              | 50 +++++++++++-------------
 fs/ubifs/commit.c                                |  4 --
 fs/xfs/xfs_trans_ail.c                           |  2 -
 kernel/audit.c                                   |  3 +-
 kernel/kthread.c                                 |  1 -
 mm/huge_memory.c                                 |  7 ++--
 mm/ksm.c                                         |  5 +--
 mm/vmscan.c                                      | 14 +++----
 net/core/pktgen.c                                |  5 ---
 net/sunrpc/svc_xprt.c                            |  1 -
 73 files changed, 72 insertions(+), 260 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index fcbea4b..4ca1e06f 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -266,7 +266,6 @@ static int agent_thread_fn(void *data)
 {
 	while (1) {
 		wait_event_interruptible(agent_wq, pci_pm_state >= 2);
-		try_to_freeze();
 
 		if (signal_pending(current) || pci_pm_state < 2)
 			continue;
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 4296d55..ea0fd8b 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -662,9 +662,7 @@ static int kopald(void *unused)
 {
 	__be64 events;
 
-	set_freezable();
 	do {
-		try_to_freeze();
 		opal_poll_events(&events);
 		opal_handle_events(be64_to_cpu(events));
 		msleep_interruptible(opal_heartbeat);
diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c
index 3f175e8..059b013 100644
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -843,7 +843,6 @@ static int ps3_probe_thread(void *data)
 
 	/* Loop here processing the requested notification events. */
 	do {
-		try_to_freeze();
 
 		memset(notify_event, 0, sizeof(*notify_event));
 
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index ae307ff..8ea8211 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -148,8 +148,6 @@ static int power_saving_thread(void *data)
 	while (!kthread_should_stop()) {
 		unsigned long expire_time;
 
-		try_to_freeze();
-
 		/* round robin to cpus */
 		expire_time = last_jiffies + round_robin_time * HZ;
 		if (time_before(expire_time, jiffies)) {
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index a39e85f..7645f1d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2202,13 +2202,13 @@ retry:
 			if (!binder_has_proc_work(proc, thread))
 				ret = -EAGAIN;
 		} else
-			ret = wait_event_freezable_exclusive(proc->wait, binder_has_proc_work(proc, thread));
+			ret = wait_event_interruptible_exclusive(proc->wait, binder_has_proc_work(proc, thread));
 	} else {
 		if (non_block) {
 			if (!binder_has_thread_work(thread))
 				ret = -EAGAIN;
 		} else
-			ret = wait_event_freezable(thread->wait, binder_has_thread_work(thread));
+			ret = wait_event_interruptible(thread->wait, binder_has_thread_work(thread));
 	}
 
 	binder_lock(__func__);
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 7be2375..e057373 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1466,7 +1466,6 @@ static int kcdrwd(void *foobar)
 	long min_sleep_time, residue;
 
 	set_user_nice(current, MIN_NICE);
-	set_freezable();
 
 	for (;;) {
 		DECLARE_WAITQUEUE(wait, current);
@@ -1511,9 +1510,6 @@ static int kcdrwd(void *foobar)
 			residue = schedule_timeout(min_sleep_time);
 			pkt_dbg(2, pd, "wake up\n");
 
-			/* make swsusp happy with our thread */
-			try_to_freeze();
-
 			list_for_each_entry(pkt, &pd->cdrw.pkt_active_list, list) {
 				if (!pkt->sleep_time)
 					continue;
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 6a685ae..af3caa3 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -598,8 +598,6 @@ int xen_blkif_schedule(void *arg)
 	xen_blkif_get(blkif);
 
 	while (!kthread_should_stop()) {
-		if (try_to_freeze())
-			continue;
 		if (unlikely(vbd->size != vbd_sz(vbd)))
 			xen_vbd_resize(blkif);
 
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index b8576fd..fa569fb 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -316,7 +316,7 @@ static unsigned int dmatest_verify(u8 **bufs, unsigned int start,
 	return error_count;
 }
 
-/* poor man's completion - we want to use wait_event_freezable() on it */
+/* poor man's completion - we want to use wait_event_interruptible() on it */
 struct dmatest_done {
 	bool			done;
 	wait_queue_head_t	*wait;
@@ -420,8 +420,6 @@ static int dmatest_func(void *data)
 	s64			runtime = 0;
 	unsigned long long	total_len = 0;
 
-	set_freezable();
-
 	ret = -ENOMEM;
 
 	smp_rmb();
@@ -620,7 +618,7 @@ static int dmatest_func(void *data)
 		}
 		dma_async_issue_pending(chan);
 
-		wait_event_freezable_timeout(done_wait, done.done,
+		wait_event_interruptible_timeout(done_wait, done.done,
 					     msecs_to_jiffies(params->timeout));
 
 		status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
diff --git a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
index f433521..94704be 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -282,9 +282,7 @@ static int monitor_task(void *arg)
 {
 	struct thermostat* th = arg;
 
-	set_freezable();
 	while(!kthread_should_stop()) {
-		try_to_freeze();
 		msleep_interruptible(2000);
 
 #ifndef DEBUG
diff --git a/drivers/macintosh/windfarm_core.c b/drivers/macintosh/windfarm_core.c
index 465d770..5683d6c 100644
--- a/drivers/macintosh/windfarm_core.c
+++ b/drivers/macintosh/windfarm_core.c
@@ -93,9 +93,7 @@ static int wf_thread_func(void *data)
 
 	DBG("wf: thread started\n");
 
-	set_freezable();
 	while (!kthread_should_stop()) {
-		try_to_freeze();
 
 		if (time_after_eq(jiffies, next)) {
 			wf_notify(WF_EVENT_TICK, NULL);
diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
index 8eeab72..ffb7178 100644
--- a/drivers/md/bcache/alloc.c
+++ b/drivers/md/bcache/alloc.c
@@ -288,7 +288,6 @@ do {									\
 		if (kthread_should_stop())				\
 			return 0;					\
 									\
-		try_to_freeze();					\
 		schedule();						\
 		mutex_lock(&(ca)->set->bucket_lock);			\
 	}								\
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 83392f8..c906d51 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -1786,7 +1786,6 @@ again:
 
 		mutex_unlock(&c->bucket_lock);
 
-		try_to_freeze();
 		schedule();
 	}
 
diff --git a/drivers/md/bcache/writeback.c b/drivers/md/bcache/writeback.c
index b23f88d..28248cd 100644
--- a/drivers/md/bcache/writeback.c
+++ b/drivers/md/bcache/writeback.c
@@ -228,7 +228,6 @@ static void read_dirty(struct cached_dev *dc)
 	 */
 
 	while (!kthread_should_stop()) {
-		try_to_freeze();
 
 		w = bch_keybuf_next(&dc->writeback_keys);
 		if (!w)
@@ -410,7 +409,6 @@ static int bch_writeback_thread(void *arg)
 			if (kthread_should_stop())
 				return 0;
 
-			try_to_freeze();
 			schedule();
 			continue;
 		}
diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c
index b2912db..fc8f377 100644
--- a/drivers/md/dm-log-writes.c
+++ b/drivers/md/dm-log-writes.c
@@ -397,13 +397,11 @@ next:
 			continue;
 		}
 
-		if (!try_to_freeze()) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			if (!kthread_should_stop() &&
-			    !atomic_read(&lc->pending_blocks))
-				schedule();
-			__set_current_state(TASK_RUNNING);
-		}
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (!kthread_should_stop() &&
+		    !atomic_read(&lc->pending_blocks))
+			schedule();
+		__set_current_state(TASK_RUNNING);
 	}
 	return 0;
 }
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index c38ef1a..a2bd323 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -725,7 +725,6 @@ static int dvb_frontend_thread(void *data)
 
 	dvb_frontend_init(fe);
 
-	set_freezable();
 	while (1) {
 		up(&fepriv->sem);	    /* is locked when we enter the thread... */
 restart:
@@ -742,9 +741,6 @@ restart:
 			break;
 		}
 
-		if (try_to_freeze())
-			goto restart;
-
 		if (down_interruptible(&fepriv->sem))
 			break;
 
diff --git a/drivers/media/i2c/msp3400-driver.c b/drivers/media/i2c/msp3400-driver.c
index bdb9400..884a1a3 100644
--- a/drivers/media/i2c/msp3400-driver.c
+++ b/drivers/media/i2c/msp3400-driver.c
@@ -339,7 +339,6 @@ int msp_sleep(struct msp_state *state, int timeout)
 	}
 
 	remove_wait_queue(&state->wq, &wait);
-	try_to_freeze();
 	return state->restart;
 }
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c b/drivers/media/i2c/msp3400-kthreads.c
index f8b5171..febcb62 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -510,7 +510,6 @@ int msp3400c_thread(void *data)
 
 	v4l_dbg(1, msp_debug, client, "msp3400 daemon started\n");
 	state->detected_std = V4L2_STD_ALL;
-	set_freezable();
 	for (;;) {
 		v4l_dbg(2, msp_debug, client, "msp3400 thread: sleep\n");
 		msp_sleep(state, -1);
@@ -700,7 +699,6 @@ int msp3410d_thread(void *data)
 
 	v4l_dbg(1, msp_debug, client, "msp3410 daemon started\n");
 	state->detected_std = V4L2_STD_ALL;
-	set_freezable();
 	for (;;) {
 		v4l_dbg(2, msp_debug, client, "msp3410 thread: sleep\n");
 		msp_sleep(state, -1);
@@ -998,7 +996,6 @@ int msp34xxg_thread(void *data)
 
 	v4l_dbg(1, msp_debug, client, "msp34xxg daemon started\n");
 	state->detected_std = V4L2_STD_ALL;
-	set_freezable();
 	for (;;) {
 		v4l_dbg(2, msp_debug, client, "msp34xxg thread: sleep\n");
 		msp_sleep(state, -1);
diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index 2a8114a..70fc264 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -314,13 +314,11 @@ static int chip_thread(void *data)
 	int mode, selected;
 
 	v4l2_dbg(1, debug, sd, "thread started\n");
-	set_freezable();
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (!kthread_should_stop())
 			schedule();
 		set_current_state(TASK_RUNNING);
-		try_to_freeze();
 		if (kthread_should_stop())
 			break;
 		v4l2_dbg(1, debug, sd, "thread wakeup\n");
diff --git a/drivers/media/pci/cx88/cx88-tvaudio.c b/drivers/media/pci/cx88/cx88-tvaudio.c
index 6bbce6a..2a4d2b0 100644
--- a/drivers/media/pci/cx88/cx88-tvaudio.c
+++ b/drivers/media/pci/cx88/cx88-tvaudio.c
@@ -992,12 +992,10 @@ int cx88_audio_thread(void *data)
 	u32 mode = 0;
 
 	dprintk("cx88: tvaudio thread started\n");
-	set_freezable();
 	for (;;) {
 		msleep_interruptible(1000);
 		if (kthread_should_stop())
 			break;
-		try_to_freeze();
 
 		switch (core->tvaudio) {
 		case WW_BG:
diff --git a/drivers/media/pci/pt1/pt1.c b/drivers/media/pci/pt1/pt1.c
index e7e4428..27c9d86 100644
--- a/drivers/media/pci/pt1/pt1.c
+++ b/drivers/media/pci/pt1/pt1.c
@@ -317,10 +317,8 @@ static int pt1_thread(void *data)
 	struct pt1_buffer_page *page;
 
 	pt1 = data;
-	set_freezable();
 
 	while (!kthread_should_stop()) {
-		try_to_freeze();
 
 		page = pt1->tables[pt1->table_index].bufs[pt1->buf_index].page;
 		if (!pt1_filter(pt1, page)) {
diff --git a/drivers/media/pci/pt3/pt3.c b/drivers/media/pci/pt3/pt3.c
index 0d2e2b2..cd67325 100644
--- a/drivers/media/pci/pt3/pt3.c
+++ b/drivers/media/pci/pt3/pt3.c
@@ -455,7 +455,6 @@ static int pt3_fetch_thread(void *data)
 
 	dev_dbg(adap->dvb_adap.device, "PT3: [%s] started\n",
 		adap->thread->comm);
-	set_freezable();
 	while (!kthread_freezable_should_stop(&was_frozen)) {
 		if (was_frozen)
 			adap->num_discard = PT3_INITIAL_BUF_DROPS;
diff --git a/drivers/media/pci/saa7134/saa7134-tvaudio.c b/drivers/media/pci/saa7134/saa7134-tvaudio.c
index 21a5793..6316b85 100644
--- a/drivers/media/pci/saa7134/saa7134-tvaudio.c
+++ b/drivers/media/pci/saa7134/saa7134-tvaudio.c
@@ -477,15 +477,12 @@ static int tvaudio_thread(void *data)
 	unsigned int i, audio, nscan;
 	int max1,max2,carrier,rx,mode,lastmode,default_carrier;
 
-	set_freezable();
-
 	for (;;) {
 		tvaudio_sleep(dev,-1);
 		if (kthread_should_stop())
 			goto done;
 
 	restart:
-		try_to_freeze();
 
 		dev->thread.scan1 = dev->thread.scan2;
 		audio_dbg(1, "tvaudio thread scan start [%d]\n",
@@ -596,8 +593,6 @@ static int tvaudio_thread(void *data)
 		lastmode = 42;
 		for (;;) {
 
-			try_to_freeze();
-
 			if (tvaudio_sleep(dev,5000))
 				goto restart;
 			if (kthread_should_stop())
@@ -774,14 +769,11 @@ static int tvaudio_thread_ddep(void *data)
 	struct saa7134_dev *dev = data;
 	u32 value, norms;
 
-	set_freezable();
 	for (;;) {
 		tvaudio_sleep(dev,-1);
 		if (kthread_should_stop())
 			goto done;
 	restart:
-		try_to_freeze();
-
 		dev->thread.scan1 = dev->thread.scan2;
 		audio_dbg(1, "tvaudio thread scan start [%d]\n",
 			  dev->thread.scan1);
diff --git a/drivers/media/pci/saa7164/saa7164-core.c b/drivers/media/pci/saa7164/saa7164-core.c
index 3206a82..7f1f493 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -1164,13 +1164,10 @@ static int saa7164_thread_function(void *data)
 
 	dprintk(DBGLVL_THR, "thread started\n");
 
-	set_freezable();
-
 	while (1) {
 		msleep_interruptible(100);
 		if (kthread_should_stop())
 			break;
-		try_to_freeze();
 
 		dprintk(DBGLVL_THR, "thread running\n");
 
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 53fff54..d229944 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -641,7 +641,6 @@ static int solo_ring_thread(void *data)
 	struct solo_dev *solo_dev = data;
 	DECLARE_WAITQUEUE(wait, current);
 
-	set_freezable();
 	add_wait_queue(&solo_dev->ring_thread_wait, &wait);
 
 	for (;;) {
@@ -650,7 +649,6 @@ static int solo_ring_thread(void *data)
 		if (timeout == -ERESTARTSYS || kthread_should_stop())
 			break;
 		solo_handle_ring(solo_dev);
-		try_to_freeze();
 	}
 
 	remove_wait_queue(&solo_dev->ring_thread_wait, &wait);
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2.c b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
index 63ae8a6..8e67881 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2.c
@@ -268,7 +268,6 @@ static int solo_thread(void *data)
 	struct solo_dev *solo_dev = data;
 	DECLARE_WAITQUEUE(wait, current);
 
-	set_freezable();
 	add_wait_queue(&solo_dev->disp_thread_wait, &wait);
 
 	for (;;) {
@@ -277,7 +276,6 @@ static int solo_thread(void *data)
 		if (timeout == -ERESTARTSYS || kthread_should_stop())
 			break;
 		solo_thread_try(solo_dev);
-		try_to_freeze();
 	}
 
 	remove_wait_queue(&solo_dev->disp_thread_wait, &wait);
diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c b/drivers/media/platform/vivid/vivid-kthread-cap.c
index 1727f54..08f6fe4 100644
--- a/drivers/media/platform/vivid/vivid-kthread-cap.c
+++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
@@ -745,8 +745,6 @@ static int vivid_thread_vid_cap(void *data)
 
 	dprintk(dev, 1, "Video Capture Thread Start\n");
 
-	set_freezable();
-
 	/* Resets frame counters */
 	dev->cap_seq_offset = 0;
 	dev->cap_seq_count = 0;
@@ -754,7 +752,6 @@ static int vivid_thread_vid_cap(void *data)
 	dev->jiffies_vid_cap = jiffies;
 
 	for (;;) {
-		try_to_freeze();
 		if (kthread_should_stop())
 			break;
 
diff --git a/drivers/media/platform/vivid/vivid-kthread-out.c b/drivers/media/platform/vivid/vivid-kthread-out.c
index d9f36cc..ea76749 100644
--- a/drivers/media/platform/vivid/vivid-kthread-out.c
+++ b/drivers/media/platform/vivid/vivid-kthread-out.c
@@ -132,8 +132,6 @@ static int vivid_thread_vid_out(void *data)
 
 	dprintk(dev, 1, "Video Output Thread Start\n");
 
-	set_freezable();
-
 	/* Resets frame counters */
 	dev->out_seq_offset = 0;
 	if (dev->seq_wrap)
@@ -143,7 +141,6 @@ static int vivid_thread_vid_out(void *data)
 	dev->out_seq_resync = false;
 
 	for (;;) {
-		try_to_freeze();
 		if (kthread_should_stop())
 			break;
 
diff --git a/drivers/media/platform/vivid/vivid-sdr-cap.c b/drivers/media/platform/vivid/vivid-sdr-cap.c
index d2f2188..15d6d02 100644
--- a/drivers/media/platform/vivid/vivid-sdr-cap.c
+++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
@@ -136,8 +136,6 @@ static int vivid_thread_sdr_cap(void *data)
 
 	dprintk(dev, 1, "SDR Capture Thread Start\n");
 
-	set_freezable();
-
 	/* Resets frame counters */
 	dev->sdr_cap_seq_offset = 0;
 	if (dev->seq_wrap)
@@ -146,7 +144,6 @@ static int vivid_thread_sdr_cap(void *data)
 	dev->sdr_cap_seq_resync = false;
 
 	for (;;) {
-		try_to_freeze();
 		if (kthread_should_stop())
 			break;
 
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-dvb.c b/drivers/media/usb/pvrusb2/pvrusb2-dvb.c
index 8c95793..5d70953 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-dvb.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-dvb.c
@@ -39,16 +39,12 @@ static int pvr2_dvb_feed_func(struct pvr2_dvb_adapter *adap)
 	struct pvr2_stream *stream;
 
 	pvr2_trace(PVR2_TRACE_DVB_FEED, "dvb feed thread started");
-	set_freezable();
 
 	stream = adap->channel.stream->stream;
 
 	for (;;) {
 		if (kthread_should_stop()) break;
 
-		/* Not sure about this... */
-		try_to_freeze();
-
 		bp = pvr2_stream_get_ready_buffer(stream);
 		if (bp != NULL) {
 			count = pvr2_buffer_get_count(bp);
diff --git a/drivers/media/v4l2-core/videobuf-dvb.c b/drivers/media/v4l2-core/videobuf-dvb.c
index b7efa45..3413e51 100644
--- a/drivers/media/v4l2-core/videobuf-dvb.c
+++ b/drivers/media/v4l2-core/videobuf-dvb.c
@@ -48,7 +48,6 @@ static int videobuf_dvb_thread(void *data)
 	void *outp;
 
 	dprintk("dvb thread started\n");
-	set_freezable();
 	videobuf_read_start(&dvb->dvbq);
 
 	for (;;) {
@@ -63,7 +62,6 @@ static int videobuf_dvb_thread(void *data)
 			break;
 		if (kthread_should_stop())
 			break;
-		try_to_freeze();
 
 		/* feed buffer data to demux */
 		outp = videobuf_queue_to_vaddr(&dvb->dvbq, buf);
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 4f59b7e..d1107b4 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -3212,8 +3212,6 @@ static int vb2_thread(void *data)
 			V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	}
 
-	set_freezable();
-
 	for (;;) {
 		struct vb2_buffer *vb;
 
@@ -3235,7 +3233,6 @@ static int vb2_thread(void *data)
 		}
 		if (ret || threadio->stop)
 			break;
-		try_to_freeze();
 
 		vb = q->bufs[fileio->b.index];
 		if (!(fileio->b.flags & V4L2_BUF_FLAG_ERROR))
diff --git a/drivers/mfd/ucb1x00-ts.c b/drivers/mfd/ucb1x00-ts.c
index 1e0e20c..250e941 100644
--- a/drivers/mfd/ucb1x00-ts.c
+++ b/drivers/mfd/ucb1x00-ts.c
@@ -211,7 +211,6 @@ static int ucb1x00_thread(void *_ts)
 	bool frozen, ignore = false;
 	int valid = 0;
 
-	set_freezable();
 	add_wait_queue(&ts->irq_wait, &wait);
 	while (!kthread_freezable_should_stop(&frozen)) {
 		unsigned int x, y, p;
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index eb4489f9..138fa35 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1415,16 +1415,12 @@ int ubi_thread(void *u)
 	ubi_msg(ubi, "background thread \"%s\" started, PID %d",
 		ubi->bgt_name, task_pid_nr(current));
 
-	set_freezable();
 	for (;;) {
 		int err;
 
 		if (kthread_should_stop())
 			break;
 
-		if (try_to_freeze())
-			continue;
-
 		spin_lock(&ubi->wl_lock);
 		if (list_empty(&ubi->works) || ubi->ro_mode ||
 		    !ubi->thread_enabled || ubi_dbg_is_bgt_disabled(ubi)) {
diff --git a/drivers/net/irda/stir4200.c b/drivers/net/irda/stir4200.c
index 83cc48a..3d5b311 100644
--- a/drivers/net/irda/stir4200.c
+++ b/drivers/net/irda/stir4200.c
@@ -747,8 +747,6 @@ static int stir_transmit_thread(void *arg)
 
 			write_reg(stir, REG_CTRL1, CTRL1_TXPWD|CTRL1_RXPWD);
 
-			try_to_freeze();
-
 			if (change_speed(stir, stir->speed))
 				break;
 		}
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index d0c97c2..addc3d6 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -3062,10 +3062,8 @@ static int airo_thread(void *data) {
 	struct airo_info *ai = dev->ml_priv;
 	int locked;
 
-	set_freezable();
 	while(1) {
 		/* make swsusp happy with our thread */
-		try_to_freeze();
 
 		if (test_bit(JOB_DIE, &ai->jobs))
 			break;
diff --git a/drivers/pcmcia/cs.c b/drivers/pcmcia/cs.c
index c3b615c..96302df 100644
--- a/drivers/pcmcia/cs.c
+++ b/drivers/pcmcia/cs.c
@@ -615,7 +615,6 @@ static int pccardd(void *__skt)
 	/* wait for userspace to catch up */
 	msleep(250);
 
-	set_freezable();
 	for (;;) {
 		unsigned long flags;
 		unsigned int events;
@@ -675,7 +674,6 @@ static int pccardd(void *__skt)
 		/* make sure we are running */
 		__set_current_state(TASK_RUNNING);
 
-		try_to_freeze();
 	}
 
 	/* shut down socket, if a device is still present */
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 131dd74..45fef60 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -2485,8 +2485,6 @@ static int hotkey_kthread(void *data)
 	if (tpacpi_lifecycle == TPACPI_LIFE_EXITING)
 		goto exit;
 
-	set_freezable();
-
 	so = 0;
 	si = 1;
 	t = 0;
diff --git a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c
index facd43b..7695223 100644
--- a/drivers/pnp/pnpbios/core.c
+++ b/drivers/pnp/pnpbios/core.c
@@ -153,7 +153,6 @@ static int pnp_dock_thread(void *unused)
 	static struct pnp_docking_station_info now;
 	int docked = -1, d = 0;
 
-	set_freezable();
 	while (1) {
 		int status;
 
@@ -162,9 +161,6 @@ static int pnp_dock_thread(void *unused)
 		 */
 		msleep_interruptible(2000);
 
-		if (try_to_freeze())
-			continue;
-
 		status = pnp_bios_dock_station_info(&now);
 
 		switch (status) {
diff --git a/drivers/power/tps65090-charger.c b/drivers/power/tps65090-charger.c
index 7e8fbd2..adf4f07 100644
--- a/drivers/power/tps65090-charger.c
+++ b/drivers/power/tps65090-charger.c
@@ -218,11 +218,8 @@ static struct tps65090_platform_data *
 
 static int tps65090_charger_poll_task(void *data)
 {
-	set_freezable();
-
 	while (!kthread_should_stop()) {
 		schedule_timeout_interruptible(POLL_INTERVAL);
-		try_to_freeze();
 		tps65090_charger_isr(-1, data);
 	}
 	return 0;
diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c
index fd13d05..f675b3f 100644
--- a/drivers/staging/android/ion/ion_heap.c
+++ b/drivers/staging/android/ion/ion_heap.c
@@ -229,7 +229,7 @@ static int ion_heap_deferred_free(void *data)
 	while (true) {
 		struct ion_buffer *buffer;
 
-		wait_event_freezable(heap->waitqueue,
+		wait_event_interruptible(heap->waitqueue,
 				     ion_heap_freelist_size(heap) > 0);
 
 		spin_lock(&heap->free_lock);
diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
index 6c79588..36ce4660 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -379,7 +379,6 @@ static int clamp_thread(void *arg)
 	unsigned int target_ratio;
 
 	set_bit(cpunr, cpu_clamping_mask);
-	set_freezable();
 	init_timer_on_stack(&wakeup_timer);
 	sched_setscheduler(current, SCHED_FIFO, &param);
 
@@ -393,7 +392,6 @@ static int clamp_thread(void *arg)
 		unsigned int duration_jiffies = msecs_to_jiffies(duration);
 		unsigned int window_size_now;
 
-		try_to_freeze();
 		/*
 		 * make sure user selected ratio does not take effect until
 		 * the next round. adjust target_ratio if user has changed
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 4e9c4cc..764a671 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -740,11 +740,9 @@ static int khvcd(void *unused)
 	int poll_mask;
 	struct hvc_struct *hp;
 
-	set_freezable();
 	do {
 		poll_mask = 0;
 		hvc_kicked = 0;
-		try_to_freeze();
 		wmb();
 		if (!cpus_are_in_xmon()) {
 			spin_lock(&hvc_structs_lock);
diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index a2ae88d..3773711 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -1930,7 +1930,6 @@ static int uea_kthread(void *data)
 	struct uea_softc *sc = data;
 	int ret = -EAGAIN;
 
-	set_freezable();
 	uea_enters(INS_TO_USBDEV(sc));
 	while (!kthread_should_stop()) {
 		if (ret < 0 || sc->reset)
@@ -1939,7 +1938,6 @@ static int uea_kthread(void *data)
 			ret = sc->stat(sc);
 		if (ret != -EAGAIN)
 			uea_wait(sc, 0, msecs_to_jiffies(1000));
-		try_to_freeze();
 	}
 	uea_leaves(INS_TO_USBDEV(sc));
 	return ret;
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index a6eb537..025c000 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -611,14 +611,12 @@ static bool start_out_transfer(struct fsg_common *common, struct fsg_buffhd *bh)
 	return true;
 }
 
-static int sleep_thread(struct fsg_common *common, bool can_freeze)
+static int sleep_thread(struct fsg_common *common)
 {
 	int	rc = 0;
 
 	/* Wait until a signal arrives or we are woken up */
 	for (;;) {
-		if (can_freeze)
-			try_to_freeze();
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (signal_pending(current)) {
 			rc = -EINTR;
@@ -692,7 +690,7 @@ static int do_read(struct fsg_common *common)
 		/* Wait for the next buffer to become available */
 		bh = common->next_buffhd_to_fill;
 		while (bh->state != BUF_STATE_EMPTY) {
-			rc = sleep_thread(common, false);
+			rc = sleep_thread(common);
 			if (rc)
 				return rc;
 		}
@@ -947,7 +945,7 @@ static int do_write(struct fsg_common *common)
 		}
 
 		/* Wait for something to happen */
-		rc = sleep_thread(common, false);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -1514,7 +1512,7 @@ static int throw_away_data(struct fsg_common *common)
 		}
 
 		/* Otherwise wait for something to happen */
-		rc = sleep_thread(common, true);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -1635,7 +1633,7 @@ static int send_status(struct fsg_common *common)
 	/* Wait for the next buffer to become available */
 	bh = common->next_buffhd_to_fill;
 	while (bh->state != BUF_STATE_EMPTY) {
-		rc = sleep_thread(common, true);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -1838,7 +1836,7 @@ static int do_scsi_command(struct fsg_common *common)
 	bh = common->next_buffhd_to_fill;
 	common->next_buffhd_to_drain = bh;
 	while (bh->state != BUF_STATE_EMPTY) {
-		rc = sleep_thread(common, true);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -2185,7 +2183,7 @@ static int get_next_command(struct fsg_common *common)
 	/* Wait for the next buffer to become available */
 	bh = common->next_buffhd_to_fill;
 	while (bh->state != BUF_STATE_EMPTY) {
-		rc = sleep_thread(common, true);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -2204,7 +2202,7 @@ static int get_next_command(struct fsg_common *common)
 
 	/* Wait for the CBW to arrive */
 	while (bh->state != BUF_STATE_FULL) {
-		rc = sleep_thread(common, true);
+		rc = sleep_thread(common);
 		if (rc)
 			return rc;
 	}
@@ -2390,7 +2388,7 @@ static void handle_exception(struct fsg_common *common)
 			}
 			if (num_active == 0)
 				break;
-			if (sleep_thread(common, true))
+			if (sleep_thread(common))
 				return;
 		}
 
@@ -2509,9 +2507,6 @@ static int fsg_main_thread(void *common_)
 	allow_signal(SIGKILL);
 	allow_signal(SIGUSR1);
 
-	/* Allow the thread to be frozen */
-	set_freezable();
-
 	/*
 	 * Arrange for userspace references to be interpreted as kernel
 	 * pointers.  That way we can pass a kernel pointer to a routine
@@ -2527,7 +2522,7 @@ static int fsg_main_thread(void *common_)
 		}
 
 		if (!common->running) {
-			sleep_thread(common, true);
+			sleep_thread(common);
 			continue;
 		}
 
diff --git a/drivers/uwb/uwbd.c b/drivers/uwb/uwbd.c
index bdcb13c..01c20a2 100644
--- a/drivers/uwb/uwbd.c
+++ b/drivers/uwb/uwbd.c
@@ -279,7 +279,6 @@ static int uwbd(void *param)
 			HZ);
 		if (should_stop)
 			break;
-		try_to_freeze();
 
 		spin_lock_irqsave(&rc->uwbd.event_list_lock, flags);
 		if (!list_empty(&rc->uwbd.event_list)) {
diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c
index b269abd..0528189 100644
--- a/drivers/video/fbdev/ps3fb.c
+++ b/drivers/video/fbdev/ps3fb.c
@@ -890,9 +890,7 @@ static int ps3fbd(void *arg)
 {
 	struct fb_info *info = arg;
 
-	set_freezable();
 	while (!kthread_should_stop()) {
-		try_to_freeze();
 		set_current_state(TASK_INTERRUPTIBLE);
 		if (ps3fb.is_kicked) {
 			ps3fb.is_kicked = 0;
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 94813af..0c1745a 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -1269,12 +1269,8 @@ static int pxafb_smart_thread(void *arg)
 
 	pr_debug("%s(): task starting\n", __func__);
 
-	set_freezable();
 	while (!kthread_should_stop()) {
 
-		if (try_to_freeze())
-			continue;
-
 		mutex_lock(&fbi->ctrlr_lock);
 
 		if (fbi->state == C_ENABLE) {
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7efc329..7b61570 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -350,12 +350,9 @@ static int balloon(void *_vballoon)
 	struct virtio_balloon *vb = _vballoon;
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 
-	set_freezable();
 	while (!kthread_should_stop()) {
 		s64 diff;
 
-		try_to_freeze();
-
 		add_wait_queue(&vb->config_change, &wait);
 		for (;;) {
 			if ((diff = towards_target(vb)) != 0 ||
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index c9a7ff6..89a7847 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -1147,7 +1147,6 @@ int w1_process(void *data)
 			jremain = 1;
 		}
 
-		try_to_freeze();
 		__set_current_state(TASK_INTERRUPTIBLE);
 
 		/* hold list_mutex until after interruptible to prevent loosing
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1e60d00..189f607 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1798,7 +1798,7 @@ static int cleaner_kthread(void *arg)
 		 */
 		btrfs_delete_unused_bgs(root->fs_info);
 sleep:
-		if (!try_to_freeze() && !again) {
+		if (!again) {
 			set_current_state(TASK_INTERRUPTIBLE);
 			if (!kthread_should_stop())
 				schedule();
@@ -1888,14 +1888,12 @@ sleep:
 		if (unlikely(test_bit(BTRFS_FS_STATE_ERROR,
 				      &root->fs_info->fs_state)))
 			btrfs_cleanup_transaction(root);
-		if (!try_to_freeze()) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			if (!kthread_should_stop() &&
-			    (!btrfs_transaction_blocked(root->fs_info) ||
-			     cannot_commit))
-				schedule_timeout(delay);
-			__set_current_state(TASK_RUNNING);
-		}
+		set_current_state(TASK_INTERRUPTIBLE);
+		if (!kthread_should_stop() &&
+		    (!btrfs_transaction_blocked(root->fs_info) ||
+		     cannot_commit))
+			schedule_timeout(delay);
+		__set_current_state(TASK_RUNNING);
 	} while (!kthread_should_stop());
 	return 0;
 }
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 773f4dc..33a4cb1 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -384,7 +384,6 @@ cifs_reconnect(struct TCP_Server_Info *server)
 	}
 
 	do {
-		try_to_freeze();
 
 		/* we should try only the port we connected to before */
 		mutex_lock(&server->srv_mutex);
@@ -563,7 +562,6 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct kvec *iov_orig,
 	smb_msg.msg_controllen = 0;
 
 	for (total_read = 0; to_read; total_read += length, to_read -= length) {
-		try_to_freeze();
 
 		if (server_unresponsive(server)) {
 			total_read = -ECONNABORTED;
@@ -855,10 +853,7 @@ cifs_demultiplex_thread(void *p)
 	if (length > 1)
 		mempool_resize(cifs_req_poolp, length + cifs_min_rcv);
 
-	set_freezable();
 	while (server->tcpStatus != CifsExiting) {
-		if (try_to_freeze())
-			continue;
 
 		if (!allocate_buffers(server))
 			continue;
diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
index 866bb18..fa3e9b5 100644
--- a/fs/ecryptfs/kthread.c
+++ b/fs/ecryptfs/kthread.c
@@ -55,11 +55,10 @@ static struct task_struct *ecryptfs_kthread;
  */
 static int ecryptfs_threadfn(void *ignored)
 {
-	set_freezable();
 	while (1)  {
 		struct ecryptfs_open_req *req;
 
-		wait_event_freezable(
+		wait_event_interruptible(
 			ecryptfs_kthread_ctl.wait,
 			(!list_empty(&ecryptfs_kthread_ctl.req_list)
 			 || kthread_should_stop()));
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a63c7b0..c2e9032 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3036,8 +3036,6 @@ cont_thread:
 		}
 		mutex_unlock(&eli->li_list_mtx);
 
-		try_to_freeze();
-
 		cur = jiffies;
 		if ((time_after_eq(cur, next_wakeup)) ||
 		    (MAX_JIFFY_OFFSET == next_wakeup)) {
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 782b8e7..026fcd7 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -34,12 +34,9 @@ static int gc_thread_func(void *data)
 	wait_ms = gc_th->min_sleep_time;
 
 	do {
-		if (try_to_freeze())
-			continue;
-		else
-			wait_event_interruptible_timeout(*wq,
-						kthread_should_stop(),
-						msecs_to_jiffies(wait_ms));
+		wait_event_interruptible_timeout(*wq,
+					kthread_should_stop(),
+					msecs_to_jiffies(wait_ms));
 		if (kthread_should_stop())
 			break;
 
diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
index 536e7a6..d352b06 100644
--- a/fs/gfs2/log.c
+++ b/fs/gfs2/log.c
@@ -930,8 +930,6 @@ int gfs2_logd(void *data)
 
 		t = gfs2_tune_get(sdp, gt_logd_secs) * HZ;
 
-		try_to_freeze();
-
 		do {
 			prepare_to_wait(&sdp->sd_logd_waitq, &wait,
 					TASK_INTERRUPTIBLE);
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 3a31226..463eeb0 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1520,8 +1520,6 @@ int gfs2_quotad(void *data)
 		/* Check for & recover partially truncated inodes */
 		quotad_check_trunc_list(sdp);
 
-		try_to_freeze();
-
 		t = min(quotad_timeo, statfs_timeo);
 
 		prepare_to_wait(&sdp->sd_quota_wait, &wait, TASK_INTERRUPTIBLE);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 8270fe9..1364fe4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -198,8 +198,6 @@ static int kjournald2(void *arg)
 	setup_timer(&journal->j_commit_timer, commit_timeout,
 			(unsigned long)current);
 
-	set_freezable();
-
 	/* Record that the journal thread is running */
 	journal->j_task = current;
 	wake_up(&journal->j_wait_done_commit);
@@ -234,7 +232,6 @@ loop:
 		 */
 		jbd_debug(1, "Now suspending kjournald2\n");
 		write_unlock(&journal->j_state_lock);
-		try_to_freeze();
 		write_lock(&journal->j_state_lock);
 	} else {
 		/*
diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index bb9cebc..8de17e7 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -88,7 +88,6 @@ static int jffs2_garbage_collect_thread(void *_c)
 
 	set_user_nice(current, 10);
 
-	set_freezable();
 	for (;;) {
 		sigprocmask(SIG_UNBLOCK, &hupmask, NULL);
 	again:
@@ -124,9 +123,6 @@ static int jffs2_garbage_collect_thread(void *_c)
 			siginfo_t info;
 			unsigned long signr;
 
-			if (try_to_freeze())
-				goto again;
-
 			signr = dequeue_signal_lock(current, &current->blocked, &info);
 
 			switch(signr) {
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index a69bdf2..6c92f30 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -2347,7 +2347,6 @@ int jfsIOWait(void *arg)
 
 		if (freezing(current)) {
 			spin_unlock_irq(&log_redrive_lock);
-			try_to_freeze();
 		} else {
 			set_current_state(TASK_INTERRUPTIBLE);
 			spin_unlock_irq(&log_redrive_lock);
diff --git a/fs/jfs/jfs_txnmgr.c b/fs/jfs/jfs_txnmgr.c
index d595856..819385a 100644
--- a/fs/jfs/jfs_txnmgr.c
+++ b/fs/jfs/jfs_txnmgr.c
@@ -2797,18 +2797,13 @@ int jfs_lazycommit(void *arg)
 		/* In case a wakeup came while all threads were active */
 		jfs_commit_thread_waking = 0;
 
-		if (freezing(current)) {
-			LAZY_UNLOCK(flags);
-			try_to_freeze();
-		} else {
-			DECLARE_WAITQUEUE(wq, current);
+		DECLARE_WAITQUEUE(wq, current);
 
-			add_wait_queue(&jfs_commit_thread_wait, &wq);
-			set_current_state(TASK_INTERRUPTIBLE);
-			LAZY_UNLOCK(flags);
-			schedule();
-			remove_wait_queue(&jfs_commit_thread_wait, &wq);
-		}
+		add_wait_queue(&jfs_commit_thread_wait, &wq);
+		set_current_state(TASK_INTERRUPTIBLE);
+		LAZY_UNLOCK(flags);
+		schedule();
+		remove_wait_queue(&jfs_commit_thread_wait, &wq);
 	} while (!kthread_should_stop());
 
 	if (!list_empty(&TxAnchor.unlock_queue))
@@ -2987,14 +2982,9 @@ int jfs_sync(void *arg)
 		/* Add anon_list2 back to anon_list */
 		list_splice_init(&TxAnchor.anon_list2, &TxAnchor.anon_list);
 
-		if (freezing(current)) {
-			TXN_UNLOCK();
-			try_to_freeze();
-		} else {
-			set_current_state(TASK_INTERRUPTIBLE);
-			TXN_UNLOCK();
-			schedule();
-		}
+		set_current_state(TASK_INTERRUPTIBLE);
+		TXN_UNLOCK();
+		schedule();
 	} while (!kthread_should_stop());
 
 	jfs_info("jfs_sync being killed");
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index d678bcc..b976670 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -130,9 +130,6 @@ lockd(void *vrqstp)
 	int		err = 0;
 	struct svc_rqst *rqstp = vrqstp;
 
-	/* try_to_freeze() is called from svc_recv() */
-	set_freezable();
-
 	/* Allow SIGKILL to tell lockd to drop all of its locks */
 	allow_signal(SIGKILL);
 
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 75f7c0a..1f4ef49 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -75,8 +75,6 @@ nfs4_callback_svc(void *vrqstp)
 	int err;
 	struct svc_rqst *rqstp = vrqstp;
 
-	set_freezable();
-
 	while (!kthread_should_stop()) {
 		/*
 		 * Listen for a request on the socket
@@ -122,11 +120,7 @@ nfs41_callback_svc(void *vrqstp)
 	int error;
 	DEFINE_WAIT(wq);
 
-	set_freezable();
-
 	while (!kthread_should_stop()) {
-		if (try_to_freeze())
-			continue;
 
 		prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE);
 		spin_lock_bh(&serv->sv_cb_lock);
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index ad4e237..16ee084 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -605,8 +605,6 @@ nfsd(void *vrqstp)
 	nfsdstats.th_cnt++;
 	mutex_unlock(&nfsd_mutex);
 
-	set_freezable();
-
 	/*
 	 * The main request loop
 	 */
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index c6abbad9..c37069c 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2506,37 +2506,31 @@ static int nilfs_segctor_thread(void *arg)
 	}
 
 
-	if (freezing(current)) {
+	DEFINE_WAIT(wait);
+	int should_sleep = 1;
+
+	prepare_to_wait(&sci->sc_wait_daemon, &wait,
+			TASK_INTERRUPTIBLE);
+
+	if (sci->sc_seq_request != sci->sc_seq_done)
+		should_sleep = 0;
+	else if (sci->sc_flush_request)
+		should_sleep = 0;
+	else if (sci->sc_state & NILFS_SEGCTOR_COMMIT)
+		should_sleep = time_before(jiffies,
+				sci->sc_timer.expires);
+
+	if (should_sleep) {
 		spin_unlock(&sci->sc_state_lock);
-		try_to_freeze();
+		schedule();
 		spin_lock(&sci->sc_state_lock);
-	} else {
-		DEFINE_WAIT(wait);
-		int should_sleep = 1;
-
-		prepare_to_wait(&sci->sc_wait_daemon, &wait,
-				TASK_INTERRUPTIBLE);
-
-		if (sci->sc_seq_request != sci->sc_seq_done)
-			should_sleep = 0;
-		else if (sci->sc_flush_request)
-			should_sleep = 0;
-		else if (sci->sc_state & NILFS_SEGCTOR_COMMIT)
-			should_sleep = time_before(jiffies,
-					sci->sc_timer.expires);
-
-		if (should_sleep) {
-			spin_unlock(&sci->sc_state_lock);
-			schedule();
-			spin_lock(&sci->sc_state_lock);
-		}
-		finish_wait(&sci->sc_wait_daemon, &wait);
-		timeout = ((sci->sc_state & NILFS_SEGCTOR_COMMIT) &&
-			   time_after_eq(jiffies, sci->sc_timer.expires));
-
-		if (nilfs_sb_dirty(nilfs) && nilfs_sb_need_update(nilfs))
-			set_nilfs_discontinued(nilfs);
 	}
+	finish_wait(&sci->sc_wait_daemon, &wait);
+	timeout = ((sci->sc_state & NILFS_SEGCTOR_COMMIT) &&
+		   time_after_eq(jiffies, sci->sc_timer.expires));
+
+	if (nilfs_sb_dirty(nilfs) && nilfs_sb_need_update(nilfs))
+		set_nilfs_discontinued(nilfs);
 	goto loop;
 
  end_thread:
diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c
index 63f5661..c760f85 100644
--- a/fs/ubifs/commit.c
+++ b/fs/ubifs/commit.c
@@ -291,15 +291,11 @@ int ubifs_bg_thread(void *info)
 
 	ubifs_msg(c, "background thread \"%s\" started, PID %d",
 		  c->bgt_name, current->pid);
-	set_freezable();
 
 	while (1) {
 		if (kthread_should_stop())
 			break;
 
-		if (try_to_freeze())
-			continue;
-
 		set_current_state(TASK_INTERRUPTIBLE);
 		/* Check if there is something to do */
 		if (!c->need_bgt) {
diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 1098cf4..250aef7 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -530,8 +530,6 @@ xfsaild(
 
 		__set_current_state(TASK_RUNNING);
 
-		try_to_freeze();
-
 		tout = xfsaild_push(ailp);
 	}
 
diff --git a/kernel/audit.c b/kernel/audit.c
index 662c007..b5452c1 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -498,7 +498,6 @@ static void flush_hold_queue(void)
 
 static int kauditd_thread(void *dummy)
 {
-	set_freezable();
 	while (!kthread_should_stop()) {
 		struct sk_buff *skb;
 
@@ -516,7 +515,7 @@ static int kauditd_thread(void *dummy)
 			continue;
 		}
 
-		wait_event_freezable(kauditd_wait, skb_queue_len(&audit_skb_queue));
+		wait_event(kauditd_wait, skb_queue_len(&audit_skb_queue));
 	}
 	return 0;
 }
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 9ff173d..bddac62 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -596,7 +596,6 @@ repeat:
 	} else if (!freezing(current))
 		schedule();
 
-	try_to_freeze();
 	goto repeat;
 }
 EXPORT_SYMBOL_GPL(kthread_worker_fn);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index bbac913..7c343b2 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2877,7 +2877,7 @@ static void khugepaged_do_scan(void)
 
 		cond_resched();
 
-		if (unlikely(kthread_should_stop() || try_to_freeze()))
+		if (unlikely(kthread_should_stop()))
 			break;
 
 		spin_lock(&khugepaged_mm_lock);
@@ -2902,21 +2902,20 @@ static void khugepaged_wait_work(void)
 		if (!khugepaged_scan_sleep_millisecs)
 			return;
 
-		wait_event_freezable_timeout(khugepaged_wait,
+		wait_event_interruptible_timeout(khugepaged_wait,
 					     kthread_should_stop(),
 			msecs_to_jiffies(khugepaged_scan_sleep_millisecs));
 		return;
 	}
 
 	if (khugepaged_enabled())
-		wait_event_freezable(khugepaged_wait, khugepaged_wait_event());
+		wait_event_interruptible(khugepaged_wait, khugepaged_wait_event());
 }
 
 static int khugepaged(void *none)
 {
 	struct mm_slot *mm_slot;
 
-	set_freezable();
 	set_user_nice(current, MAX_NICE);
 
 	while (!kthread_should_stop()) {
diff --git a/mm/ksm.c b/mm/ksm.c
index 7ee101e..21f4c48 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -1712,7 +1712,6 @@ static int ksmd_should_run(void)
 
 static int ksm_scan_thread(void *nothing)
 {
-	set_freezable();
 	set_user_nice(current, 5);
 
 	while (!kthread_should_stop()) {
@@ -1722,13 +1721,11 @@ static int ksm_scan_thread(void *nothing)
 			ksm_do_scan(ksm_thread_pages_to_scan);
 		mutex_unlock(&ksm_thread_mutex);
 
-		try_to_freeze();
-
 		if (ksmd_should_run()) {
 			schedule_timeout_interruptible(
 				msecs_to_jiffies(ksm_thread_sleep_millisecs));
 		} else {
-			wait_event_freezable(ksm_thread_wait,
+			wait_event_interruptible(ksm_thread_wait,
 				ksmd_should_run() || kthread_should_stop());
 		}
 	}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7f63a93..93520f7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3314,7 +3314,7 @@ static unsigned long balance_pgdat(pg_data_t *pgdat, int order,
 			order = sc.order = 0;
 
 		/* Check if kswapd should be suspending */
-		if (try_to_freeze() || kthread_should_stop())
+		if (kthread_should_stop())
 			break;
 
 		/*
@@ -3445,7 +3445,6 @@ static int kswapd(void *p)
 	 * trying to free the first piece of memory in the first place).
 	 */
 	tsk->flags |= PF_MEMALLOC | PF_SWAPWRITE | PF_KSWAPD;
-	set_freezable();
 
 	order = new_order = 0;
 	balanced_order = 0;
@@ -3485,7 +3484,6 @@ static int kswapd(void *p)
 			pgdat->classzone_idx = pgdat->nr_zones - 1;
 		}
 
-		ret = try_to_freeze();
 		if (kthread_should_stop())
 			break;
 
@@ -3493,12 +3491,10 @@ static int kswapd(void *p)
 		 * We can speed up thawing tasks if we don't call balance_pgdat
 		 * after returning from the refrigerator
 		 */
-		if (!ret) {
-			trace_mm_vmscan_kswapd_wake(pgdat->node_id, order);
-			balanced_classzone_idx = classzone_idx;
-			balanced_order = balance_pgdat(pgdat, order,
-						&balanced_classzone_idx);
-		}
+		trace_mm_vmscan_kswapd_wake(pgdat->node_id, order);
+		balanced_classzone_idx = classzone_idx;
+		balanced_order = balance_pgdat(pgdat, order,
+					&balanced_classzone_idx);
 	}
 
 	tsk->flags &= ~(PF_MEMALLOC | PF_SWAPWRITE | PF_KSWAPD);
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index de8d5cc..0ff3730 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3511,8 +3511,6 @@ static int pktgen_thread_worker(void *arg)
 
 	pr_debug("starting pktgen/%d:  pid=%d\n", cpu, task_pid_nr(current));
 
-	set_freezable();
-
 	while (!kthread_should_stop()) {
 		pkt_dev = next_to_run(t);
 
@@ -3522,7 +3520,6 @@ static int pktgen_thread_worker(void *arg)
 			wait_event_interruptible_timeout(t->queue,
 							 t->control != 0,
 							 HZ/10);
-			try_to_freeze();
 			continue;
 		}
 
@@ -3554,8 +3551,6 @@ static int pktgen_thread_worker(void *arg)
 			pktgen_rem_one_if(t);
 			t->control &= ~(T_REMDEV);
 		}
-
-		try_to_freeze();
 	}
 
 	pr_debug("%s stopping all device\n", t->tsk->comm);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index a6cbb21..ddef1dc9 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -806,7 +806,6 @@ int svc_recv(struct svc_rqst *rqstp, long timeout)
 	if (err)
 		goto out;
 
-	try_to_freeze();
 	cond_resched();
 	err = -EINTR;
 	if (signalled() || kthread_should_stop())
-- 
1.9.2

Re: [PATCH 2/3] freezer: get rid of the kthread freezer

[kbuild test robot]

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

Hi Jiri,

[auto build test WARNING on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/PM-vfs-use-filesystem-freezing-instead-of-kthread-freezer/20151030-215223
config: x86_64-randconfig-x011-10300134 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/fs.h:5:0,
                    from fs/jfs/jfs_txnmgr.c:45:
   fs/jfs/jfs_txnmgr.c: In function 'jfs_lazycommit':
>> include/linux/wait.h:57:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     wait_queue_t name = __WAITQUEUE_INITIALIZER(name, tsk)
     ^
>> fs/jfs/jfs_txnmgr.c:2800:3: note: in expansion of macro 'DECLARE_WAITQUEUE'
      DECLARE_WAITQUEUE(wq, current);
      ^
--
   In file included from include/linux/mmzone.h:9:0,
                    from include/linux/gfp.h:5,
                    from include/linux/mm.h:9,
                    from include/linux/pagemap.h:7,
                    from fs/nilfs2/segment.c:24:
   fs/nilfs2/segment.c: In function 'nilfs_segctor_thread':
   include/linux/wait.h:935:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     wait_queue_t name = {      \
     ^
>> include/linux/wait.h:941:27: note: in expansion of macro 'DEFINE_WAIT_FUNC'
    #define DEFINE_WAIT(name) DEFINE_WAIT_FUNC(name, autoremove_wake_function)
                              ^
>> fs/nilfs2/segment.c:2509:2: note: in expansion of macro 'DEFINE_WAIT'
     DEFINE_WAIT(wait);
     ^
--
   mm/vmscan.c: In function 'kswapd':
>> mm/vmscan.c:3454:8: warning: unused variable 'ret' [-Wunused-variable]
      bool ret;
           ^

vim +/DECLARE_WAITQUEUE +2800 fs/jfs/jfs_txnmgr.c

^1da177e Linus Torvalds    2005-04-16  2784  				sbi->commit_state &= ~IN_LAZYCOMMIT;
^1da177e Linus Torvalds    2005-04-16  2785  				/*
^1da177e Linus Torvalds    2005-04-16  2786  				 * Don't continue in the for loop.  (We can't
^1da177e Linus Torvalds    2005-04-16  2787  				 * anyway, it's unsafe!)  We want to go back to
^1da177e Linus Torvalds    2005-04-16  2788  				 * the beginning of the list.
^1da177e Linus Torvalds    2005-04-16  2789  				 */
^1da177e Linus Torvalds    2005-04-16  2790  				break;
^1da177e Linus Torvalds    2005-04-16  2791  			}
^1da177e Linus Torvalds    2005-04-16  2792  
^1da177e Linus Torvalds    2005-04-16  2793  			/* If there was nothing to do, don't continue */
^1da177e Linus Torvalds    2005-04-16  2794  			if (!WorkDone)
^1da177e Linus Torvalds    2005-04-16  2795  				break;
^1da177e Linus Torvalds    2005-04-16  2796  		}
^1da177e Linus Torvalds    2005-04-16  2797  		/* In case a wakeup came while all threads were active */
^1da177e Linus Torvalds    2005-04-16  2798  		jfs_commit_thread_waking = 0;
^1da177e Linus Torvalds    2005-04-16  2799  
^1da177e Linus Torvalds    2005-04-16 @2800  		DECLARE_WAITQUEUE(wq, current);
^1da177e Linus Torvalds    2005-04-16  2801  
^1da177e Linus Torvalds    2005-04-16  2802  		add_wait_queue(&jfs_commit_thread_wait, &wq);
^1da177e Linus Torvalds    2005-04-16  2803  		set_current_state(TASK_INTERRUPTIBLE);
^1da177e Linus Torvalds    2005-04-16  2804  		LAZY_UNLOCK(flags);
^1da177e Linus Torvalds    2005-04-16  2805  		schedule();
^1da177e Linus Torvalds    2005-04-16  2806  		remove_wait_queue(&jfs_commit_thread_wait, &wq);
91dbb4de Christoph Hellwig 2006-02-15  2807  	} while (!kthread_should_stop());
^1da177e Linus Torvalds    2005-04-16  2808  

:::::: The code at line 2800 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 27388 bytes --]

Re: [PATCH 2/3] freezer: get rid of the kthread freezer

[kbuild test robot]

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

Hi Jiri,

[auto build test WARNING on v4.3-rc7 -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Jiri-Kosina/PM-vfs-use-filesystem-freezing-instead-of-kthread-freezer/20151030-215223
config: x86_64-randconfig-x017-10300134 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/media/dvb-core/dvb_frontend.c: In function 'dvb_frontend_thread':
>> drivers/media/dvb-core/dvb_frontend.c:730:1: warning: label 'restart' defined but not used [-Wunused-label]
    restart:
    ^

vim +/restart +730 drivers/media/dvb-core/dvb_frontend.c

9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  714  	if (ret) {
9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  715  		/* FIXME: return an error if it fails */
9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  716  		dev_info(fe->dvb->device,
9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  717  			"proceeding with FE task\n");
8a26a258b drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-02-23  718  	} else if (fepriv->pipe_start_entity) {
135f9be91 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  719  		ret = media_entity_pipeline_start(fepriv->pipe_start_entity,
135f9be91 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  720  						  &fepriv->pipe);
135f9be91 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  721  		if (ret)
135f9be91 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  722  			return ret;
9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  723  	}
135f9be91 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  724  #endif
9239effd5 drivers/media/dvb-core/dvb_frontend.c     Mauro Carvalho Chehab 2015-01-06  725  
04c56d0e5 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-07-10  726  	dvb_frontend_init(fe);
36cb557a2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-01-09  727  
36cb557a2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-01-09  728  	while (1) {
36cb557a2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-01-09  729  		up(&fepriv->sem);	    /* is locked when we enter the thread... */
6591691b2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew Morton         2007-02-08 @730  restart:
94238e9b1 drivers/media/dvb/dvb-core/dvb_frontend.c Hans Verkuil          2011-08-27  731  		wait_event_interruptible_timeout(fepriv->wait_queue,
e42837bcd drivers/media/dvb/dvb-core/dvb_frontend.c Rafael J. Wysocki     2007-10-18  732  			dvb_frontend_should_wakeup(fe) || kthread_should_stop()
e42837bcd drivers/media/dvb/dvb-core/dvb_frontend.c Rafael J. Wysocki     2007-10-18  733  				|| freezing(current),
36cb557a2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-01-09  734  			fepriv->delay);
8eec14295 drivers/media/dvb/dvb-core/dvb_frontend.c Herbert Poetzl        2007-02-08  735  
8eec14295 drivers/media/dvb/dvb-core/dvb_frontend.c Herbert Poetzl        2007-02-08  736  		if (kthread_should_stop() || dvb_frontend_is_exiting(fe)) {
36cb557a2 drivers/media/dvb/dvb-core/dvb_frontend.c Andrew de Quincey     2006-01-09  737  			/* got signal or quitting */
6ae232245 drivers/media/dvb-core/dvb_frontend.c     Juergen Lock          2012-12-23  738  			if (!down_interruptible(&fepriv->sem))

:::::: The code at line 730 was first introduced by commit
:::::: 6591691b259f9487f374f35c0c310aa220f829c6 V4L/DVB (5209): Kthread api conversion for dvb_frontend and av7110 fix

:::::: TO: akpm@linux-foundation.org <akpm@linux-foundation.org>
:::::: CC: Mauro Carvalho Chehab <mchehab@infradead.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21521 bytes --]

[PATCH 3/3] freezer: warn if anyone is trying to use freezer on kthreads

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

kthread freezing has been deprecated in favor of fs freezing. Issue
a warning if anyone attempts to still use this API.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
 include/linux/freezer.h | 2 ++
 kernel/freezer.c        | 1 +
 2 files changed, 3 insertions(+)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 81335f6..c0bc520 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -59,6 +59,8 @@ static inline bool try_to_freeze_unsafe(void)
 
 static inline bool try_to_freeze(void)
 {
+	WARN_ON(current->flags & PF_KTHREAD);
+
 	if (!(current->flags & PF_NOFREEZE))
 		debug_check_no_locks_held();
 	return try_to_freeze_unsafe();
diff --git a/kernel/freezer.c b/kernel/freezer.c
index a8900a3..fe2afb9 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -164,6 +164,7 @@ void __thaw_task(struct task_struct *p)
 bool set_freezable(void)
 {
 	might_sleep();
+	WARN_ON(current->flags & PF_KTHREAD);
 
 	/*
 	 * Modify flags while holding freezer_lock.  This ensures the
-- 
1.9.2

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> This series is a followup to my proposal I brought up on Kernel Summit in 
> Seoul. Noone seemed to had any principal objections, so let's have wider 
> audience look into it.
> 
> In a nuthsell: freezing of kernel threads is horrible interface with 
> unclear semantics and guarantees, and I am surprised it ever worked 
> properly. Plus there are a lot of places that simply use it in a 
> completely wrong way (which is not suprising, given the lack of defined 
> semantics and requirements).
> 
> I've tested this over a series of suspend/resume cycles on several 
> machines with at least ext4, btrfs and xfs, and it survived the testing 
> without any harm.
> 
> Patch 1/3 	implements the actual change, and has a more detailed 
> 		explanation on "why?" and "how?" questions in the changelog

This patch talks about freezing in relation to hibernation.  What about 
other forms of suspend?

Also, it replaces kthread freezing with filesystem freezing.  What 
about kthreads performing I/O that doesn't go through a filesystem?  
You write:

> the only facility that is needed during suspend: "no persistent fs
> changes are allowed from now on"

I would say instead "no I/O is allowed from now on".  Maybe that's an 
overstatement, but I think it comes closer to the truth.

> Patch 2/3	nukes all (hopefully) the calls to freezer from kthreads 
> 		in Linus' tree (as of 858e904bd71)
> 
> Patch 3/3	introduces WARN_ON() if anyone is trying to make use of 
> 		this again
> 
> Open questions / discussion points:
> 
> - is the way I am traversing list of superblocks backwards enough to 
>   guarantee correct ordering? Especially: does this work as intended for 
>   FUSE?
> 
> - should freezable workqueues be dealt with the same way? I haven't even 
>   started to look into them in a serious way, but it seems like the 
>   drivers that are making use of them would actually like to use proper
>   PM callbacks instead

In the examples of freezable workqueues that I'm familiar with, what we 
really want is for the queue to stop running before the system goes any 
further into suspend.  This could be implemented with PM callbacks, but 
using the freezer instead seems a lot simpler.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> This series is a followup to my proposal I brought up on Kernel Summit in 
> Seoul. Noone seemed to had any principal objections, so let's have wider 
> audience look into it.
> 
> In a nuthsell: freezing of kernel threads is horrible interface with 
> unclear semantics and guarantees, and I am surprised it ever worked 
> properly. Plus there are a lot of places that simply use it in a 
> completely wrong way (which is not suprising, given the lack of defined 
> semantics and requirements).
> 
> I've tested this over a series of suspend/resume cycles on several 
> machines with at least ext4, btrfs and xfs, and it survived the testing 
> without any harm.
> 
> Patch 1/3 	implements the actual change, and has a more detailed 
> 		explanation on "why?" and "how?" questions in the changelog

This patch talks about freezing in relation to hibernation.  What about 
other forms of suspend?

Also, it replaces kthread freezing with filesystem freezing.  What 
about kthreads performing I/O that doesn't go through a filesystem?  
You write:

> the only facility that is needed during suspend: "no persistent fs
> changes are allowed from now on"

I would say instead "no I/O is allowed from now on".  Maybe that's an 
overstatement, but I think it comes closer to the truth.

> Patch 2/3	nukes all (hopefully) the calls to freezer from kthreads 
> 		in Linus' tree (as of 858e904bd71)
> 
> Patch 3/3	introduces WARN_ON() if anyone is trying to make use of 
> 		this again
> 
> Open questions / discussion points:
> 
> - is the way I am traversing list of superblocks backwards enough to 
>   guarantee correct ordering? Especially: does this work as intended for 
>   FUSE?
> 
> - should freezable workqueues be dealt with the same way? I haven't even 
>   started to look into them in a serious way, but it seems like the 
>   drivers that are making use of them would actually like to use proper
>   PM callbacks instead

In the examples of freezable workqueues that I'm familiar with, what we 
really want is for the queue to stop running before the system goes any 
further into suspend.  This could be implemented with PM callbacks, but 
using the freezer instead seems a lot simpler.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Pavel Machek]

On Fri 2015-10-30 11:29:08, Alan Stern wrote:
> On Fri, 30 Oct 2015, Jiri Kosina wrote:
> 
> > This series is a followup to my proposal I brought up on Kernel Summit in 
> > Seoul. Noone seemed to had any principal objections, so let's have wider 
> > audience look into it.
> > 
> > In a nuthsell: freezing of kernel threads is horrible interface with 
> > unclear semantics and guarantees, and I am surprised it ever worked 
> > properly. Plus there are a lot of places that simply use it in a 
> > completely wrong way (which is not suprising, given the lack of defined 
> > semantics and requirements).
> > 
> > I've tested this over a series of suspend/resume cycles on several 
> > machines with at least ext4, btrfs and xfs, and it survived the testing 
> > without any harm.
> > 
> > Patch 1/3 	implements the actual change, and has a more detailed 
> > 		explanation on "why?" and "how?" questions in the changelog
> 
> This patch talks about freezing in relation to hibernation.  What about 
> other forms of suspend?
> 
> Also, it replaces kthread freezing with filesystem freezing.  What 
> about kthreads performing I/O that doesn't go through a filesystem?  
> You write:
> 
> > the only facility that is needed during suspend: "no persistent fs
> > changes are allowed from now on"
> 
> I would say instead "no I/O is allowed from now on".  Maybe that's an 
> overstatement, but I think it comes closer to the truth.

Exactly. And I'm pretty sure hardware drivers do use kernel threads,
and do I/O from them.

LEDs are just one example

Best regards,
								Pavel

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

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Pavel Machek wrote:

> > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > overstatement, but I think it comes closer to the truth.

But that's what PM callbacks are for.

> Exactly. And I'm pretty sure hardware drivers do use kernel threads,
> and do I/O from them.
> 
> LEDs are just one example

And why is that relevant? First, I don't see any freezable kthread in leds 
class implementation whatsoever. Second, I am pretty sure that it's quite 
unlikely to participate in filesystem I/O.

Sure, you need to suspend and resume the devices when going through 
suspend. That's why led_suspend() exists. How would try_to_freeze() help 
at all? That's basically just a fancy schedule() called at certain points.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Pavel Machek wrote:
> 
> > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > overstatement, but I think it comes closer to the truth.
> 
> But that's what PM callbacks are for.

Why are PM callbacks any more suitable than the freezer?  The most 
natural implementation would be for the callback routine to set a flag; 
at various strategic points the kthread would check the flag and if it 
was set, call a routine that sits around and waits for the suspend to 
be over.  How does that differ from using the freezer, apart from being 
more cumbersome and involving more code?

Also, you never replied to my question about suspend vs. hibernation.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Pavel Machek wrote:
> 
> > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > overstatement, but I think it comes closer to the truth.
> 
> But that's what PM callbacks are for.

Why are PM callbacks any more suitable than the freezer?  The most 
natural implementation would be for the callback routine to set a flag; 
at various strategic points the kthread would check the flag and if it 
was set, call a routine that sits around and waits for the suspend to 
be over.  How does that differ from using the freezer, apart from being 
more cumbersome and involving more code?

Also, you never replied to my question about suspend vs. hibernation.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Fri, 30 Oct 2015, Alan Stern wrote:

> > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > overstatement, but I think it comes closer to the truth.
> > 
> > But that's what PM callbacks are for.
> 
> Why are PM callbacks any more suitable than the freezer?  

Once the PM callback triggers, you know that you are really actually 
undergoing suspend and have to do whatever is necessary.

OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
whole state needs to be prepared pro-actively for suspend already when you 
call it, each and every time, even if you are not going through suspend at 
all.

That's sub-optimal, and very easy to get wrong over gradual code changes.

> The most natural implementation would be for the callback routine to set 
> a flag; at various strategic points the kthread would check the flag and 
> if it was set, call a routine that sits around and waits for the suspend 
> to be over.  

Could you name at least some existing kthreads that would actually *need* 
such complex handling, instead of just waiting in schedule() until 
suspend-resume cycle is over, given that PM callbacks do all the necessary 
cleanup (putting HW to sleep, cancelling timers, etc) anyway?

PM callback can always explicitly do kthread_stop() on a particular 
kthread if really necessary.

> Also, you never replied to my question about suspend vs. hibernation.

The main point of freezer is to reach quiescent state wrt. filesystems 
(metadata in memory need to be absolutely in sync with what's on disk). 
That's no different between hibernation and s2ram, is it?

BTW, a quite some of this has been already "pre-discussed" in 
Documentation/power/freezing-of-tasks.txt (which has BTW been written 
before we've had the possibility to freeze filesystems, and this fact is 
even point there out).

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Friday, October 30, 2015 10:17:54 PM Jiri Kosina wrote:
> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.

Not really.  In fact, PM callbacks may not be suitable for some use cases even
in theory (that's if you want something to stop running before any PM callbacks
are executed during suspend).

> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.
> 
> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> 
> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

That's because we don't want async runtime PM to happen during system
suspend/resume and for good reasons, so if you want to remove the freezing
mechanism, you need to stop that workqueue at the beginning of dpm_prepare
and start it again at the end of dpm_complete().

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?
> 
> BTW, a quite some of this has been already "pre-discussed" in 
> Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> before we've had the possibility to freeze filesystems, and this fact is 
> even point there out).

That is somewhat outdated in my view.  At least my list of reasons for using
the freezer is now somewhat different from the one given in that file.

In any case, there may be legitimate reasons for kernel threads to be
freezable, so you need to be careful about the wholesale removal of that
feature.

The above is the only one I can recall ATM, but that doesn't mean there
aren't any other.

Thanks,
Rafael

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> > > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > > overstatement, but I think it comes closer to the truth.
> > > > 
> > > > But that's what PM callbacks are for.
> 
> Not really.  In fact, PM callbacks may not be suitable for some use cases even
> in theory (that's if you want something to stop running before any PM callbacks
> are executed during suspend).

If that's the case, wouldn't then it be better to have .pre_suspend 
callback as well, instead of having to do this kind of polling?

> > > The most natural implementation would be for the callback routine to set 
> > > a flag; at various strategic points the kthread would check the flag and 
> > > if it was set, call a routine that sits around and waits for the suspend 
> > > to be over.  
> > 
> > Could you name at least some existing kthreads that would actually *need* 
> > such complex handling, instead of just waiting in schedule() until 
> > suspend-resume cycle is over, given that PM callbacks do all the necessary 
> > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > 
> > PM callback can always explicitly do kthread_stop() on a particular 
> > kthread if really necessary.
> 
> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().

Yeah, agreed, as I said in the original mail, freezable workqueues I am 
not covering for the moment, there might be a additional issues with them 
compared to kthreads. That needs to be further looked into.

> > The main point of freezer is to reach quiescent state wrt. filesystems 
> > (metadata in memory need to be absolutely in sync with what's on disk). 
> > That's no different between hibernation and s2ram, is it?
> > 
> > BTW, a quite some of this has been already "pre-discussed" in 
> > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > before we've had the possibility to freeze filesystems, and this fact is 
> > even point there out).
> 
> That is somewhat outdated in my view.  At least my list of reasons for using
> the freezer is now somewhat different from the one given in that file.

Well, so what is the list then, and how do you know that kthread_run() 
users are behaving according to that list? That's basically exactly what 
this effort is about -- making some sense out of current situation.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Saturday, October 31, 2015 09:19:33 AM Jiri Kosina wrote:
> On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:
> 
> > > > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > > > overstatement, but I think it comes closer to the truth.
> > > > > 
> > > > > But that's what PM callbacks are for.
> > 
> > Not really.  In fact, PM callbacks may not be suitable for some use cases even
> > in theory (that's if you want something to stop running before any PM callbacks
> > are executed during suspend).
> 
> If that's the case, wouldn't then it be better to have .pre_suspend 
> callback as well, instead of having to do this kind of polling?

If that's an individual driver's kthread, I guess it should be sufficient to
stop it from the .prepare callback.  If it is something more generic, creating
a device for it just in order to be able to execute a PM callback from there
may be slightly overkill.

But again, I'm not aware of any real cases like that, so it may not be an issue
in practice.

> > > > The most natural implementation would be for the callback routine to set 
> > > > a flag; at various strategic points the kthread would check the flag and 
> > > > if it was set, call a routine that sits around and waits for the suspend 
> > > > to be over.  
> > > 
> > > Could you name at least some existing kthreads that would actually *need* 
> > > such complex handling, instead of just waiting in schedule() until 
> > > suspend-resume cycle is over, given that PM callbacks do all the necessary 
> > > cleanup (putting HW to sleep, cancelling timers, etc) anyway?
> > > 
> > > PM callback can always explicitly do kthread_stop() on a particular 
> > > kthread if really necessary.
> > 
> > Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> Yeah, agreed, as I said in the original mail, freezable workqueues I am 
> not covering for the moment, there might be a additional issues with them 
> compared to kthreads. That needs to be further looked into.

OK

> > > The main point of freezer is to reach quiescent state wrt. filesystems 
> > > (metadata in memory need to be absolutely in sync with what's on disk). 
> > > That's no different between hibernation and s2ram, is it?
> > > 
> > > BTW, a quite some of this has been already "pre-discussed" in 
> > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > before we've had the possibility to freeze filesystems, and this fact is 
> > > even point there out).
> > 
> > That is somewhat outdated in my view.  At least my list of reasons for using
> > the freezer is now somewhat different from the one given in that file.
> 
> Well, so what is the list then, and how do you know that kthread_run() 
> users are behaving according to that list? That's basically exactly what 
> this effort is about -- making some sense out of current situation.

Currently, the #1 reason for using the freezer is to prevent user space
from interacting with devices during system suspend/resume.  BTW, that also
covers devices with runtime PM support, because the handling of user space
access to them in the runtime PM case may be different.

For example, if user space does a "read" or "write" on a character device
which is runtime-suspended at that point, the driver may want to resume the
device temporarily, carry out the operation and suspend it again, but that
generally won't work for the system suspend case.

Note that this applies to both system suspend (all variants of it for that
matter) and hibernation.

The #2 reason in my view is that essentially the freezer is what makes a
difference between runtime idle and system suspend on platforms without
firmware/hardware suspend support (or the lightweight variants of suspend
in general).  Namely, it effectively prevents user space from setting up
timers in the future and helps to reduce interrupt noise causing the CPUs
to leave deep low-power states too often (at least in some cases).

The #3 reason is to provide a way for things that might touch persistent
storage after a hibernation image had been created to prevent themselves from
running during that time.  Note that it also is a good idea to prevent such
things from running during system suspend/resume in general so they don't
try to access devices at wrong times.

BTW, the freezing of filesystems during system suspend (not hibernation) makes
sense too, because it will help to address the long-standing issue with storage
devices that go away while the system is suspended.

I guess it may also helps to address the case when a device is removed from a
suspended system, written to on another system in the meantime and inserted
back into the (still suspended) original system which then is resumed.  Today
this is an almost guaranteed data corruption scenario, but if the filesystem in
question is properly frozen during suspend, the driver should be able to detect
superblock changes during unfreeze.

So the approach I'd suggest would be to add the freezing of filesystems to the
suspend/resume code paths just for the above reasons and drop the kthreads
freezing from the filesystems that support the proper freezing.  The rest
should be easier to deal with then.

Thanks,
Rafael

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jiri Kosina]

On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > > > BTW, a quite some of this has been already "pre-discussed" in 
> > > > Documentation/power/freezing-of-tasks.txt (which has BTW been written 
> > > > before we've had the possibility to freeze filesystems, and this fact is 
> > > > even point there out).
> > > 
> > > That is somewhat outdated in my view.  At least my list of reasons for using
> > > the freezer is now somewhat different from the one given in that file.
> > 
> > Well, so what is the list then, and how do you know that kthread_run() 
> > users are behaving according to that list? That's basically exactly what 
> > this effort is about -- making some sense out of current situation.
> 
> Currently, the #1 reason for using the freezer is to prevent user space
> from interacting with devices during system suspend/resume.  BTW, that also
> covers devices with runtime PM support, because the handling of user space
> access to them in the runtime PM case may be different.
> 
> For example, if user space does a "read" or "write" on a character device
> which is runtime-suspended at that point, the driver may want to resume the
> device temporarily, carry out the operation and suspend it again, but that
> generally won't work for the system suspend case.

But why would this even be relevant in this discussion, given that at the 
point we are talking about, the whole userspace has been frozen already?

> The #2 reason in my view is that essentially the freezer is what makes a
> difference between runtime idle and system suspend on platforms without
> firmware/hardware suspend support (or the lightweight variants of suspend
> in general).  Namely, it effectively prevents user space from setting up
> timers in the future and helps to reduce interrupt noise causing the CPUs
> to leave deep low-power states too often (at least in some cases).

I am of course at all not trying to remove freezer for userspace. This 
whole discussion is solely about explicit usage of freezer in kthreads.

> The #3 reason is to provide a way for things that might touch persistent
> storage after a hibernation image had been created to prevent themselves from
> running during that time.  Note that it also is a good idea to prevent such
> things from running during system suspend/resume in general so they don't
> try to access devices at wrong times.
> 
> BTW, the freezing of filesystems during system suspend (not hibernation) makes
> sense too, because it will help to address the long-standing issue with storage
> devices that go away while the system is suspended.
> 
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem in
> question is properly frozen during suspend, the driver should be able to detect
> superblock changes during unfreeze.
> 
> So the approach I'd suggest would be to add the freezing of filesystems to the
> suspend/resume code paths just for the above reasons and drop the kthreads
> freezing from the filesystems that support the proper freezing.  The rest
> should be easier to deal with then.

That alone makes sense. It'll however leave a load of freezer users in the 
kernel that make no sense (one example picked completely out of the air: 
w1_process(); what is the reason for it there?) and are likely broken 
(completely random examples again: md, xfsaild -- they think they are 
freezable, but they are not).

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Oliver Neukum]

On Mon, 2015-11-02 at 11:45 +0100, Jiri Kosina wrote:
> > For example, if user space does a "read" or "write" on a character
> device
> > which is runtime-suspended at that point, the driver may want to
> resume the
> > device temporarily, carry out the operation and suspend it again,
> but that
> > generally won't work for the system suspend case.
> 
> But why would this even be relevant in this discussion, given that at
> the 
> point we are talking about, the whole userspace has been frozen
> already?

It really doesn't matter whether the thread in question is a kernel
thread or user space. Device detection is even worse.
Kernel threads that do either of these things must stop at
defined points. You can use the freezer or go to another mechanism.
I just doubt they'd act much different in the end.

	Regards
		Oliver

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Mon, 2 Nov 2015, Jiri Kosina wrote:

> On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > BTW, the freezing of filesystems during system suspend (not hibernation) makes
> > sense too, because it will help to address the long-standing issue with storage
> > devices that go away while the system is suspended.
> > 
> > I guess it may also helps to address the case when a device is removed from a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  Today
> > this is an almost guaranteed data corruption scenario, but if the filesystem in
> > question is properly frozen during suspend, the driver should be able to detect
> > superblock changes during unfreeze.

I agree completely; adding filesystem freezing is a great idea.

> > So the approach I'd suggest would be to add the freezing of filesystems to the
> > suspend/resume code paths just for the above reasons and drop the kthreads
> > freezing from the filesystems that support the proper freezing.  The rest
> > should be easier to deal with then.
> 
> That alone makes sense. It'll however leave a load of freezer users in the 
> kernel that make no sense (one example picked completely out of the air: 
> w1_process(); what is the reason for it there?) and are likely broken 
> (completely random examples again: md, xfsaild -- they think they are 
> freezable, but they are not).

If you fix just the kthreads which do this (along with an explanation
in the changelog of why the kthread is wrong), while leaving the others
alone, that ought to be acceptable.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Mon, 2 Nov 2015, Jiri Kosina wrote:

> On Mon, 2 Nov 2015, Rafael J. Wysocki wrote:

> > BTW, the freezing of filesystems during system suspend (not hibernation) makes
> > sense too, because it will help to address the long-standing issue with storage
> > devices that go away while the system is suspended.
> > 
> > I guess it may also helps to address the case when a device is removed from a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  Today
> > this is an almost guaranteed data corruption scenario, but if the filesystem in
> > question is properly frozen during suspend, the driver should be able to detect
> > superblock changes during unfreeze.

I agree completely; adding filesystem freezing is a great idea.

> > So the approach I'd suggest would be to add the freezing of filesystems to the
> > suspend/resume code paths just for the above reasons and drop the kthreads
> > freezing from the filesystems that support the proper freezing.  The rest
> > should be easier to deal with then.
> 
> That alone makes sense. It'll however leave a load of freezer users in the 
> kernel that make no sense (one example picked completely out of the air: 
> w1_process(); what is the reason for it there?) and are likely broken 
> (completely random examples again: md, xfsaild -- they think they are 
> freezable, but they are not).

If you fix just the kthreads which do this (along with an explanation
in the changelog of why the kthread is wrong), while leaving the others
alone, that ought to be acceptable.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Dave Chinner]

On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> I guess it may also helps to address the case when a device is removed from a
> suspended system, written to on another system in the meantime and inserted
> back into the (still suspended) original system which then is resumed.  Today
> this is an almost guaranteed data corruption scenario, but if the filesystem in
> question is properly frozen during suspend, the driver should be able to detect
> superblock changes during unfreeze.

Never going to work. There is no guarantee that a write to a
filesystem by a third party device is going to change the superblock
(or any metadata in the rest of the filesystem) in any detectable
way.  Hence freezing filesystems will not prevent Bad Things
Happening if you do this while your system is suspended.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

On Tuesday, November 03, 2015 11:10:53 AM Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  Today
> > this is an almost guaranteed data corruption scenario, but if the filesystem in
> > question is properly frozen during suspend, the driver should be able to detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

OK, thanks for the clarification.

Cheers,
Rafael

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Jan Kara]

On Tue 03-11-15 11:10:53, Dave Chinner wrote:
> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
> > I guess it may also helps to address the case when a device is removed from a
> > suspended system, written to on another system in the meantime and inserted
> > back into the (still suspended) original system which then is resumed.  Today
> > this is an almost guaranteed data corruption scenario, but if the filesystem in
> > question is properly frozen during suspend, the driver should be able to detect
> > superblock changes during unfreeze.
> 
> Never going to work. There is no guarantee that a write to a
> filesystem by a third party device is going to change the superblock
> (or any metadata in the rest of the filesystem) in any detectable
> way.  Hence freezing filesystems will not prevent Bad Things
> Happening if you do this while your system is suspended.

Agreed, we should never advertise something like this works. OTOH the truth
is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
of cases where user shot himself in the foot like this (i.e., ext4 driver
will update write time in superblock if it gets mounted somewhere else and
we can check whether that didn't change in ext4_unfreeze()) and refuse to
touch the filesystem... It is not 100% reliable since user could have used
e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
have to know what they are doing anyway.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Rafael J. Wysocki]

Hi,

On Tue, Nov 3, 2015 at 10:31 AM, Jan Kara <jack@suse.cz> wrote:
> On Tue 03-11-15 11:10:53, Dave Chinner wrote:
>> On Mon, Nov 02, 2015 at 03:43:07AM +0100, Rafael J. Wysocki wrote:
>> > I guess it may also helps to address the case when a device is removed from a
>> > suspended system, written to on another system in the meantime and inserted
>> > back into the (still suspended) original system which then is resumed.  Today
>> > this is an almost guaranteed data corruption scenario, but if the filesystem in
>> > question is properly frozen during suspend, the driver should be able to detect
>> > superblock changes during unfreeze.
>>
>> Never going to work. There is no guarantee that a write to a
>> filesystem by a third party device is going to change the superblock
>> (or any metadata in the rest of the filesystem) in any detectable
>> way.  Hence freezing filesystems will not prevent Bad Things
>> Happening if you do this while your system is suspended.
>
> Agreed, we should never advertise something like this works. OTOH the truth
> is that e.g. in ext4 case a simple check in ext4_unfreeze() could catch 90%
> of cases where user shot himself in the foot like this (i.e., ext4 driver
> will update write time in superblock if it gets mounted somewhere else and
> we can check whether that didn't change in ext4_unfreeze()) and refuse to
> touch the filesystem... It is not 100% reliable since user could have used
> e.g. debuge2fs to arbitrarily modify the filesystem but in such cases they
> have to know what they are doing anyway.

Well, my idea was to use this for sanity checking.  I guess that
sanity checks here don't really hurt, do they?  And if they fail,
perhaps we can just avoid touching the fs again for safety reasons?

Thanks,
Rafael

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> That's because we don't want async runtime PM to happen during system
> suspend/resume and for good reasons, so if you want to remove the freezing
> mechanism, you need to stop that workqueue at the beginning of dpm_prepare
> and start it again at the end of dpm_complete().

The same sort of thing is true for the USB hub driver's workqueue.  
Since it registers new devices (as a result of hotplugs), it must stop
running at the beginning of the dpm_prepare stage.  Of course, that's
also a workqueue and not a kthread.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Sat, 31 Oct 2015, Rafael J. Wysocki wrote:

> Runtime PM uses a freezable workqueue, allocated in pm_start_workqueue().
> 
> That's because we don't want async runtime PM to happen during system
> suspend/resume and for good reasons, so if you want to remove the freezing
> mechanism, you need to stop that workqueue at the beginning of dpm_prepare
> and start it again at the end of dpm_complete().

The same sort of thing is true for the USB hub driver's workqueue.  
Since it registers new devices (as a result of hotplugs), it must stop
running at the beginning of the dpm_prepare stage.  Of course, that's
also a workqueue and not a kthread.

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.
> > 
> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.

I think we are talking at cross purposes.  Your view of how a kthread 
works appears to be very different from mine.

Here's how I see it: A kthread performs some service, generally in a
big loop.  At certain points in that loop (perhaps only at the head),
the kthread will be in a suitable state for suspend: sufficiently
quiescent, no locks held, no I/O in progress, and so on.  At other
points, the kthread is not ready for suspend.

Therefore the fact that a PM callback tells you exactly when a supsend 
is about to occur is of no use.  The kthread can't act on that 
information directly, because most of the time it isn't ready for a 
suspend.  Only when it reaches one of its quiescent points will it be 
ready to do whatever is necessary -- which usually is nothing at all.

Given this picture, I don't see any alternative to a polling approach
of one kind or another.  At various quiescent points the kthread checks
to see if a suspend is imminent before moving forward.  At other times
the kthread can't handle suspend anyway, so it ignores the issue.  
This approach is exactly what try_to_freeze and friends support.

> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?

Out of all the kthreads you modified in your patch 2/3, the only one
I'm familiar with is the one in f_mass_storage.c (the USB mass-storage
gadget driver).  That's kind of a special case, and I don't mind you
ripping out all the freezer stuff from it because its approach was
almost completely arbitrary all along.  AFAIK, we have never settled on
the right way for a USB gadget to handle system sleep.

Other cases I don't know about.  My argument is based on the general
analysis given above.  But consider one point: You said "instead of
just waiting in schedule() until suspend-resume cycle is over".  What
if, at the time of the PM callback, the kthread happens to be holding a
mutex that some driver needs to acquire while suspending?  If the
kthread just hops into schedule() and waits there, the suspend will
fail or deadlock.  This example shows that the situation is more
complicated than you make it appear.

> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Don't kthreads have to poll to find out when they need to stop?  How is 
that different from (or better than) polling to see when they need to 
freeze?

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?

That was my point.  Since there's no difference, why did your patch 
talk about hibernation only, and not s2ram?

(Of course, some people like Len Brown have been arguing that for 
s2ram, the metadata in memory does _not_ need to be in sync with what's 
on disk.  I'm ignoring that for now, but such things would have to be 
taken into account when the final patch is written.)

Alan Stern

Re: [PATCH 0/3] PM, vfs: use filesystem freezing instead of kthread freezer

[Alan Stern]

On Fri, 30 Oct 2015, Jiri Kosina wrote:

> On Fri, 30 Oct 2015, Alan Stern wrote:
> 
> > > > > I would say instead "no I/O is allowed from now on".  Maybe that's an 
> > > > > overstatement, but I think it comes closer to the truth.
> > > 
> > > But that's what PM callbacks are for.
> > 
> > Why are PM callbacks any more suitable than the freezer?  
> 
> Once the PM callback triggers, you know that you are really actually 
> undergoing suspend and have to do whatever is necessary.
> 
> OTOH, try_to_freeze() is a kind of "are we there yet?" polling, and the 
> whole state needs to be prepared pro-actively for suspend already when you 
> call it, each and every time, even if you are not going through suspend at 
> all.
> 
> That's sub-optimal, and very easy to get wrong over gradual code changes.

I think we are talking at cross purposes.  Your view of how a kthread 
works appears to be very different from mine.

Here's how I see it: A kthread performs some service, generally in a
big loop.  At certain points in that loop (perhaps only at the head),
the kthread will be in a suitable state for suspend: sufficiently
quiescent, no locks held, no I/O in progress, and so on.  At other
points, the kthread is not ready for suspend.

Therefore the fact that a PM callback tells you exactly when a supsend 
is about to occur is of no use.  The kthread can't act on that 
information directly, because most of the time it isn't ready for a 
suspend.  Only when it reaches one of its quiescent points will it be 
ready to do whatever is necessary -- which usually is nothing at all.

Given this picture, I don't see any alternative to a polling approach
of one kind or another.  At various quiescent points the kthread checks
to see if a suspend is imminent before moving forward.  At other times
the kthread can't handle suspend anyway, so it ignores the issue.  
This approach is exactly what try_to_freeze and friends support.

> > The most natural implementation would be for the callback routine to set 
> > a flag; at various strategic points the kthread would check the flag and 
> > if it was set, call a routine that sits around and waits for the suspend 
> > to be over.  
> 
> Could you name at least some existing kthreads that would actually *need* 
> such complex handling, instead of just waiting in schedule() until 
> suspend-resume cycle is over, given that PM callbacks do all the necessary 
> cleanup (putting HW to sleep, cancelling timers, etc) anyway?

Out of all the kthreads you modified in your patch 2/3, the only one
I'm familiar with is the one in f_mass_storage.c (the USB mass-storage
gadget driver).  That's kind of a special case, and I don't mind you
ripping out all the freezer stuff from it because its approach was
almost completely arbitrary all along.  AFAIK, we have never settled on
the right way for a USB gadget to handle system sleep.

Other cases I don't know about.  My argument is based on the general
analysis given above.  But consider one point: You said "instead of
just waiting in schedule() until suspend-resume cycle is over".  What
if, at the time of the PM callback, the kthread happens to be holding a
mutex that some driver needs to acquire while suspending?  If the
kthread just hops into schedule() and waits there, the suspend will
fail or deadlock.  This example shows that the situation is more
complicated than you make it appear.

> PM callback can always explicitly do kthread_stop() on a particular 
> kthread if really necessary.

Don't kthreads have to poll to find out when they need to stop?  How is 
that different from (or better than) polling to see when they need to 
freeze?

> > Also, you never replied to my question about suspend vs. hibernation.
> 
> The main point of freezer is to reach quiescent state wrt. filesystems 
> (metadata in memory need to be absolutely in sync with what's on disk). 
> That's no different between hibernation and s2ram, is it?

That was my point.  Since there's no difference, why did your patch 
talk about hibernation only, and not s2ram?

(Of course, some people like Len Brown have been arguing that for 
s2ram, the metadata in memory does _not_ need to be in sync with what's 
on disk.  I'm ignoring that for now, but such things would have to be 
taken into account when the final patch is written.)

Alan Stern