Kernel/linux
1
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2025-01-15 01:24:33 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

namespace: only take read lock in do_reconfigure_mnt()

Browse Source 
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 43f5e655eff7 ("vfs: Separate changing mount flags full remount")
[2]: commit 2e4b7fcd9260 ("[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: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
This commit is contained in:
Christian Brauner 2021-01-21 14:19:50 +01:00
parent d033cb6784
commit e58ace1a0f
No known key found for this signature in database
GPG Key ID: 91C61BC06578DCA2
1 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
fs/namespace.c
View File

@ -2527,10 +2527,6 @@ static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags)
return 0; return 0;
} }
/*
* Update the user-settable attributes on a mount. The caller must hold
* sb->s_umount for writing.
*/
static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags) static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags)
{ {
mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK; mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK;
@ -2580,13 +2576,17 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags)
if (!can_change_locked_flags(mnt, mnt_flags)) if (!can_change_locked_flags(mnt, mnt_flags))
return -EPERM; return -EPERM;
down_write(&sb->s_umount); /*
* We're only checking whether the superblock is read-only not
* changing it, so only take down_read(&sb->s_umount).
*/
down_read(&sb->s_umount);
lock_mount_hash(); lock_mount_hash();
ret = change_mount_ro_state(mnt, mnt_flags); ret = change_mount_ro_state(mnt, mnt_flags);
if (ret == 0) if (ret == 0)
set_mount_attributes(mnt, mnt_flags); set_mount_attributes(mnt, mnt_flags);
unlock_mount_hash(); unlock_mount_hash();
up_write(&sb->s_umount); up_read(&sb->s_umount);
mnt_warn_timestamp_expiry(path, &mnt->mnt); mnt_warn_timestamp_expiry(path, &mnt->mnt);