Message ID | alpine.LNX.2.00.1510301439080.17538@pobox.suse.cz (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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: