From 75764b41bfc3a463b8a5f240e93f6342510dc944 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Tue, 6 May 2025 20:58:26 +0100 Subject: [PATCH] btrfs: open code fc_mount() to avoid releasing s_umount rw_sempahore [CURRENT BEHAVIOR] Currently inside btrfs_get_tree_subvol(), we call fc_mount() to grab a tree, then re-lock s_umount inside btrfs_reconfigure_for_mount() to avoid race with remount. However fc_mount() itself is just doing two things: 1. Call vfs_get_tree() 2. Release s_umount then call vfs_create_mount() [ENHANCEMENT] Instead of calling fc_mount(), we can open-code it with vfs_get_tree() first. This provides a benefit that, since we have the full control of s_umount, we do not need to re-lock that rw_sempahore when calling btrfs_reconfigure_for_mount(), meaning less race between RO/RW remount. Signed-off-by: Al Viro Reviewed-by: Qu Wenruo [ Rework the subject and commit message, refactor the error handling ] Signed-off-by: Qu Wenruo Tested-by: Qu Wenruo Signed-off-by: David Sterba --- fs/btrfs/super.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index a8dfbb84c15c..2d0d8c6e77b4 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -1996,17 +1996,13 @@ error: * btrfs or not, setting the whole super block RO. To make per-subvolume mounting * work with different options work we need to keep backward compatibility. */ -static int btrfs_reconfigure_for_mount(struct fs_context *fc, struct vfsmount *mnt) +static int btrfs_reconfigure_for_mount(struct fs_context *fc) { int ret = 0; - if (fc->sb_flags & SB_RDONLY) - return ret; - - down_write(&mnt->mnt_sb->s_umount); - if (!(fc->sb_flags & SB_RDONLY) && (mnt->mnt_sb->s_flags & SB_RDONLY)) + if (!(fc->sb_flags & SB_RDONLY) && (fc->root->d_sb->s_flags & SB_RDONLY)) ret = btrfs_reconfigure(fc); - up_write(&mnt->mnt_sb->s_umount); + return ret; } @@ -2059,17 +2055,18 @@ static int btrfs_get_tree_subvol(struct fs_context *fc) security_free_mnt_opts(&fc->security); fc->security = NULL; - mnt = fc_mount(dup_fc); - if (IS_ERR(mnt)) { - put_fs_context(dup_fc); - return PTR_ERR(mnt); - } - ret = btrfs_reconfigure_for_mount(dup_fc, mnt); + ret = vfs_get_tree(dup_fc); + if (ret) + goto error; + + ret = btrfs_reconfigure_for_mount(dup_fc); + up_write(&dup_fc->root->d_sb->s_umount); + if (ret) + goto error; + mnt = vfs_create_mount(dup_fc); put_fs_context(dup_fc); - if (ret) { - mntput(mnt); - return ret; - } + if (IS_ERR(mnt)) + return PTR_ERR(mnt); /* * This free's ->subvol_name, because if it isn't set we have to @@ -2083,6 +2080,9 @@ static int btrfs_get_tree_subvol(struct fs_context *fc) fc->root = dentry; return 0; +error: + put_fs_context(dup_fc); + return ret; } static int btrfs_get_tree(struct fs_context *fc) -- 2.25.1