Linux NFS development
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Al Viro <viro@zeniv.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Daire Byrne <daire@dneg.com>,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Chuck Lever <chuck.lever@oracle.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 03/10] VFS: move want_write checks into lookup_hash_update()
Date: Fri, 26 Aug 2022 12:10:43 +1000	[thread overview]
Message-ID: <166147984374.25420.3477094952897986387.stgit@noble.brown> (raw)
In-Reply-To: <166147828344.25420.13834885828450967910.stgit@noble.brown>

mnt_want_write() is always called before lookup_hash_update(), so
we can simplify the code by moving the call into that function.

If lookup_hash_update() succeeds, it now will have claimed the
want_write lock.  If it fails, the want_write lock isn't held.

To allow this, lookup_hash_update() now receives a 'struct path *'
instead of 'struct dentry *'

Note that when creating a name, any error from mnt_want_write() does not
get reported unless there is no other error.  For unlink/rmdir though,
an error from mnt_want_write() is immediately fatal - overriding ENOENT
for example.  This behaviour seems strange, but this patch is careful to
preserve it.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 fs/namei.c |   72 ++++++++++++++++++++++++++----------------------------------
 1 file changed, 31 insertions(+), 41 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 09c2d007814a..73c3319a1703 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1632,12 +1632,22 @@ static struct dentry *__lookup_hash(const struct qstr *name,
  */
 static struct dentry *lookup_hash_update(
 	const struct qstr *name,
-	struct dentry *base, unsigned int flags,
+	struct path *path, unsigned int flags,
 	wait_queue_head_t *wq)
 {
 	struct dentry *dentry;
+	struct dentry *base = path->dentry;
 	struct inode *dir = base->d_inode;
-	int err;
+	int err, err2;
+
+	/* For create, don't fail immediately if it's r/o,
+	 * at least try to report other errors.
+	 * For unlink/rmdir where LOOKUP_REVAl is the only
+	 * flag, fail immediately if r/o.
+	 */
+	err2 = mnt_want_write(path->mnt);
+	if (err2 && (flags & ~LOOKUP_REVAL) == 0)
+		return ERR_PTR(err2);
 
 	if (wq && IS_PAR_UPDATE(dir))
 		inode_lock_shared_nested(dir, I_MUTEX_PARENT);
@@ -1677,6 +1687,13 @@ static struct dentry *lookup_hash_update(
 			goto out_err;
 		}
 	}
+	if (err2) {
+		err = err2;
+		d_lookup_done(dentry);
+		d_unlock_update(dentry);
+		dput(dentry);
+		goto out_err;
+	}
 	return dentry;
 
 out_err:
@@ -1684,6 +1701,8 @@ static struct dentry *lookup_hash_update(
 		inode_unlock_shared(dir);
 	else
 		inode_unlock(dir);
+	if (!err2)
+		mnt_drop_write(path->mnt);
 	return ERR_PTR(err);
 }
 
@@ -1700,7 +1719,7 @@ struct dentry *lookup_hash_update_len(const char *name, int nlen,
 				    path->dentry, nlen, &this);
 	if (err)
 		return ERR_PTR(err);
-	return lookup_hash_update(&this, path->dentry, flags, wq);
+	return lookup_hash_update(&this, path, flags, wq);
 }
 EXPORT_SYMBOL(lookup_hash_update_len);
 
@@ -3891,15 +3910,10 @@ static struct dentry *filename_create_one(struct qstr *last, struct path *path,
 					  unsigned int lookup_flags,
 					  wait_queue_head_t *wq)
 {
-	struct dentry *dentry;
 	bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
 	unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
 	unsigned int create_flag = LOOKUP_CREATE;
-	int err2;
-	int error;
 
-	/* don't fail immediately if it's r/o, at least try to report other errors */
-	err2 = mnt_want_write(path->mnt);
 	/*
 	 * Do the final lookup.  Suppress 'create' if there is a trailing
 	 * '/', and a directory wasn't requested.
@@ -3910,24 +3924,9 @@ static struct dentry *filename_create_one(struct qstr *last, struct path *path,
 		 * or -EEXIST.
 		 */
 		create_flag = 0;
-	dentry = lookup_hash_update(last, path->dentry,
-				    reval_flag | create_flag | LOOKUP_EXCL, wq);
-	if (IS_ERR(dentry))
-		goto drop_write;
-
-	if (unlikely(err2)) {
-		error = err2;
-		goto fail;
-	}
-	return dentry;
-fail:
-	done_path_update(path, dentry, !!wq);
-	dput(dentry);
-	dentry = ERR_PTR(error);
-drop_write:
-	if (!err2)
-		mnt_drop_write(path->mnt);
-	return dentry;
+	return lookup_hash_update(last, path,
+				  reval_flag | create_flag | LOOKUP_EXCL,
+				  wq);
 }
 
 struct dentry *filename_create_one_len(const char *name, int nlen,
@@ -4300,23 +4299,18 @@ int do_rmdir(int dfd, struct filename *name)
 		goto exit2;
 	}
 
