[PATCH] vfs: avoid sb->s_umount write lock while changing bind-mount flags

From: Aditya Kali
Date: Fri Oct 25 2013 - 17:01:54 EST


During remount of a bind mount (mount -o remount,bind,ro,... /mnt/mntpt),
we currently take down_write(&sb->s_umount). This causes the remount
operation to get blocked behind writes occuring on device (possibly
mounted somewhere else). We have observed that simply trying to change
the bind-mount from read-write to read-only can take several seconds
becuase writeback is in progress (which takes read-lock on sb->s_umount).
Since we are not modifying the superblock during MS_BIND remount, it
should be enough to take a read-lock on sb->s_umount instead of the
write-lock to protect against parallel unmounts and (in conjunction with
vfsmount_lock) parallel remounts.
This patch fixes the locking so that the MS_BIND remount changes happen
only under down_read(&sb->s_umount).

Changes from RFC:
Instead of completely removing the locking in MS_BIND case, take
read-lock on sb->s_umount to protect against parallel unmount.

Signed-off-by: Aditya Kali <adityakali@xxxxxxxxxx>
---
fs/namespace.c | 42 ++++++++++++++++++++++++++++--------------
1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index da5c494..cf74920 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -454,11 +454,13 @@ void mnt_drop_write_file(struct file *file)
}
EXPORT_SYMBOL(mnt_drop_write_file);

+/*
+ * Must be called under br_write_lock(&vfsmount_lock).
+ */
static int mnt_make_readonly(struct mount *mnt)
{
int ret = 0;

- br_write_lock(&vfsmount_lock);
mnt->mnt.mnt_flags |= MNT_WRITE_HOLD;
/*
* After storing MNT_WRITE_HOLD, we'll read the counters. This store
@@ -492,15 +494,15 @@ static int mnt_make_readonly(struct mount *mnt)
*/
smp_wmb();
mnt->mnt.mnt_flags &= ~MNT_WRITE_HOLD;
- br_write_unlock(&vfsmount_lock);
return ret;
}

+/*
+ * Must be called under br_write_lock(&vfsmount_lock).
+ */
static void __mnt_unmake_readonly(struct mount *mnt)
{
- br_write_lock(&vfsmount_lock);
mnt->mnt.mnt_flags &= ~MNT_READONLY;
- br_write_unlock(&vfsmount_lock);
}

int sb_prepare_remount_readonly(struct super_block *sb)
@@ -1796,6 +1798,9 @@ out:
return err;
}

+/*
+ * Must be called under br_write_lock(&vfsmount_lock).
+ */
static int change_mount_flags(struct vfsmount *mnt, int ms_flags)
{
int error = 0;
@@ -1838,20 +1843,29 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
if (err)
return err;

- down_write(&sb->s_umount);
- if (flags & MS_BIND)
+ if (flags & MS_BIND) {
+ down_read(&sb->s_umount);
+ br_write_lock(&vfsmount_lock);
err = change_mount_flags(path->mnt, flags);
- else if (!capable(CAP_SYS_ADMIN))
+ if (!err) {
+ mnt_flags |= mnt->mnt.mnt_flags & MNT_PROPAGATION_MASK;
+ mnt->mnt.mnt_flags = mnt_flags;
+ }
+ br_write_unlock(&vfsmount_lock);
+ up_read(&sb->s_umount);
+ } else if (!capable(CAP_SYS_ADMIN))
err = -EPERM;
- else
+ else {
+ down_write(&sb->s_umount);
err = do_remount_sb(sb, flags, data, 0);
- if (!err) {
- br_write_lock(&vfsmount_lock);
- mnt_flags |= mnt->mnt.mnt_flags & MNT_PROPAGATION_MASK;
- mnt->mnt.mnt_flags = mnt_flags;
- br_write_unlock(&vfsmount_lock);
+ if (!err) {
+ br_write_lock(&vfsmount_lock);
+ mnt_flags |= mnt->mnt.mnt_flags & MNT_PROPAGATION_MASK;
+ mnt->mnt.mnt_flags = mnt_flags;
+ br_write_unlock(&vfsmount_lock);
+ }
+ up_write(&sb->s_umount);
}
- up_write(&sb->s_umount);
if (!err) {
br_write_lock(&vfsmount_lock);
touch_mnt_namespace(mnt->mnt_ns);
--
1.8.4.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/