Re: [PATCH 1/4] namespace: take lock_mount_hash() directly when changing flags
From: Jann Horn <jannh@google.com>
Date: 2020-07-14 16:50:11
Also in:
linux-fsdevel, lkml
On Tue, Jul 14, 2020 at 6:16 PM Christian Brauner [off-list ref] wrote:
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.
[...]
quoted hunk ↗ jump to hunk
diff --git a/fs/namespace.c b/fs/namespace.c
[...]
quoted hunk ↗ jump to hunk
@@ -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; }
It might be a good idea, instead of completely removing the locking calls here, to replace them with lockdep_assert_held(...). (Currently that doesn't appear much in core VFS code though.)