On Mon, May 05, 2025 at 04:03:45AM +0100, Al Viro wrote:
> it's simpler to do btrfs_reconfigure_for_mount() right after vfs_get_tree() -
> no need to mess with ->s_umount.
>
> Objections?
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 7121d8c7a318..a3634e7f2304 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1984,17 +1984,13 @@ static int btrfs_get_tree_super(struct fs_context *fc)
> * 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;
> }
>
> @@ -2047,17 +2043,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 = vfs_get_tree(dup_fc);
So this open codes fc_mount(), which is vfs_get_tree() + vfs_create_mount(),
the only difference I see in the new code is that
btrfs_reconfigure_for_mount() dropped the SB_RDONLY check.
Why the check is there is explained in the lengthy comment above
btrfs_reconfigure_for_mount(), so it should stay. If it can be removed
then it should be a separate patch from the cleanup.
> + if (!ret) {
> + ret = btrfs_reconfigure_for_mount(dup_fc);
> + up_write(&fc->root->d_sb->s_umount);
> }
> - ret = btrfs_reconfigure_for_mount(dup_fc, mnt);
> + if (!ret)
> + mnt = vfs_create_mount(fc);
> + else
> + mnt = ERR_PTR(ret);
> 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
