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