On 8/16/23 17:25, Miklos Szeredi wrote:
On Wed, 16 Aug 2023 at 16:34, Bernd Schubert <bschubert@xxxxxxx> wrote:
From: Miklos Szeredi <miklos@xxxxxxxxxx>
atomic_open() will do an open-by-name or create-and-open
depending on the flags.
If file was created, then the old positive dentry is obviously
stale, so it will be invalidated and a new one will be allocated.
If not created, then check whether it's the same inode (same as in
->d_revalidate()) and if not, invalidate & allocate new dentry.
Changes (v7 global series) from Miklos initial patch (by Bernd):
- LOOKUP_ATOMIC_REVALIDATE was added and is set for revalidate
calls into the file system when revalidate by atomic open is
supported - this is to avoid that ->d_revalidate() would skip
revalidate and set DCACHE_ATOMIC_OPEN, although vfs
does not supported it in the given code path (for example
when LOOKUP_RCU is set)).
- Support atomic-open-revalidate in lookup_fast() - allow atomic
open for positive dentries without O_CREAT being set.
Changes (v8 global series)
- Introduce enum for d_revalidate return values
- LOOKUP_ATOMIC_REVALIDATE is removed again
- DCACHE_ATOMIC_OPEN flag is replaced by D_REVALIDATE_ATOMIC
return value
Co-developed-by: Bernd Schubert <bschubert@xxxxxxx>
Signed-off-by: Miklos Szeredi <miklos@xxxxxxxxxx>
Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
Cc: Christian Brauner <brauner@xxxxxxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Dharmendra Singh <dsingh@xxxxxxx>
Cc: linux-fsdevel@xxxxxxxxxxxxxxx
---
fs/namei.c | 25 +++++++++++++++++++------
include/linux/namei.h | 6 ++++++
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index e4fe0879ae55..8381ec7645f5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -858,7 +858,7 @@ static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
return dentry->d_op->d_revalidate(dentry, flags);
else
- return 1;
+ return D_REVALIDATE_VALID;
}
/**
@@ -1611,10 +1611,11 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
}
EXPORT_SYMBOL(lookup_one_qstr_excl);
-static struct dentry *lookup_fast(struct nameidata *nd)
+static struct dentry *lookup_fast(struct nameidata *nd, int *atomic_revalidate)
bool?
{
struct dentry *dentry, *parent = nd->path.dentry;
int status = 1;
+ *atomic_revalidate = 0;
/*
* Rename seqlock is not required here because in the off chance
@@ -1656,6 +1657,10 @@ static struct dentry *lookup_fast(struct nameidata *nd)
dput(dentry);
return ERR_PTR(status);
}
+
+ if (status == D_REVALIDATE_ATOMIC)
+ *atomic_revalidate = 1;
+
return dentry;
}
@@ -1981,6 +1986,7 @@ static const char *handle_dots(struct nameidata *nd, int type)
static const char *walk_component(struct nameidata *nd, int flags)
{
struct dentry *dentry;
+ int atomic_revalidate;
/*
* "." and ".." are special - ".." especially so because it has
* to be able to know about the current root directory and
@@ -1991,7 +1997,7 @@ static const char *walk_component(struct nameidata *nd, int flags)
put_link(nd);
return handle_dots(nd, nd->last_type);
}
- dentry = lookup_fast(nd);
+ dentry = lookup_fast(nd, &atomic_revalidate);
if (IS_ERR(dentry))
return ERR_CAST(dentry);
if (unlikely(!dentry)) {
@@ -1999,6 +2005,9 @@ static const char *walk_component(struct nameidata *nd, int flags)
if (IS_ERR(dentry))
return ERR_CAST(dentry);
}
+
+ WARN_ON(atomic_revalidate);
+
if (!(flags & WALK_MORE) && nd->depth)
put_link(nd);
return step_into(nd, flags, dentry);
@@ -3430,7 +3439,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
dput(dentry);
dentry = NULL;
}
- if (dentry->d_inode) {
+ if (dentry->d_inode && error != D_REVALIDATE_ATOMIC) {
/* Cached positive dentry: will open in f_op->open */
return dentry;
}
@@ -3523,15 +3532,19 @@ static const char *open_last_lookups(struct nameidata *nd,
}
if (!(open_flag & O_CREAT)) {
+ int atomic_revalidate;
if (nd->last.name[nd->last.len])
nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
/* we _can_ be in RCU mode here */
- dentry = lookup_fast(nd);
+ dentry = lookup_fast(nd, &atomic_revalidate);
if (IS_ERR(dentry))
return ERR_CAST(dentry);
+ if (dentry && unlikely(atomic_revalidate)) {
Need to assert !LOOKUP_RCU
Are you sure? There is the BUG_ON(nd->flags & LOOKUP_RCU) directly after
- should be enough?
+ dput(dentry);
+ dentry = NULL;
+ }
Feels a shame to throw away the dentry. May be worth adding a helper
for the plain atomic open, most of the complexity of lookup_open() is
because of O_CREAT, so this should be much simplified.
Thanks, I'm going to look into it.
if (likely(dentry))
goto finish_lookup;
-
Adding/removing empty lines is just a distraction, so it shouldn't be
done unless it serves a real purpose.
Ah sorry, accidentally. I'm going to travel the next two days, going to
update it on Monday (or at best over the weekend).
Thanks,
Bernd
