diff mbox series

[v9,3/7,RFC] Allow atomic_open() on positive dentry

Message ID 20230920173445.3943581-4-bschubert@ddn.com (mailing list archive)
State New, archived
Headers show
Series fuse: full atomic open and atomic-open-revalidate | expand

Commit Message

Bernd Schubert Sept. 20, 2023, 5:34 p.m. UTC
From: Miklos Szeredi <miklos@szeredi.hu>

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.

Co-developed-by: Bernd Schubert <bschubert@ddn.com>
Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Bernd Schubert <bschubert@ddn.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Dharmendra Singh <dsingh@ddn.com>
Cc: linux-fsdevel@vger.kernel.org
---
 fs/namei.c            | 25 ++++++++++++++++++++-----
 include/linux/namei.h |  7 +++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

Comments

Amir Goldstein Sept. 21, 2023, 6:20 a.m. UTC | #1
On Wed, Sep 20, 2023 at 8:51 PM Bernd Schubert <bschubert@ddn.com> wrote:
>
> From: Miklos Szeredi <miklos@szeredi.hu>
>
> 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.
>
> Co-developed-by: Bernd Schubert <bschubert@ddn.com>
> Signed-off-by: Miklos Szeredi <miklos@szeredi.hu>
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Dharmendra Singh <dsingh@ddn.com>
> Cc: linux-fsdevel@vger.kernel.org
> ---
>  fs/namei.c            | 25 ++++++++++++++++++++-----
>  include/linux/namei.h |  7 +++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index e56ff39a79bc..f01b278ac0ef 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, 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
> @@ -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 = true;
> +
>         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;
> +       bool 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);
> +

WARN_ON_ONCE should be enough in most cases
to signal bad code without spamming the log.

Is there an error path that can be taken in this case?

Thanks,
Amir.

>         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,12 +3532,18 @@ static const char *open_last_lookups(struct nameidata *nd,
>         }
>
>         if (!(open_flag & O_CREAT)) {
> +               bool 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)) {
> +                       dput(dentry);
> +                       dentry = NULL;
> +               }
>                 if (likely(dentry))
>                         goto finish_lookup;
>
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 1463cbda4888..a70e87d2b2a9 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -47,6 +47,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
>  /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
>  #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
>
> +/* ->d_revalidate return codes */
> +enum {
> +       D_REVALIDATE_INVALID = 0, /* invalid dentry */
> +       D_REVALIDATE_VALID   = 1, /* valid dentry */
> +       D_REVALIDATE_ATOMIC =  2, /* atomic_open will revalidate */
> +};
> +
>  extern int path_pts(struct path *path);
>
>  extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/fs/namei.c b/fs/namei.c
index e56ff39a79bc..f01b278ac0ef 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, 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
@@ -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 = true;
+
 	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;
+	bool 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,12 +3532,18 @@  static const char *open_last_lookups(struct nameidata *nd,
 	}
 
 	if (!(open_flag & O_CREAT)) {
+		bool 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)) {
+			dput(dentry);
+			dentry = NULL;
+		}
 		if (likely(dentry))
 			goto finish_lookup;
 
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 1463cbda4888..a70e87d2b2a9 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -47,6 +47,13 @@  enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
 /* LOOKUP_* flags which do scope-related checks based on the dirfd. */
 #define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
 
+/* ->d_revalidate return codes */
+enum {
+	D_REVALIDATE_INVALID = 0, /* invalid dentry */
+	D_REVALIDATE_VALID   = 1, /* valid dentry */
+	D_REVALIDATE_ATOMIC =  2, /* atomic_open will revalidate */
+};
+
 extern int path_pts(struct path *path);
 
 extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);