[removing Ted from CC since I am not going over ext4 part and he's quite
busy already I am sure.]
On Mar 9, 2016, at 9:20 PM, Al Viro wrote:
> [ext4 and ocfs2 folks Cc'd]
> On Wed, Mar 09, 2016 at 11:47:50PM +0000, Drokin, Oleg wrote:
>
>> ocfs2 reimplements d_splice_alias as ocfs2_find_local_alias where they
>> sidestep the initialization of d_fsdata issue (even though they
>> refer to it as d_splice_alias elsewhere in the comments).
>
> Not really. One of the callers (ocfs2_lookup() -> ocfs2_dentry_attach_lock()
> -> ocfs2_find_local_alias()) does that after d_splice_alias(); it doesn't
Oh, I missed that call somehow.
> 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?
>> With this (and the other one) fixes in place I see no other problems
>> with your patch.
>>
>> So if you think the patch below is ok, I can submit it to Greg
>> (with addition of removing the extra init after ll_find_alias call),
>> or to you if you think it's better for you to carry it in your tree
>> (the one below probably has whitespace all wrong, so it won't really
>> apply as is and needs a proper resend).
>
> I'll pick it. Mind if I fold it into the one I'd posted (with credits,
> obviously)?
Sure. Thanks.
Bye,
Oleg
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
