Re: Q. hlist_bl_add_head_rcu() in d_alloc_parallel()

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

On Sun, Jun 19, 2016 at 02:24:44PM +0900, J. R. Okajima wrote:
> - two processes try opening the same file
> - the both enter the hlist_bl_lock protected loop in d_alloc_parallel()
> 
> - the winner puts the new dentry into in-lookup hash
>   + here d_unhashed(dentry) would still return true.
> - then the winner process will call ->atomic_open or ->lookup. finally
>   d_add() and rehash will be called and the dentry will be moved to the
>   primary hash.
>   + here d_unhashed(dentry) would return false.


> As soon as the winner calls hlist_bl_unlock(), the looser starts
> d_in_lookup_hash loop and find the dentry which the winner added.
> 
> - the looser (or we should call processB) do the tests
> 	dentry->d_name.hash != hash
> 	dentry->d_parent != parent
> 	d_unhashed(dentry)
> - if processA has already called d_add and rehash, then this
>   d_unhashed() test would return false, and processB will throw away his
>   own 'new' dentry and return the found one.
> - if processA has NOT called d_add and rehash yet (due to the schedule
>   timing), then this d_unhashed() test would return true, and processB
>   will simply skip the found dentry.
>   in this case, processB will add his own 'new' dentry into in-lookup
>   hash and return it.

How would processB get past d_wait_lookup()?  It would have to have
observed !d_in_lookup() while holding ->d_lock on that sucker; it does
*not* drop ->d_lock through the tests you've mentioned.  And both the
removals of in-lookup flag and insertion into primary hash are done under
->d_lock without dropping it in between.

That was the point of taking security_d_instantiate() prior to attaching
to inode, etc. - that way we can make those actions (removal from in-lookup
hash, possibly attaching to inode, inserting into the primary hash) atomic
wrt d_wait_lookup().