On Fri, 11 Aug 2023 at 20:38, 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 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)).
I don't get it. We don't get so far as to set DCACHE_ATOMIC_OPEN if
LOOKUP_RCU is set.
> - Support atomic-open-revalidate in lookup_fast() - allow atomic
> open for positive dentries without O_CREAT being set.
>
> 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/fuse/dir.c | 5 ++---
> fs/namei.c | 17 +++++++++++++----
> include/linux/dcache.h | 6 ++++++
> include/linux/namei.h | 1 +
> 4 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index c02b63fe91ca..8ccd49d63235 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -380,7 +380,6 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
> if (name->len > FUSE_NAME_MAX)
> goto out;
>
> -
> forget = fuse_alloc_forget();
> err = -ENOMEM;
> if (!forget)
> @@ -771,8 +770,8 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> }
>
> static int _fuse_atomic_open(struct inode *dir, struct dentry *entry,
> - struct file *file, unsigned flags,
> - umode_t mode)
> + struct file *file, unsigned flags,
> + umode_t mode)
> {
>
> int err;
> diff --git a/fs/namei.c b/fs/namei.c
> index e4fe0879ae55..5dae1b1afd0e 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1643,12 +1643,14 @@ static struct dentry *lookup_fast(struct nameidata *nd)
> return ERR_PTR(-ECHILD);
> if (status == -ECHILD)
> /* we'd been told to redo it in non-rcu mode */
> - status = d_revalidate(dentry, nd->flags);
> + status = d_revalidate(dentry,
> + nd->flags | LOOKUP_ATOMIC_REVALIDATE);
> } else {
> dentry = __d_lookup(parent, &nd->last);
> if (unlikely(!dentry))
> return NULL;
> - status = d_revalidate(dentry, nd->flags);
> + status = d_revalidate(dentry,
> + nd->flags | LOOKUP_ATOMIC_REVALIDATE);
> }
> if (unlikely(status <= 0)) {
> if (!status)
> @@ -1656,6 +1658,12 @@ static struct dentry *lookup_fast(struct nameidata *nd)
> dput(dentry);
> return ERR_PTR(status);
> }
> +
> + if (unlikely(d_atomic_open(dentry))) {
> + dput(dentry);
> + return NULL;
> + }
> +
This looks like a hack. Why not move the d_atomic_open() check to the caller?
> return dentry;
> }
>
> @@ -3421,7 +3429,8 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
> if (d_in_lookup(dentry))
> break;
>
> - error = d_revalidate(dentry, nd->flags);
> + error = d_revalidate(dentry,
> + nd->flags | LOOKUP_ATOMIC_REVALIDATE);
> if (likely(error > 0))
> break;
> if (error)
> @@ -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 && !d_atomic_open(dentry)) {
> /* Cached positive dentry: will open in f_op->open */
> return dentry;
> }
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index 6b351e009f59..f90eec22691c 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -208,6 +208,7 @@ struct dentry_operations {
> #define DCACHE_FALLTHRU 0x01000000 /* Fall through to lower layer */
> #define DCACHE_NOKEY_NAME 0x02000000 /* Encrypted name encoded without key */
> #define DCACHE_OP_REAL 0x04000000
> +#define DCACHE_ATOMIC_OPEN 0x08000000 /* Always use ->atomic_open() to open this file */
>
> #define DCACHE_PAR_LOOKUP 0x10000000 /* being looked up (with parent locked shared) */
> #define DCACHE_DENTRY_CURSOR 0x20000000
> @@ -496,6 +497,11 @@ static inline bool d_is_fallthru(const struct dentry *dentry)
> return dentry->d_flags & DCACHE_FALLTHRU;
> }
>
> +static inline bool d_atomic_open(const struct dentry *dentry)
> +{
> + return dentry->d_flags & DCACHE_ATOMIC_OPEN;
> +}
> +
>
> extern int sysctl_vfs_cache_pressure;
>
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 1463cbda4888..7eec6c06b192 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -33,6 +33,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> #define LOOKUP_CREATE 0x0200 /* ... in object creation */
> #define LOOKUP_EXCL 0x0400 /* ... in exclusive creation */
> #define LOOKUP_RENAME_TARGET 0x0800 /* ... in destination of rename() */
> +#define LOOKUP_ATOMIC_REVALIDATE 0x1000 /* atomic revalidate possible */
>
> /* internal use only */
> #define LOOKUP_PARENT 0x0010
> --
> 2.34.1
>
