On Sat, Jun 18, 2016 at 05:50:30AM +0900, J. R. Okajima wrote:
> hlist_bl_lock(b);
> rcu_read_unlock();
> hlist_bl_for_each_entry(dentry, node, b, d_u.d_in_lookup_hash) {
> if (!matched_dentry_found)
> continue;
> dget(dentry);
> hlist_bl_unlock(b);
> return dentry;
> }
> hlist_bl_add_head_rcu(&new->d_u.d_in_lookup_hash, b);
> hlist_bl_unlock(b);
> return new;
> }
> When two processes try opening a single existing file and enters
> d_alloc_parallel() at the same time, only one process wins and should
> succeeds hlist_bl_add_head_rcu(). The other process should find the
> dentry in d_u.d_in_lookup_hash and return 'dentry' (instead of
> 'new'). Am I right?
>
> My question is when will 'new' be added into d_u.d_in_lookup_hash?
You've quoted it yourself:
> hlist_bl_add_head_rcu(&new->d_u.d_in_lookup_hash, b);
> It should be between these two lines, I guess.
> rcu_read_unlock();
> hlist_bl_for_each_entry(dentry, node, b, d_u.d_in_lookup_hash) {
> But can it surely happen?
> If 'new' is not added here because someone else is in rcu_read_lock
> region or other reason, then both processes will add the same named but
> different dentry?
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.
> Is it better to change the lock/unlock-order like this?
>
> rcu_read_unlock();
> rcu_barrier();
> hlist_bl_lock(b);
> hlist_bl_for_each_entry(dentry, node, b, d_u.d_in_lookup_hash) {
No. We need to verify that nothing had been transferred from in-lookup to
primary hash after we'd found no match in the primary. That's what the
->i_dir_seq check is about, and it has to be done after we'd locked the
in-lookup hash chain. We could drop rcu_read_lock before locking the
chain, but this way we make sure we won't get preempted between fetching
the ->i_dir_seq and checking if it had been changed. The last thing we
want is rcu_barrier() - WTF for? To get an extra chance of something
being moved from in-lookup to primary, forcing us to repeat the primary
hash lookup?
We are *NOT* modifying the primary hash in d_alloc_parallel(). At all.
With respect to the primary hash it's a pure reader. And in-lookup hash
is only accessed under hlist_bl_lock() on its chains - no RCU accesses to
that one.
--
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
