On Thu, May 19, 2022 at 04:43:58PM +0200, Miklos Szeredi wrote:
> Hi Al,
>
> Do you see anything bad with allowing ->atomic_open() to take a positive dentry
> and possibly invalidate it after it does the atomic LOOKUP/CREATE+OPEN?
>
> It looks wrong not to allow optimizing away the roundtrip associated with
> revalidation when we do allow optimizing away the roundtrip for the initial
> lookup in the same situation.
>
> Thanks,
> Miklos
>
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 509657fdf4f5..d35b5cbf7f64 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3267,7 +3267,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;
Hi Miklos,
I see that lookup_open() calls d_revalidate() first. So basically
idea is that fuse ->.d_revalidate will skip LOOKUP needed to make sure
dentry is still valid (Only if atomic lookup+open is implemented) and
return 1 claiming dentry is valid.
And later in ->atomic_open(), it will either open the file or
get an error and invalidate dentry. Hence will save one LOOKUP in
success case. Do I understand the intent right?
Thanks
Vivek
> }
> diff --git a/include/linux/dcache.h b/include/linux/dcache.h
> index f5bba51480b2..da681bdbc34e 100644
> --- a/include/linux/dcache.h
> +++ b/include/linux/dcache.h
> @@ -208,6 +208,7 @@ struct dentry_operations {
> #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
> #define DCACHE_NORCU 0x40000000 /* No RCU delay for freeing */
> @@ -446,6 +447,11 @@ static inline bool d_is_positive(const struct dentry *dentry)
> return !d_is_negative(dentry);
> }
>
> +static inline bool d_atomic_open(const struct dentry *dentry)
> +{
> + return dentry->d_flags & DCACHE_ATOMIC_OPEN;
> +}
> +
> /**
> * d_really_is_negative - Determine if a dentry is really negative (ignoring fallthroughs)
> * @dentry: The dentry in question
>
