Re: [PATCH v2 3/4] power: freeze filesystems during suspend/resume

From: Jan Kara
Date: Thu Apr 03 2025 - 12:29:23 EST

Next message: Hans Zhang: "Re: [v7 2/5] PCI: Refactor capability search functions to eliminate code duplication"
Previous message: Olivier Moysan: "[PATCH v2] iio: adc: stm32: add oversampling support"
In reply to: Christian Brauner: "[PATCH v2 3/4] power: freeze filesystems during suspend/resume"
Next in thread: Peter Zijlstra: "Re: [PATCH 0/6] power: wire-up filesystem freeze/thaw with suspend/resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed 02-04-25 16:07:33, Christian Brauner wrote:
> Now all the pieces are in place to actually allow the power subsystem
> to freeze/thaw filesystems during suspend/resume. Filesystems are only
> frozen and thawed if the power subsystem does actually own the freeze.
>
> We could bubble up errors and fail suspend/resume if the error isn't
> EBUSY (aka it's already frozen) but I don't think that this is worth it.
> Filesystem freezing during suspend/resume is best-effort. If the user
> has 500 ext4 filesystems mounted and 4 fail to freeze for whatever
> reason then we simply skip them.
>
> What we have now is already a big improvement and let's see how we fare
> with it before making our lives even harder (and uglier) than we have
> to.
>
> We add a new sysctl know /sys/power/freeze_filesystems that will allow
> userspace to freeze filesystems during suspend/hibernate. For now it
> defaults to off. The thaw logic doesn't require checking whether
> freezing is enabled because the power subsystem exclusively owns frozen
> filesystems for the duration of suspend/hibernate and is able to skip
> filesystems it doesn't need to freeze.
>
> Also it is technically possible that filesystem
> filesystem_freeze_enabled is true and power freezes the filesystems but
> before freezing all processes another process disables
> filesystem_freeze_enabled. If power were to place the filesystems_thaw()
> call under filesystems_freeze_enabled it would fail to thaw the
> fileystems it frozw. The exclusive holder mechanism makes it possible to
> iterate through the list without any concern making sure that no
> filesystems are left frozen.
>
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>

Looks good modulo the nesting issue I've mentioned in my comments to patch
1. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

Honza

> ---
> fs/super.c | 14 ++++++++++----
> kernel/power/hibernate.c | 16 +++++++++++++++-
> kernel/power/main.c | 31 +++++++++++++++++++++++++++++++
> kernel/power/power.h | 4 ++++
> kernel/power/suspend.c | 7 +++++++
> 5 files changed, 67 insertions(+), 5 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 3ddded4360c6..b4bdbc509dba 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1187,6 +1187,8 @@ static inline bool get_active_super(struct super_block *sb)
> return active;
> }
>
> +static const char *filesystems_freeze_ptr = "filesystems_freeze";
> +
> static void filesystems_freeze_callback(struct super_block *sb, void *unused)
> {
> if (!sb->s_op->freeze_fs && !sb->s_op->freeze_super)
> @@ -1196,9 +1198,11 @@ static void filesystems_freeze_callback(struct super_block *sb, void *unused)
> return;
>
> if (sb->s_op->freeze_super)
> - sb->s_op->freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL, NULL);
> + sb->s_op->freeze_super(sb, FREEZE_EXCL | FREEZE_HOLDER_KERNEL,
> + filesystems_freeze_ptr);
> else
> - freeze_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL, NULL);
> + freeze_super(sb, FREEZE_EXCL | FREEZE_HOLDER_KERNEL,
> + filesystems_freeze_ptr);
>
> deactivate_super(sb);
> }
> @@ -1218,9 +1222,11 @@ static void filesystems_thaw_callback(struct super_block *sb, void *unused)
> return;
>
> if (sb->s_op->thaw_super)
> - sb->s_op->thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL, NULL);
> + sb->s_op->thaw_super(sb, FREEZE_EXCL | FREEZE_HOLDER_KERNEL,
> + filesystems_freeze_ptr);
> else
> - thaw_super(sb, FREEZE_MAY_NEST | FREEZE_HOLDER_KERNEL, NULL);
> + thaw_super(sb, FREEZE_EXCL | FREEZE_HOLDER_KERNEL,
> + filesystems_freeze_ptr);
>
> deactivate_super(sb);
> }
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 50ec26ea696b..37d733945c59 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -777,6 +777,8 @@ int hibernate(void)
> goto Restore;
>
> ksys_sync_helper();
> + if (filesystem_freeze_enabled)
> + filesystems_freeze();
>
> error = freeze_processes();
> if (error)
> @@ -845,6 +847,7 @@ int hibernate(void)
> /* Don't bother checking whether freezer_test_done is true */
> freezer_test_done = false;
> Exit:
> + filesystems_thaw();
> pm_notifier_call_chain(PM_POST_HIBERNATION);
> Restore:
> pm_restore_console();
> @@ -881,6 +884,9 @@ int hibernate_quiet_exec(int (*func)(void *data), void *data)
> if (error)
> goto restore;
>
> + if (filesystem_freeze_enabled)
> + filesystems_freeze();
> +
> error = freeze_processes();
> if (error)
> goto exit;
> @@ -940,6 +946,7 @@ int hibernate_quiet_exec(int (*func)(void *data), void *data)
> thaw_processes();
>
> exit:
> + filesystems_thaw();
> pm_notifier_call_chain(PM_POST_HIBERNATION);
>
> restore:
> @@ -1028,19 +1035,26 @@ static int software_resume(void)
> if (error)
> goto Restore;
>
> + if (filesystem_freeze_enabled)
> + filesystems_freeze();
> +
> pm_pr_dbg("Preparing processes for hibernation restore.\n");
> error = freeze_processes();
> - if (error)
> + if (error) {
> + filesystems_thaw();
> goto Close_Finish;
> + }
>
> error = freeze_kernel_threads();
> if (error) {
> thaw_processes();
> + filesystems_thaw();
> goto Close_Finish;
> }
>
> error = load_image_and_restore();
> thaw_processes();
> + filesystems_thaw();
> Finish:
> pm_notifier_call_chain(PM_POST_RESTORE);
> Restore:
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 6254814d4817..0b0e76324c43 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -962,6 +962,34 @@ power_attr(pm_freeze_timeout);
>
> #endif /* CONFIG_FREEZER*/
>
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATION)
> +bool filesystem_freeze_enabled = false;
> +
> +static ssize_t freeze_filesystems_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + return sysfs_emit(buf, "%d\n", filesystem_freeze_enabled);
> +}
> +
> +static ssize_t freeze_filesystems_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t n)
> +{
> + unsigned long val;
> +
> + if (kstrtoul(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val > 1)
> + return -EINVAL;
> +
> + filesystem_freeze_enabled = !!val;
> + return n;
> +}
> +
> +power_attr(freeze_filesystems);
> +#endif /* CONFIG_SUSPEND || CONFIG_HIBERNATION */
> +
> static struct attribute * g[] = {
> &state_attr.attr,
> #ifdef CONFIG_PM_TRACE
> @@ -991,6 +1019,9 @@ static struct attribute * g[] = {
> #endif
> #ifdef CONFIG_FREEZER
> &pm_freeze_timeout_attr.attr,
> +#endif
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATION)
> + &freeze_filesystems_attr.attr,
> #endif
> NULL,
> };
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index c352dea2f67b..2eb81662b8fa 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -18,6 +18,10 @@ struct swsusp_info {
> unsigned long size;
> } __aligned(PAGE_SIZE);
>
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATION)
> +extern bool filesystem_freeze_enabled;
> +#endif
> +
> #ifdef CONFIG_HIBERNATION
> /* kernel/power/snapshot.c */
> extern void __init hibernate_reserved_size_init(void);
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 8eaec4ab121d..76b141b9aac0 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -30,6 +30,7 @@
> #include <trace/events/power.h>
> #include <linux/compiler.h>
> #include <linux/moduleparam.h>
> +#include <linux/fs.h>
>
> #include "power.h"
>
> @@ -374,6 +375,8 @@ static int suspend_prepare(suspend_state_t state)
> if (error)
> goto Restore;
>
> + if (filesystem_freeze_enabled)
> + filesystems_freeze();
> trace_suspend_resume(TPS("freeze_processes"), 0, true);
> error = suspend_freeze_processes();
> trace_suspend_resume(TPS("freeze_processes"), 0, false);
> @@ -550,6 +553,7 @@ int suspend_devices_and_enter(suspend_state_t state)
> static void suspend_finish(void)
> {
> suspend_thaw_processes();
> + filesystems_thaw();
> pm_notifier_call_chain(PM_POST_SUSPEND);
> pm_restore_console();
> }
> @@ -588,6 +592,8 @@ static int enter_state(suspend_state_t state)
> ksys_sync_helper();
> trace_suspend_resume(TPS("sync_filesystems"), 0, false);
> }
> + if (filesystem_freeze_enabled)
> + filesystems_freeze();
>
> pm_pr_dbg("Preparing system for sleep (%s)\n", mem_sleep_labels[state]);
> pm_suspend_clear_flags();
> @@ -609,6 +615,7 @@ static int enter_state(suspend_state_t state)
> pm_pr_dbg("Finishing wakeup.\n");
> suspend_finish();
> Unlock:
> + filesystems_thaw();
> mutex_unlock(&system_transition_mutex);
> return error;
> }
>
> --
> 2.47.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR

Next message: Hans Zhang: "Re: [v7 2/5] PCI: Refactor capability search functions to eliminate code duplication"
Previous message: Olivier Moysan: "[PATCH v2] iio: adc: stm32: add oversampling support"
In reply to: Christian Brauner: "[PATCH v2 3/4] power: freeze filesystems during suspend/resume"
Next in thread: Peter Zijlstra: "Re: [PATCH 0/6] power: wire-up filesystem freeze/thaw with suspend/resume"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]