public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 07/11] VFS: introduce lookup_and_lock()
Date: Fri, 20 Dec 2024 13:54:25 +1100	[thread overview]
Message-ID: <20241220030830.272429-8-neilb@suse.de> (raw)
In-Reply-To: <20241220030830.272429-1-neilb@suse.de>

lookup_and_lock() combines locking the directory and performing a lookup
prior to a change to the directory.
Abstracting this prepares for changing the locking requirements.

done_lookup_and_lock() will be called by all callers of
lookup_and_lock() to unlock and dput()

lookup_and_lock() returns -ENOENT if LOOKUP_CREATE was NOT given and the
name cannot be found,, and returns -EEXIST if LOOKUP_EXCL WAS given and
the name CAN be found.  This is what callers want.

These functions replace all uses of lookup_one_qstr_excl() in namei.c
except for those used for rename.

The name might seem backwards as the lock happens before the lookup.
A future patch will change this so that only a shared lock is taken
before the lookup, and an exclusive lock on the dentry is taken after a
successful lookup.  So the order "lookup" then "lock" will make sense.

This functionality is exported as lookup_and_lock_one() which takes a
name and len rather than a qstr.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/namei.c            | 118 ++++++++++++++++++++++--------------------
 include/linux/namei.h |   3 ++
 2 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 29f86df4b9dc..371c80902c59 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1703,6 +1703,33 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
 }
 EXPORT_SYMBOL(lookup_one_qstr_excl);
 
+static struct dentry *lookup_and_lock(const struct qstr *last,
+				      struct dentry *base,
+				      unsigned int lookup_flags)
+{
+	struct dentry *dentry;
+	int err;
+
+	inode_lock_nested(base->d_inode, I_MUTEX_PARENT);
+	dentry = lookup_one_qstr_excl(last, base, lookup_flags);
+	if (IS_ERR(dentry))
+		goto out;
+	err = -EEXIST;
+	if ((lookup_flags & LOOKUP_EXCL) && d_is_positive(dentry))
+		goto err;
+	err = -ENOENT;
+	if (!(lookup_flags & LOOKUP_CREATE) && d_is_negative(dentry))
+		goto err;
+	return dentry;
+
+err:
+	dput(dentry);
+	dentry = ERR_PTR(err);
+out:
+	inode_unlock(base->d_inode);
+	return dentry;
+}
+
 /**
  * lookup_fast - do fast lockless (but racy) lookup of a dentry
  * @nd: current nameidata
@@ -2741,16 +2768,9 @@ static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct
 		path_put(path);
 		return ERR_PTR(-EINVAL);
 	}
-	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
-	d = lookup_one_qstr_excl(&last, path->dentry, 0);
-	if (!IS_ERR(d) && d_is_negative(d)) {
-		dput(d);
-		d = ERR_PTR(-ENOENT);
-	}
-	if (IS_ERR(d)) {
-		inode_unlock(path->dentry->d_inode);
+	d = lookup_and_lock(&last, path->dentry, 0);
+	if (IS_ERR(d))
 		path_put(path);
-	}
 	return d;
 }
 
@@ -3051,6 +3071,22 @@ struct dentry *lookup_positive_unlocked(const char *name,
 }
 EXPORT_SYMBOL(lookup_positive_unlocked);
 
+struct dentry *lookup_and_lock_one(struct mnt_idmap *idmap,
+				   const char *name, int len, struct dentry *base,
+				   unsigned int lookup_flags)
+{
+	struct qstr this;
+	int err;
+
+	if (!idmap)
+		idmap = &nop_mnt_idmap;
+	err = lookup_one_common(idmap, name, base, len, &this);
+	if (err)
+		return ERR_PTR(err);
+	return lookup_and_lock(&this, base, lookup_flags);
+}
+EXPORT_SYMBOL(lookup_and_lock_one);
+
 #ifdef CONFIG_UNIX98_PTYS
 int path_pts(struct path *path)
 {
@@ -4080,7 +4116,6 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
 	unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
 	int type;
-	int err2;
 	int error;
 
 	error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
@@ -4092,50 +4127,31 @@ static struct dentry *filename_create(int dfd, struct filename *name,
 	 * (foo/., foo/.., /////)
 	 */
 	if (unlikely(type != LAST_NORM))
-		goto out;
+		goto put;
 
 	/* don't fail immediately if it's r/o, at least try to report other errors */
-	err2 = mnt_want_write(path->mnt);
+	error = mnt_want_write(path->mnt);
 	/*
 	 * Do the final lookup.  Suppress 'create' if there is a trailing
 	 * '/', and a directory wasn't requested.
 	 */
 	if (last.name[last.len] && !want_dir)
-		create_flags = 0;
-	inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr_excl(&last, path->dentry,
-				      reval_flag | create_flags);
+		create_flags &= ~LOOKUP_CREATE;
+	dentry = lookup_and_lock(&last, path->dentry, reval_flag | create_flags);
 	if (IS_ERR(dentry))
-		goto unlock;
+		goto drop;
 
-	error = -EEXIST;
-	if (d_is_positive(dentry))
-		goto fail;
-
-	/*
-	 * Special case - lookup gave negative, but... we had foo/bar/
-	 * From the vfs_mknod() POV we just have a negative dentry -
-	 * all is fine. Let's be bastards - you had / on the end, you've
-	 * been asking for (non-existent) directory. -ENOENT for you.
-	 */
-	if (unlikely(!create_flags)) {
-		error = -ENOENT;
-		goto fail;
-	}
-	if (unlikely(err2)) {
-		error = err2;
+	if (unlikely(error))
 		goto fail;
-	}
 	return dentry;
 fail:
 	d_lookup_done(dentry);
