Re: Q. hlist_bl_add_head_rcu() in d_alloc_parallel()

["J. R. Okajima" <hooanon05g@gmail.com>]

Al Viro:
> Huh?  If you have two processes reaching that insertion into the in-lookup
> hash, whoever gets the hlist_bl_lock() first wins; the loser will find
> the instance added by the winner and bugger off with it.  RCU is completely
> unrelated to that.  It's about the search in *primary* hash.

Ok, forget about rcu_barrier.

----------------------------------------

> 	look for match in in-lookup hash
> 	if found
> 		unlock the chain
> 		wait for the match to cease being in-lookup
> 		drop the match
> 		goto retry	[see below]
> 	insert new into in-lookup hash

The actual matching test which corresponds to above pseudo-code (if
found) is this (from v4.7-rc3).

	dentry->d_name.hash != hash
	dentry->d_parent != parent
	d_unhashed(dentry)
	name (length and string)

I am afraid this d_unhashed() test is racy.
Here is what I am guessing.

- 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.

Finally this race between these two
- d_add and rehash via ->atomic_open or ->lookup
- d_unhashed test in d_alloc_parallel
leads to the duplicated dentries (same named dentry under the same
parent).

Do you think it can happen?


J. R. Okajima