On Wed, 2025-03-26 at 04:22 -0700, Luis Chamberlain wrote:
> Add support to automatically handle freezing and thawing filesystems
> during the kernel's suspend/resume cycle.
>
> This is needed so that we properly really stop IO in flight without
> races after userspace has been frozen. Without this we rely on
> kthread freezing and its semantics are loose and error prone.
> For instance, even though a kthread may use try_to_freeze() and end
> up being frozen we have no way of being sure that everything that
> has been spawned asynchronously from it (such as timers) have also
> been stopped as well.
>
> A long term advantage of also adding filesystem freeze / thawing
> supporting during suspend / hibernation is that long term we may
> be able to eventually drop the kernel's thread freezing completely
> as it was originally added to stop disk IO in flight as we hibernate
> or suspend.
>
> This does not remove the superfluous freezer calls on all
> filesystems.
> Each filesystem must remove all the kthread freezer stuff and peg
> the fs_type flags as supporting auto-freezing with the FS_AUTOFREEZE
> flag.
>
> Subsequent patches remove the kthread freezer usage from each
> filesystem, one at a time to make all this work bisectable.
> Once all filesystems remove the usage of the kthread freezer we
> can remove the FS_AUTOFREEZE flag.
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
> ---
> fs/super.c | 50
> ++++++++++++++++++++++++++++++++++++++++++
> include/linux/fs.h | 14 ++++++++++++
> kernel/power/process.c | 15 ++++++++++++-
> 3 files changed, 78 insertions(+), 1 deletion(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 9995546cf159..7428f0b2251c 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -2279,3 +2279,53 @@ int sb_init_dio_done_wq(struct super_block
> *sb)
> return 0;
> }
> EXPORT_SYMBOL_GPL(sb_init_dio_done_wq);
> +
> +#ifdef CONFIG_PM_SLEEP
> +static bool super_should_freeze(struct super_block *sb)
> +{
> + if (!(sb->s_type->fs_flags & FS_AUTOFREEZE))
> + return false;
> + /*
> + * We don't freeze virtual filesystems, we skip those
> filesystems with
> + * no backing device.
> + */
> + if (sb->s_bdi == &noop_backing_dev_info)
> + return false;
This logic won't work for me because efivarfs is a pseudofilesystem and
will have a noop bdi (or simply a null s_bdev, which is easier to check
for). I was thinking of allowing freeze/thaw to continue for a s_bdev
== NULL filesystem if it provided a freeze or thaw callback, which will
cover efivarfs.
> +
> + return true;
> +}
> +
> +int fs_suspend_freeze_sb(struct super_block *sb, void *priv)
> +{
> + int error = 0;
> +
> + if (!super_should_freeze(sb))
> + goto out;
> +
> + pr_info("%s (%s): freezing\n", sb->s_type->name, sb->s_id);
> +
> + error = freeze_super(sb, false);
This is actually not wholly correct now. If the fs provides a sb-
>freeze() method, you should use that instead of freeze_super() ... see
how fs_bdev_freeze() is doing it.
Additionally, the first thing freeze_super() does is take the
superblock lock exclusively. Since you've already taken it exclusively
in your iterate super, how does this not deadlock?
You also need to handle the hibernate deadlock I ran into where a
process (and some of the systemd processes are very fast at doing this)
touches the filesystem and gets blocked on uninterruptible wait before
the remainder of freeze_processes() runs. Once a task is
uninterruptible hibernate fails. I came up with a simplistic solution:
https://lore.kernel.org/linux-fsdevel/1af829aa7a65eb5ebc0614a00f7019615ed0f62b.camel@xxxxxxxxxxxxxxxxxxxxx/
But there should probably be a freezable percpu_rwsem that
sb_write_started() can use to get these semantics rather than making
every use of percpu_rwsem freezable.
Regards,
James
