On 4 Jun 2019, at 8:56, Trond Myklebust wrote: > On Tue, 2019-06-04 at 08:41 -0400, Benjamin Coddington wrote: >> Hey linux-nfs, and especially maintainers, >> >> I'm still interested in working on a problem raised a couple weeks >> ago, but >> confusion muddled that discussion and it died: >> >> If the client holds a read delegation, it will skip revalidation of a >> dentry >> in lookup. If the file was moved on the server, the client can end >> up with >> two positive dentries in cache for the same inode, and the dentry >> that >> doesn't exist on the server will never time out of the cache. >> >> The client can detect this happening because the directory of the >> dentry >> that should be revalidated updates it's change attribute. Skipping >> revalidation is an optimization in the case we hold a delegation, but >> this >> optimization should only be used when the delegation was obtained via >> a >> lookup of the dentry we are currently revalidating. >> >> Keeping the optimization might be done by tying the delegation to the >> dentry. Lacking some (easy?) way to do that currently, it seems >> simpler to >> remove the optimization altogether, and I will send a patch to remove >> it. > > A delegation normally applies to the entire inode. It covers _all_ > dentries that point to that inode too because create, rename and unlink > are always atomically accompanied by an inode change attribute. It should cover all dentries that point to that inode at the time the delegation was handed out. Shouldn't dentries cached _before_ the delegation be invalidated? The client doesn't currently care about the order of dentries cached with respect to delegations. > IOW: The proposed restriction is both unnecessary and incorrect. But then I think: need to store that change attribute on the dentry instead of what we currently use - a client-only monotonic counter. Then, we could compare the delegation's change attr to the dentry's. But that assumes they are both globally related -- that a directory's change_attr on lookup relates to an inode's change attribute. I don't see that anywhere (I'm looking in 7530).. >> Any thoughts on this? Any response, even asserting that this is not >> something >> we will fix are welcome. I think, what I am lacking (and I admit to have a tendency to become fixated) is proper guidance on whether or not work on this front is acceptable. I am happy to work on this, but not if my time is wasted. Help! Ben
