Re: [RFC PATCH v3 0/9] Suppress negative dentry

[Amir Goldstein <amir73il@gmail.com>]

On Mon, May 18, 2020 at 3:53 AM Ian Kent <raven@themaw.net> wrote:
>
> On Fri, 2020-05-15 at 15:20 +0800, Chengguang Xu wrote:
> > This series adds a new lookup flag LOOKUP_DONTCACHE_NEGATIVE
> > to indicate to drop negative dentry in slow path of lookup.
> >
> > In overlayfs, negative dentries in upper/lower layers are useless
> > after construction of overlayfs' own dentry, so in order to
> > effectively reclaim those dentries, specify LOOKUP_DONTCACHE_NEGATIVE
> > flag when doing lookup in upper/lower layers.
>
> I've looked at this a couple of times now.
>
> I'm not at all sure of the wisdom of adding a flag to a VFS function
> that allows circumventing what a file system chooses to do.

But it is not really a conscious choice is it?
How exactly does a filesystem express its desire to cache a negative
dentry? The documentation of lookup() in vfs.rst makes it clear that
it is not up to the filesystem to make that decision.
The VFS needs to cache the negative dentry on lookup(), so
it can turn it positive on create().
Low level kernel modules that call the VFS lookup() might know
that caching the negative dentry is counter productive.

>
> I also do really see the need for it because only hashed negative
> dentrys will be retained by the VFS so, if you see a hashed negative
> dentry then you can cause it to be discarded on release of the last
> reference by dropping it.
>
> So what's different here, why is adding an argument to do that drop
> in the VFS itself needed instead of just doing it in overlayfs?

That was v1 patch. It was dealing with the possible race of
returned negative dentry becoming positive before dropping it
in an intrusive manner.

In retrospect, I think this race doesn't matter and there is no
harm in dropping a positive dentry in a race obviously caused by
accessing the underlying layer, which as documented results in
"undefined behavior".

Miklos, am I missing something?

Thanks,
Amir.