[PATCH] VFS/autofs: try_lookup_one_len() does not need any locks

[PATCH] VFS/autofs: try_lookup_one_len() does not need any locks

[NeilBrown]

try_lookup_one_len() is identical to lookup_one_unlocked() except that
it doesn't include the call to lookup_slow().  The latter doesn't need
the inode to be locked, so the former cannot either.

So fix the documentation, remove the WARN_ON and fix the only caller to
not take the lock.

Signed-off-by: NeilBrown <neilb@suse.de>
---

Note that in current upstream fs/afs/dynroot.c also contains a call to
try_lookup_one_len() with unnecessary locking.  However
vfs-6.15.shared.afs contains a patch which removes that call, so I
didn't bother addressing it here.

NeilBrown


 fs/autofs/dev-ioctl.c | 3 ---
 fs/namei.c            | 5 ++---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c
index 6d57efbb8110..c5a6aae12d2c 100644
--- a/fs/autofs/dev-ioctl.c
+++ b/fs/autofs/dev-ioctl.c
@@ -442,7 +442,6 @@ static int autofs_dev_ioctl_timeout(struct file *fp,
 		sbi->exp_timeout = timeout * HZ;
 	} else {
 		struct dentry *base = fp->f_path.dentry;
-		struct inode *inode = base->d_inode;
 		int path_len = param->size - AUTOFS_DEV_IOCTL_SIZE - 1;
 		struct dentry *dentry;
 		struct autofs_info *ino;
@@ -460,9 +459,7 @@ static int autofs_dev_ioctl_timeout(struct file *fp,
 				"the parent autofs mount timeout which could "
 				"prevent shutdown\n");
 
-		inode_lock_shared(inode);
 		dentry = try_lookup_one_len(param->path, base, path_len);
-		inode_unlock_shared(inode);
 		if (IS_ERR_OR_NULL(dentry))
 			return dentry ? PTR_ERR(dentry) : -ENOENT;
 		ino = autofs_dentry_ino(dentry);
diff --git a/fs/namei.c b/fs/namei.c
index ecb7b95c2ca3..c6cef0af0625 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2863,15 +2863,14 @@ static int lookup_one_common(struct mnt_idmap *idmap,
  * Note that this routine is purely a helper for filesystem usage and should
  * not be called by generic code.
  *
- * The caller must hold base->i_mutex.
+ * No locks need be held - only a counted reference to @base is needed.
+ *
  */
 struct dentry *try_lookup_one_len(const char *name, struct dentry *base, int len)
 {
 	struct qstr this;
 	int err;
 
-	WARN_ON_ONCE(!inode_is_locked(base->d_inode));
-
 	err = lookup_one_common(&nop_mnt_idmap, name, base, len, &this);
 	if (err)
 		return ERR_PTR(err);
-- 
2.48.1

Re: [PATCH] VFS/autofs: try_lookup_one_len() does not need any locks

[Christian Brauner]

On Fri, 14 Mar 2025 09:32:54 +1100, NeilBrown wrote:
> try_lookup_one_len() is identical to lookup_one_unlocked() except that
> it doesn't include the call to lookup_slow().  The latter doesn't need
> the inode to be locked, so the former cannot either.
> 
> So fix the documentation, remove the WARN_ON and fix the only caller to
> not take the lock.
> 
> [...]

Ok, I merged this into the afs branch with a merge commit explaining why.

---

Applied to the vfs-6.15.shared.afs branch of the vfs/vfs.git tree.
Patches in the vfs-6.15.shared.afs branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.15.shared.afs

[1/1] VFS/autofs: try_lookup_one_len() does not need any locks
      https://git.kernel.org/vfs/vfs/c/be348aed9b44