-	dput(dentry);
+	done_lookup_and_lock(path->dentry, dentry);
 	dentry = ERR_PTR(error);
-unlock:
-	inode_unlock(path->dentry->d_inode);
-	if (!err2)
+drop:
+	if (!error)
 		mnt_drop_write(path->mnt);
-out:
+put:
 	path_put(path);
 	return dentry;
 }
@@ -4555,23 +4571,18 @@ int do_rmdir(int dfd, struct filename *name)
 	if (error)
 		goto exit2;
 
-	inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags);
+	dentry = lookup_and_lock(&last, path.dentry, lookup_flags);
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit3;
-	if (!dentry->d_inode) {
-		error = -ENOENT;
-		goto exit4;
-	}
+
 	error = security_path_rmdir(&path, dentry);
 	if (error)
 		goto exit4;
 	error = vfs_rmdir(mnt_idmap(path.mnt), path.dentry->d_inode, dentry);
 exit4:
-	dput(dentry);
+	done_lookup_and_lock(path.dentry, dentry);
 exit3:
-	inode_unlock(path.dentry->d_inode);
 	mnt_drop_write(path.mnt);
 exit2:
 	path_put(&path);
@@ -4691,13 +4702,11 @@ int do_unlinkat(int dfd, struct filename *name)
 	if (error)
 		goto exit2;
 retry_deleg:
-	inode_lock_nested(path.dentry->d_inode, I_MUTEX_PARENT);
-	dentry = lookup_one_qstr_excl(&last, path.dentry, lookup_flags);
+	dentry = lookup_and_lock(&last, path.dentry, lookup_flags);
 	error = PTR_ERR(dentry);
 	if (!IS_ERR(dentry)) {
-
 		/* Why not before? Because we want correct error value */
-		if (last.name[last.len] || d_is_negative(dentry))
+		if (last.name[last.len])
 			goto slashes;
 		inode = dentry->d_inode;
 		ihold(inode);
@@ -4707,9 +4716,8 @@ int do_unlinkat(int dfd, struct filename *name)
 		error = vfs_unlink(mnt_idmap(path.mnt), path.dentry->d_inode,
 				   dentry, &delegated_inode);
 exit3:
-		dput(dentry);
+		done_lookup_and_lock(path.dentry, dentry);
 	}
-	inode_unlock(path.dentry->d_inode);
 	if (inode)
 		iput(inode);	/* truncate the inode here */
 	inode = NULL;
@@ -4731,9 +4739,7 @@ int do_unlinkat(int dfd, struct filename *name)
 	return error;
 
 slashes:
-	if (d_is_negative(dentry))
-		error = -ENOENT;
-	else if (d_is_dir(dentry))
+	if (d_is_dir(dentry))
 		error = -EISDIR;
 	else
 		error = -ENOTDIR;
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 898fc8ba37e1..f882874a7b00 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -83,6 +83,9 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap,
 struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap,
 					    const char *name,
 					    struct dentry *base, int len);
+struct dentry *lookup_and_lock_one(struct mnt_idmap *idmap,
+				   const char *name, int len, struct dentry *base,
+				   unsigned int lookup_flags);
 
 extern int follow_down_one(struct path *);
 extern int follow_down(struct path *path, unsigned int flags);
-- 
2.47.0


  parent reply	other threads:[~2024-12-20  3:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-20  2:54 [PATCH 00/11 RFC] Allow concurrent changes in a directory NeilBrown
2024-12-20  2:54 ` [PATCH 01/11] VFS: introduce vfs_mkdir_return() NeilBrown
2024-12-23  5:04   ` Al Viro
2024-12-23  7:26     ` NeilBrown
2024-12-20  2:54 ` [PATCH 02/11] VFS: add _shared versions of the various directory modifying inode_operations NeilBrown
2024-12-23  5:08   ` Al Viro
2024-12-20  2:54 ` [PATCH 03/11] VFS: use global wait-queue table for d_alloc_parallel() NeilBrown
2024-12-20  2:54 ` [PATCH 04/11] VFS: use d_alloc_parallel() in lookup_one_qstr_excl() NeilBrown
2024-12-20  2:54 ` [PATCH 05/11] VFS: change kern_path_locked() and user_path_locked_at() to never return negative dentry NeilBrown
2024-12-20  2:54 ` [PATCH 06/11] VFS: introduce done_lookup_and_lock() NeilBrown
2024-12-20  2:54 ` NeilBrown [this message]
2024-12-20  2:54 ` [PATCH 08/11] VFS: add inode_dir_lock/unlock NeilBrown
2024-12-21  1:21   ` Hillf Danton
2024-12-23  3:10     ` NeilBrown
2024-12-23 11:12       ` Hillf Danton
2024-12-23 20:36         ` NeilBrown
2024-12-24 10:26           ` Hillf Danton
2024-12-20  2:54 ` [PATCH 09/11] VFS: re-pack DENTRY_ flags NeilBrown
2024-12-20  2:54 ` [PATCH 10/11] VFS: take a shared lock for create/remove directory operations NeilBrown
2024-12-23  5:19   ` Al Viro
2024-12-23  7:11     ` NeilBrown
2024-12-23  7:26       ` Al Viro
2024-12-23 20:40         ` NeilBrown
2024-12-20  2:54 ` [PATCH 11/11] nfsd: use lookup_and_lock_one() NeilBrown
2024-12-20 20:55 ` [PATCH 00/11 RFC] Allow concurrent changes in a directory Andreas Dilger
2024-12-23  5:22 ` Al Viro

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=20241220030830.272429-8-neilb@suse.de \
    --to=neilb@suse.de \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --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: link
Be 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