Re: d_splice_alias() problem.

Nikita Danilov
2004-04-30T13:28:28Z

Neil Brown writes:
> On Friday April 23, Nikita@Namesys.COM wrote:
> > Hello,
> >
> > for some time I am observing that during stress tests over NFS
> >
> > shrink_slab->...->prune_dcache()->prune_one_dentry()->...->iput()
> >
> > is called on inode with ->i_nlink == 0 which results in truncate and
> > file deletion. This is wrong in general (file system is re-entered), and
> > deadlock prone on some file systems.
> >
> > After some debugging, I tracked problem down the to d_splice_alias()
> > failing to identify dentries when necessary.
> >
> > Suppose we have an inode with ->i_nlink == 1. It's accessed over NFS and
> > DCACHE_DISCONNECTED dentry D1 is created for it. Then, unlink request
> > comes for this file. nfsd looks name up in the parent directory
> > (nfsd_unlink()->lookup_one_len()). File system back-end uses
> > d_splice_alias(), but it only works for directories and we end up with
> > second (this time connected) dentry D2.
> >
> > D2 is successfully unlinked, file has ->i_nlink == 0, and ->i_count == 1
> > from D1, and when prune_dcache() hits D1 bad things happen.
> >
> > It's hard to imagine how new name can be identified with one among
> > multiple anonymous dentries, which is necessary for
> > NFSEXP_NOSUBTREECHECK export to work reliably.
> >
> > One possible work-around is to forcibly destroy all remaining
> > DCACHE_DISCONNECTED dentries when ->i_nlink drops to zero, but I am not
> > sure that this is possible and solves all problems of having more
> > dentries than there are nlinks.
> >
> > Nikita.
>
> If I understand you correctly, the main problem is that a disconnected
> dentry can hold an inode active after the last link has been removed.
> The file will not then be truncated and removed until memory pressure
> flushes the disconnected dentry from the dcache.
>
> This problem can be resolved by making sure that an inode never has
> both a connected and a disconnected dentry.
>
> This is already the case for directories (as they must only have one
> dentry), but it is not the case for non-directories.
>
> The following patch tries to address this. It is a "technology
> preview" in that the only testing I have done is that it compiles OK.

I have a test where such situation is reproducible and will give patch a
try.

Also, Al Viro pointed to me that it's not clear why DCACHE_DISCONNECTED
dentry is DCACHE_HASHED at all. If it were unhashed, last dput (done by
nfsd thread) would destroy it, truncating file if necessary.

>

Nikita.