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.
