Linux-Fsdevel Archive on lore.kernel.org help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com> To: David Howells <dhowells@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Michael Kerrisk <mtk.manpages@gmail.com>, Christian Brauner <christian.brauner@ubuntu.com> Subject: [PATCH 1/4] namespace: take lock_mount_hash() directly when changing flags Date: Tue, 14 Jul 2020 18:14:13 +0200 [thread overview] Message-ID: <20200714161415.3886463-3-christian.brauner@ubuntu.com> (raw) In-Reply-To: <20200714161415.3886463-1-christian.brauner@ubuntu.com> Changing mount options always ends up taking lock_mount_hash() but when MNT_READONLY is requested and neither the mount nor the superblock are not already MNT_READONLY we end up taking the lock, dropping it, and retaking it to change the other mount attributes. Instead of this, acquire the lock once when changing mount properties. This simplifies the locking in these codepath, makes them easier to reason about and avoids having to reacquire the lock right after dropping it. Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: David Howells <dhowells@redhat.com> Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- fs/namespace.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/namespace.c b/fs/namespace.c index f30ed401cc6d..395b9c912edf 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -463,7 +463,6 @@ static int mnt_make_readonly(struct mount *mnt) { int ret = 0; - lock_mount_hash(); mnt->mnt.mnt_flags |= MNT_WRITE_HOLD; /* * After storing MNT_WRITE_HOLD, we'll read the counters. This store @@ -497,15 +496,12 @@ static int mnt_make_readonly(struct mount *mnt) */ smp_wmb(); mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD; - unlock_mount_hash(); return ret; } static int __mnt_unmake_readonly(struct mount *mnt) { - lock_mount_hash(); mnt->mnt.mnt_flags &= ~MNT_READONLY; - unlock_mount_hash(); return 0; } @@ -2517,11 +2513,9 @@ static int change_mount_ro_state(struct mount *mnt, unsigned int mnt_flags) */ static void set_mount_attributes(struct mount *mnt, unsigned int mnt_flags) { - lock_mount_hash(); mnt_flags |= mnt->mnt.mnt_flags & ~MNT_USER_SETTABLE_MASK; mnt->mnt.mnt_flags = mnt_flags; touch_mnt_namespace(mnt->mnt_ns); - unlock_mount_hash(); } static void mnt_warn_timestamp_expiry(struct path *mountpoint, struct vfsmount *mnt) @@ -2567,9 +2561,11 @@ static int do_reconfigure_mnt(struct path *path, unsigned int mnt_flags) return -EPERM; down_write(&sb->s_umount); + lock_mount_hash(); ret = change_mount_ro_state(mnt, mnt_flags); if (ret == 0) set_mount_attributes(mnt, mnt_flags); + unlock_mount_hash(); up_write(&sb->s_umount); mnt_warn_timestamp_expiry(path, &mnt->mnt); @@ -2609,8 +2605,11 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags, err = -EPERM; if (ns_capable(sb->s_user_ns, CAP_SYS_ADMIN)) { err = reconfigure_super(fc); - if (!err) + if (!err) { + lock_mount_hash(); set_mount_attributes(mnt, mnt_flags); + unlock_mount_hash(); + } } up_write(&sb->s_umount); } -- 2.27.0
next prev parent reply other threads:[~2020-07-14 16:15 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-14 16:14 [PATCH 0/4] fs: add mount_setattr() Christian Brauner 2020-07-14 16:14 ` [PATCH] mount_setattr.2: New manual page documenting the mount_setattr() system call Christian Brauner 2020-07-21 9:59 ` Michael Kerrisk (man-pages) 2020-07-14 16:14 ` Christian Brauner [this message] 2020-07-14 16:49 ` [PATCH 1/4] namespace: take lock_mount_hash() directly when changing flags Jann Horn 2020-07-14 16:14 ` [PATCH 2/4] namespace: only take read lock in do_reconfigure_mnt() Christian Brauner 2020-07-14 16:14 ` [PATCH 3/4] fs: add mount_setattr() Christian Brauner 2020-07-15 8:29 ` Geert Uytterhoeven 2020-07-15 11:02 ` Christian Brauner 2020-07-14 16:14 ` [PATCH 4/4] tests: add mount_setattr() selftests Christian Brauner 2020-07-19 17:10 ` [PATCH 0/4] fs: add mount_setattr() Al Viro 2020-07-19 17:55 ` Christian Brauner 2020-10-01 14:17 ` Pavel Tikhomirov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200714161415.3886463-3-christian.brauner@ubuntu.com \ --to=christian.brauner@ubuntu.com \ --cc=dhowells@redhat.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mtk.manpages@gmail.com \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).