| Message ID | 20250402-work-freeze-v2-3-6719a97b52ac@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | power: wire-up filesystem freeze/thaw with suspend/resume | expand |
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@kernel.org> Looks good modulo the nesting issue I've mentioned in my comments to patch 1. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> 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 >
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; }
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@kernel.org> --- 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(-)