在 2023/5/7 13:23, Luis Chamberlain 写道:
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.
Hi, Luis
Thanks for the great idea. I also need this upgraded API for a unbind
mechanism on pmem device, which is finally called in
xfs_notify_failure.c where we want to freeze the fs to prevent any other
new file mappings from being created. In my case, I think we should
think it as a kernel-initiated freeze, and hope it won't be thaw by
others, especially userspace-initiated thaw.
In my understanding of your implementation, if there is a
userspace-initiated thaw, with @usercall is set true, thaw_super(sb,
true) will ignore any others' freeze and thaw the fs anyway. But,
except in my case, I think the order of userspace-initiated freeze/thaw
may be messed up due to bugs in the user app, then the kernel-initiated
freeze state could be accidentally broken... In my opinion, the kernel
code is more reliable. Therefore, kernel-initiated freeze should be
exclusive at least.
--
Thanks,
Ruan.
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
