On Fri, Aug 29, 2025 at 12:07:21AM +0100, Al Viro wrote:
> Prior to the call of do_new_mount_fc() the caller has just done successful
> vfs_get_tree(). Then do_new_mount_fc() does several checks on resulting
> superblock, and either does fc_drop_locked() and returns an error or
> proceeds to unlock the superblock and call vfs_create_mount().
>
> The thing is, there's no reason to delay that unlock + vfs_create_mount() -
> the tests do not rely upon the state of ->s_umount and
> fc_drop_locked()
> put_fs_context()
> is equivalent to
> unlock ->s_umount
> put_fs_context()
>
> Doing vfs_create_mount() before the checks allows us to move vfs_get_tree()
> from caller to do_new_mount_fc() and collapse it with vfs_create_mount()
> into an fc_mount() call.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx>
> fs/namespace.c | 29 ++++++++++++-----------------
> 1 file changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 0474b3a93dbf..9b575c9eee0b 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -3705,25 +3705,20 @@ static bool mount_too_revealing(const struct super_block *sb, int *new_mnt_flags
> static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
> unsigned int mnt_flags)
> {
> - struct vfsmount *mnt;
> struct pinned_mountpoint mp = {};
> - struct super_block *sb = fc->root->d_sb;
> + struct super_block *sb;
> + struct vfsmount *mnt = fc_mount(fc);
> int error;
>
> + if (IS_ERR(mnt))
> + return PTR_ERR(mnt);
Fwiw, I find this pattern where the variable is assigned by function
call at declaration time in the middle of other variables and then
immediately further below check for the error to be rather ugly. I'd
much rather just do:
+ struct vfsmount *mnt;
int error;
mnt = fc_mount(fc)
+ if (IS_ERR(mnt))
+ return PTR_ERR(mnt);
But anyway, I acknowledge the difference in taste here is really not
that important.
