On Tue, May 02, 2023 at 08:02:18PM -0700, Darrick J. Wong wrote:
> diff --git a/fs/super.c b/fs/super.c
> index 04bc62ab7dfe..01891f9e6d5e 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1736,18 +1747,33 @@ int freeze_super(struct super_block *sb)
> up_write(&sb->s_umount);
> return 0;
> }
> +
> +/*
> + * freeze_super - lock the filesystem and force it into a consistent state
> + * @sb: the super to lock
> + *
> + * Syncs the super to make sure the filesystem is consistent and calls the fs's
> + * freeze_fs. Subsequent calls to this without first thawing the fs will return
> + * -EBUSY. See the comment for __freeze_super for more information.
> + */
> +int freeze_super(struct super_block *sb)
> +{
> + return __freeze_super(sb, USERSPACE_FREEZE_COOKIE);
> +}
> EXPORT_SYMBOL(freeze_super);
>
> -static int thaw_super_locked(struct super_block *sb)
> +static int thaw_super_locked(struct super_block *sb, unsigned long cookie)
> {
> int error;
>
> - if (sb->s_writers.frozen != SB_FREEZE_COMPLETE) {
> + if (sb->s_writers.frozen != SB_FREEZE_COMPLETE ||
> + sb->s_writers.freeze_cookie != cookie) {
> up_write(&sb->s_umount);
> return -EINVAL;
We get the same by just having drivers use freeze_super(sb, true) in the
patches I have, ie, we treat it a user-initiated.
On freeze() we have:
int freeze_super(struct super_block *sb, bool usercall)
{
int ret;
if(!usercall && sb_is_frozen(sb))
return 0;
if (!sb_is_unfrozen(sb))
return -EBUSY;
...
}
On thaw we end up with:
int thaw_super(struct super_block *sb, bool usercall)
{
int error;
if (!usercall) {
/*
* If userspace initiated the freeze don't let the kernel
* thaw it on return from a kernel initiated freeze.
*/
if (sb_is_unfrozen(sb) || sb_is_frozen_by_user(sb))
return 0;
}
if (!sb_is_frozen(sb))
return -EINVAL;
...
}
As I had it, I had made the drivers and the bdev freeze use the usercall as
true and so there is no change.
In case there is a filesystem already frozen then which was initiated by
the filesystem, for whatever reason, the filesystem the kernel auto-freeze
will chug on happy with the system freeze, it bails out withour error
and moves on to the next filesystem to freeze.
Upon thaw, the kernel auto-thaw will detect that the filesystem was
frozen by user on sb_is_frozen_by_user() and so will just bail and not
thaw it.
If the mechanism you want to introduce is to allow a filesystem to even
prevent kernel auto-freeze with -EBUSY it begs the question if that
shouldn't also prevent suspend. Because it would anyway as you have it
right now with your patch but it would return -EINVAL. I also ask because of
the possible issues with the filesystem not going to suspend but the backing
or other possible related devices going to suspend.
Since I think the goal is to prevent the kernel auto-freeze due to
online fsck to complete, then I think you *do* want to prevent full
system suspend from moving forward. In that case, why not just have
the filesystem check for that and return -EBUSY on its respective
filesystem sb->s_op->freeze_fs(sb) callback?
Luis
