On Thu, May 19, 2022 at 10:43:54PM +0200, Bernd Schubert wrote:
>
>
> On 5/19/22 22:09, Vivek Goyal wrote:
> > 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?
>
> Yeah, I think Dharmendra and I had internally already debated over this. In
> order to reduce complexity for the patches we preferred to go without vfs
> modifications.
Fair enough. It is not trivial to be able to see all the paths and make
sure none of these paths is broken.
>
> I assume the patch is a follow up to this comment
> https://lore.kernel.org/all/20220517100744.26849-1-dharamhans87@xxxxxxxxx/T/#m8bd440ddea4c135688c829f34e93371e861ba9fa
>
Yes looks like. There are too many paths here and being able to wrap
one's head around all the paths is not trivial. Thankfully miklos
has summarized it here.
https://lore.kernel.org/all/20220517100744.26849-1-dharamhans87@xxxxxxxxx/T/#m90f64cd8c8fff70e2fba2b551ae01d0d47b3337e
Thanks
Vivek
