From patchwork Fri Oct 30 13:47:16 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiri Kosina X-Patchwork-Id: 7526971 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 7405F9F2F7 for ; Fri, 30 Oct 2015 13:47:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 327E9206E8 for ; Fri, 30 Oct 2015 13:47:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BB367206E4 for ; Fri, 30 Oct 2015 13:47:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1031182AbbJ3NrU (ORCPT ); Fri, 30 Oct 2015 09:47:20 -0400 Received: from mx2.suse.de ([195.135.220.15]:36577 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1031164AbbJ3NrR (ORCPT ); Fri, 30 Oct 2015 09:47:17 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 4E70FAC27; Fri, 30 Oct 2015 13:47:37 +0000 (UTC) Date: Fri, 30 Oct 2015 14:47:16 +0100 (CET) From: Jiri Kosina X-X-Sender: jkosina@pobox.suse.cz To: "Rafael J. Wysocki" , Dave Chinner , Jan Kara , Christoph Hellwig , Linus Torvalds , Al Viro , Tejun Heo , Pavel Machek cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-pm@vger.kernel.org Subject: [PATCH 1/3] power, vfs: move away from PF_KTHREAD freezing in favor of fs freezing In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Jiri Kosina 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 --- 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 #include #include +#include #include /* @@ -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: