From: NeilBrown <neilb@suse.de>
To: Al Viro <viro@zeniv.linux.org.uk>, 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/12] VFS: move want_write checks into lookup_hash_update()
Date: Tue, 14 Jun 2022 09:18:21 +1000 [thread overview]
Message-ID: <165516230197.21248.171443631120298429.stgit@noble.brown> (raw)
In-Reply-To: <165516173293.21248.14587048046993234326.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.
Also lookup_hash_update() now receives a 'struct path'.
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 | 71 +++++++++++++++++++++++++-----------------------------------
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index e7a56d6e2472..83ce2f7083be 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1631,12 +1631,22 @@ static struct dentry *__lookup_hash(const struct qstr *name,
* If not LOOKUP_CREATE, name should already exist, else -ENOENT
*/
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 (!(dir->i_flags & S_PAR_UPDATE))
wq = NULL;
@@ -1675,6 +1685,11 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
dput(dentry);
goto retry;
}
+ if (err2) {
+ err = err2;
+ dput(dentry);
+ goto out_err;
+ }
return dentry;
out_err:
@@ -1682,6 +1697,8 @@ static struct dentry *lookup_hash_update(const struct qstr *name,
inode_unlock_shared(dir);
else
inode_unlock(dir);
+ if (!err2)
+ mnt_drop_write(path->mnt);
return ERR_PTR(err);
}
@@ -1698,7 +1715,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);
@@ -3857,15 +3874,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.
@@ -3876,25 +3888,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,
@@ -4269,23 +4265,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, &wq);
dput(dentry);
-exit3:
mnt_drop_write(path.mnt);
exit2:
path_put(&path);
@@ -4402,12 +4393,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;
@@ -4426,6 +4413,7 @@ int do_unlinkat(int dfd, struct filename *name)
exit3:
done_path_update(&path, dentry, &wq);
dput(dentry);
+ mnt_drop_write(path.mnt);
}
if (inode)
iput(inode); /* truncate the inode here */
@@ -4435,7 +4423,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)) {
next prev parent reply other threads:[~2022-06-13 23:20 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-13 23:18 [PATCH RFC 00/12] Allow concurrent directory updates NeilBrown
2022-06-13 23:18 ` [PATCH 04/12] VFS: move dput() and mnt_drop_write() into done_path_update() NeilBrown
2022-06-13 23:18 ` NeilBrown [this message]
2022-06-13 23:18 ` [PATCH 02/12] VFS: move EEXIST and ENOENT tests into lookup_hash_update() NeilBrown
2022-06-13 23:18 ` [PATCH 01/12] VFS: support parallel updates in the one directory NeilBrown
2022-06-13 23:18 ` [PATCH 05/12] VFS: export done_path_update() NeilBrown
2022-06-13 23:18 ` [PATCH 08/12] nfsd: allow parallel creates from nfsd NeilBrown
2022-06-24 14:43 ` Chuck Lever III
2022-06-28 22:35 ` Chuck Lever III
2022-06-28 23:09 ` NeilBrown
2022-07-04 17:17 ` Chuck Lever III
2022-06-13 23:18 ` [PATCH 07/12] NFS: support parallel updates in the one directory NeilBrown
2022-06-13 23:18 ` [PATCH 11/12] nfsd: use (un)lock_inode instead of fh_(un)lock NeilBrown
2022-06-24 14:43 ` Chuck Lever III
2022-06-13 23:18 ` [PATCH 06/12] VFS: support concurrent renames NeilBrown
2022-06-14 4:35 ` kernel test robot
2022-06-14 12:37 ` kernel test robot
2022-06-14 13:28 ` kernel test robot
2022-06-26 13:07 ` [VFS] 46a2afd9f6: ltp.rename10.fail kernel test robot
2022-06-13 23:18 ` [PATCH 12/12] nfsd: discard fh_locked flag and fh_lock/fh_unlock NeilBrown
2022-06-24 14:43 ` Chuck Lever III
2022-06-13 23:18 ` [PATCH 10/12] nfsd: reduce locking in nfsd_lookup() NeilBrown
2022-06-24 14:43 ` Chuck Lever III
2022-06-13 23:18 ` [PATCH 09/12] nfsd: support concurrent renames NeilBrown
2022-06-24 14:43 ` Chuck Lever III
2022-06-15 13:46 ` [PATCH RFC 00/12] Allow concurrent directory updates Daire Byrne
2022-06-16 0:55 ` NeilBrown
2022-06-16 10:48 ` Daire Byrne
2022-06-17 5:49 ` NeilBrown
2022-06-17 15:27 ` Daire Byrne
2022-06-20 10:18 ` Daire Byrne
2022-06-16 13:49 ` Anna Schumaker
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=165516230197.21248.171443631120298429.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=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;
as well as URLs for NNTP newsgroup(s).