Re: races in ll_splice_alias() and elsewhere (ext4, ocfs2)

[Al Viro <viro@ZenIV.linux.org.uk>]

On Thu, Mar 10, 2016 at 03:08:57AM +0000, Drokin, Oleg wrote:
> > Note, BTW, that d_splice_alias() will not look for aliases in case of
> > non-directories - for those it's the same d_add(), since there we can
> > legitimately have many dentry aliases over the same inode.  For directories
> > we *can't*.
> 
> Ah! Btw this highlights the missed case for d_exact_alias + d_splice_alias
> in Lustre with current collection of patches you carry.
> 
> Suppose we have a dentry pointing to an inode all nicely covered by a lustre
> lock (to ensure it is valid).
> Now some other client does something that invalidates the name (rename,
> or just open(O_CREAT) even), this causes our local lock to disappear
> and causes the dentry to be declared "lustre invalid" via
> ll_md_blocking_ast()->ll_invalidate_aliases()->d_lustre_invalidate()
> 
> This sets the "invalid" flag in __d_lustre_invalidate() and
> also would try to unhash the dentry in some cases:
>         if (d_count(dentry) == 0 && !(dentry->d_flags & DCACHE_DISCONNECTED))
>                 __d_drop(dentry);
> 
> Now if none of those conditions hit, the dentry stays hashed.
> But the problem is if it's not a directory, then d_splice_alias would
> also ignore it, while d_exact_alias would ignore it due to it being still hashed
> and this dentry would just hang around uselessly taking RAM.
> 
> Is there an easy way to rectify this, I wonder?

Wait a minute.  If it's hashed, has the right name and the right parent,
why the hell are we calling ->lookup() on a new dentry in the first place?
Why hadn't we simply picked it from dcache?