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

[Amir Goldstein <amir73il@gmail.com>]

> > > 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?
>
> Dropping a positive dentry is harmful in case there's a long term
> reference to the dentry (e.g. an open file) since it will look as if
> the file was deleted, when in fact it wasn't.
>

I see. My point was that the negative->positive transition cannot
happen on underlying layers without user modifying underlying
layers underneath overlay, so it is fine to be in the "undefined" behavior
zone.

> It's possible to unhash a negative dentry in a safe way if we make
> sure it cannot become positive.  One way is to grab d_lock and remove
> it from the hash table only if count is one.
>
> So yes, we could have a helper to do that instead of the lookup flag.
> The disadvantage being that we'd also be dropping negatives that did
> not enter the cache because of our lookup.
>
> I don't really care, both are probably good enough for the overlayfs case.
>

There is another point to consider.
A negative underlying fs dentry may be useless for *this* overlayfs instance,
but since lower layers can be shared among many overlayfs instances,
for example, thousands of containers all testing for existence of file /etc/FOO
on startup.

It sounds like if we want to go through with DONTCACHE_NEGATIVE, that
it should be opt-in behavior for overlayfs.

Thanks,
Amir.