Re: [RFC][PATCH] btrfs_get_tree_subvol(): switch from fc_mount() to vfs_create_mount()

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, May 05, 2025 at 07:58:07PM +0200, David Sterba wrote:

> > -	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);

> 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.

What do you mean, dropped?  It's still right there - the current
variant checks it *twice*, once before grabbing ->s_umount, then
after it's been grabbed.  Checking it before down_write() makes sense
if we are called after ->s_umount had been dropped (by fc_mount()).
I'm not sure why you recheck it after down_write(), since it's not
going to change, but you do recheck it.  In this variant we don't need
to bother grabbing the rwsem, since that thing is called while ->s_umount
is still held...

I can turn that into
	if (fc->sb_flags & SB_RDONLY)
		return ret;
	if (fc->root->d_sb->s_flags & SB_RDONLY)
		ret = btrfs_reconfigure(fc);
	return ret;
but I don't see how it's better than the variant posted; up to you, of course...