-	error = mnt_want_write(path.mnt);
-	if (error)
-		goto exit2;
-
-	dentry = lookup_hash_update(&last, path.dentry, lookup_flags, &wq);
+	dentry = lookup_hash_update(&last, &path, lookup_flags, &wq);
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
-		goto exit3;
+		goto exit2;
 	error = security_path_rmdir(&path, dentry);
 	if (error)
-		goto exit4;
+		goto exit3;
 	mnt_userns = mnt_user_ns(path.mnt);
 	error = vfs_rmdir(mnt_userns, path.dentry->d_inode, dentry);
-exit4:
+exit3:
 	done_path_update(&path, dentry, true);
 	dput(dentry);
-exit3:
 	mnt_drop_write(path.mnt);
 exit2:
 	path_put(&path);
@@ -4433,12 +4427,8 @@ int do_unlinkat(int dfd, struct filename *name)
 	if (type != LAST_NORM)
 		goto exit2;
 
-	error = mnt_want_write(path.mnt);
-	if (error)
-		goto exit2;
-
 retry_deleg:
-	dentry = lookup_hash_update(&last, path.dentry, lookup_flags, &wq);
+	dentry = lookup_hash_update(&last, &path, lookup_flags, &wq);
 	error = PTR_ERR(dentry);
 	if (!IS_ERR(dentry)) {
 		struct user_namespace *mnt_userns;
@@ -4457,6 +4447,7 @@ int do_unlinkat(int dfd, struct filename *name)
 exit3:
 		done_path_update(&path, dentry, true);
 		dput(dentry);
+		mnt_drop_write(path.mnt);
 	}
 	if (inode)
 		iput(inode);	/* truncate the inode here */
@@ -4466,7 +4457,6 @@ int do_unlinkat(int dfd, struct filename *name)
 		if (!error)
 			goto retry_deleg;
 	}
-	mnt_drop_write(path.mnt);
 exit2:
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {



  parent reply	other threads:[~2022-08-26  2:17 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-26  2:10 [PATCH/RFC 00/10 v5] Improve scalability of directory operations NeilBrown
2022-08-26  2:10 ` [PATCH 08/10] NFSD: allow parallel creates from nfsd NeilBrown
2022-08-27  4:37   ` Al Viro
2022-08-29  3:12     ` NeilBrown
2022-08-26  2:10 ` [PATCH 06/10] VFS: support concurrent renames NeilBrown
2022-08-27  4:12   ` Al Viro
2022-08-29  3:08     ` NeilBrown
2022-08-26  2:10 ` [PATCH 01/10] VFS: support parallel updates in the one directory NeilBrown
2022-08-26 19:06   ` Linus Torvalds
2022-08-26 23:06     ` NeilBrown
2022-08-27  0:13       ` Linus Torvalds
2022-08-27  0:23         ` Al Viro
2022-08-27 21:14         ` Al Viro
2022-08-27  0:17     ` Al Viro
2022-09-01  0:31       ` NeilBrown
2022-09-01  3:44         ` Al Viro
2022-08-27  3:43   ` Al Viro
2022-08-29  1:59     ` NeilBrown
2022-09-03  0:06       ` Al Viro
2022-09-03  1:40         ` NeilBrown
2022-09-03  2:12           ` Al Viro
2022-09-03 17:52             ` Al Viro
2022-09-04 23:33               ` NeilBrown
2022-08-26  2:10 ` [PATCH 02/10] VFS: move EEXIST and ENOENT tests into lookup_hash_update() NeilBrown
2022-08-26  2:10 ` [PATCH 07/10] VFS: hold DCACHE_PAR_UPDATE lock across d_revalidate() NeilBrown
2022-08-26  2:10 ` NeilBrown [this message]
2022-08-27  3:48   ` [PATCH 03/10] VFS: move want_write checks into lookup_hash_update() Al Viro
2022-08-26  2:10 ` [PATCH 05/10] VFS: export done_path_update() NeilBrown
2022-08-26  2:10 ` [PATCH 04/10] VFS: move dput() and mnt_drop_write() into done_path_update() NeilBrown
2022-08-26  2:10 ` [PATCH 09/10] VFS: add LOOKUP_SILLY_RENAME NeilBrown
2022-08-27  1:21   ` Al Viro
2022-08-29  3:15     ` NeilBrown
2022-08-26  2:10 ` [PATCH 10/10] NFS: support parallel updates in the one directory NeilBrown
2022-08-26 15:31   ` John Stoffel
2022-08-26 23:13     ` NeilBrown
2022-08-26 14:42 ` [PATCH/RFC 00/10 v5] Improve scalability of directory operations John Stoffel
2022-08-26 23:30   ` NeilBrown

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=166147984374.25420.3477094952897986387.stgit@noble.brown \
    --to=neilb@suse.de \
    --cc=chuck.lever@oracle.com \
    --cc=daire@dneg.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=trond.myklebust@hammerspace.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: 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