On 10/23/23 20:30, Bernd Schubert wrote: > Previous patch allowed atomic-open on a positive dentry when > O_CREAT was set (in lookup_open). This adds in atomic-open > when O_CREAT is not set. > > Code wise it would be possible to just drop the dentry in > open_last_lookups and then fall through to lookup_open. > But then this would add some overhead for dentry drop, > re-lookup and actually also call into d_revalidate. > So as suggested by Miklos, this adds a helper function > (atomic_revalidate_open) to immediately open the dentry > with atomic_open. > > Signed-off-by: Bernd Schubert <bschubert@xxxxxxx> > Cc: Miklos Szeredi <miklos@xxxxxxxxxx> > Cc: Dharmendra Singh <dsingh@xxxxxxx> > Cc: Christian Brauner <brauner@xxxxxxxxxx> > Cc: Amir Goldstein <amir73il@xxxxxxxxx> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > Cc: linux-fsdevel@xxxxxxxxxxxxxxx > --- > fs/namei.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 63 insertions(+), 3 deletions(-) > > diff --git a/fs/namei.c b/fs/namei.c > index ff913e6b12b4..5e2d569ffe38 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -1614,10 +1614,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, bool *atomic_revalidate) > { > struct dentry *dentry, *parent = nd->path.dentry; > int status = 1; > + *atomic_revalidate = false; > > /* > * Rename seqlock is not required here because in the off chance > @@ -1659,6 +1660,10 @@ static struct dentry *lookup_fast(struct nameidata *nd) > dput(dentry); > return ERR_PTR(status); > } > + > + if (status == D_REVALIDATE_ATOMIC) > + *atomic_revalidate = true; > + > return dentry; > } > > @@ -1984,6 +1989,7 @@ static const char *handle_dots(struct nameidata *nd, int type) > static const char *walk_component(struct nameidata *nd, int flags) > { > struct dentry *dentry; > + bool atomic_revalidate; > /* > * "." and ".." are special - ".." especially so because it has > * to be able to know about the current root directory and > @@ -1994,7 +2000,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)) { > @@ -2002,6 +2008,9 @@ static const char *walk_component(struct nameidata *nd, int flags) > if (IS_ERR(dentry)) > return ERR_CAST(dentry); > } > + > + WARN_ON_ONCE(atomic_revalidate); > + > if (!(flags & WALK_MORE) && nd->depth) > put_link(nd); > return step_into(nd, flags, dentry); > @@ -3383,6 +3392,42 @@ static struct dentry *atomic_open(struct nameidata *nd, struct dentry *dentry, > return dentry; > } > > +static struct dentry *atomic_revalidate_open(struct dentry *dentry, > + struct nameidata *nd, > + struct file *file, > + const struct open_flags *op, > + bool *got_write) > +{ > + struct mnt_idmap *idmap; As kernel test robot noticed, idmap is unused in this function.