• Christian Brauner's avatar
    namespace: only take read lock in do_reconfigure_mnt() · e58ace1a
    Christian Brauner authored
    do_reconfigure_mnt() used to take the down_write(&sb->s_umount) lock
    which seems unnecessary since we're not changing the superblock. We're
    only checking whether it is already read-only. Setting other mount
    attributes is protected by lock_mount_hash() afaict and not by s_umount.
    
    The history of down_write(&sb->s_umount) lock being taken when setting
    mount attributes dates back to the introduction of MNT_READONLY in [2].
    This introduced the concept of having read-only mounts in contrast to
    just having a read-only superblock. When it got introduced it was simply
    plumbed into do_remount() which already took down_write(&sb->s_umount)
    because it was only used to actually change the superblock before [2].
    Afaict, it would've already been possible back then to only use
    down_read(&sb->s_umount) for MS_BIND | MS_REMOUNT since actual mount
    options were protected by the vfsmount lock already. But that would've
    meant special casing the locking for MS_BIND | MS_REMOUNT in
    do_remount() which people might not have considered worth it.
    Then in [1] MS_BIND | MS_REMOUNT mount option changes were split out of
    do_remount() into do_reconfigure_mnt() but the down_write(&sb->s_umount)
    lock was simply copied over.
    Now that we have this be a separate helper only take the
    down_read(&sb->s_umount) lock since we're only interested in checking
    whether the super block is currently read-only and blocking any writers
    from changing it. Essentially, checking that the super block is
    read-only has the advantage that we can avoid having to go into the
    slowpath and through MNT_WRITE_HOLD and can simply set the read-only
    flag on the mount in set_mount_attributes().
    
    [1]: commit 43f5e655 ("vfs: Separate changing mount flags full remount")
    [2]: commit 2e4b7fcd ("[PATCH] r/o bind mounts: honor mount writer counts at remount")
    
    Link: https://lore.kernel.org/r/20210121131959.646623-32-christian.brauner@ubuntu.com
    Cc: David Howells <dhowells@redhat.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: linux-fsdevel@vger.kernel.org
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarChristian Brauner <christian.brauner@ubuntu.com>
    e58ace1a
namespace.c 98.9 KB