shrinking rcu_read_lock() scope in d_alloc_parallel()

	The current use of rcu_read_lock() uses in d_alloc_parallel()
is fairly opaque - the single large scope serves two purposes.

	We start with lookup in normal hash, and there rcu_read_lock()
scope puts __d_lookup_rcu() and subsequent lockref_get_not_dead() into
the same RCU read-side critical area.

	If no match is found, we proceed to lock the hash chain of
in-lookup hash and scan that for a match.  If we find a match, we want
to grab it and wait for lookup in progress to finish.  Since the bitlock
we use for these hash chains has to nest inside ->d_lock, we need to
unlock the chain first and use lockref_get_not_dead() on the match.
That has to be done without breaking the RCU read-side critical area,
and we use the same rcu_read_lock() scope to bridge over.

	The thing is, after having grabbed the reference (and it is
very unlikely to fail) we proceed to grab ->d_lock - d_wait_lookup()
and __d_lookup_unhash()/__d_wake_in_lookup_waiters() are using that for
serialization.	That makes lockref_get_not_dead() pointless - trying
to avoid grabbing ->d_lock for refcount increment, only to grab it
anyway immediately after that.	If we grab ->d_lock first and replace
lockref_get_not_dead() with direct check for sign and increment if
non-negative we can move rcu_read_unlock() to immediately after grabbing
->d_lock.  Moreover, we don't need the RCU read-side critical area to
be contiguous since before earlier __d_lookup_rcu() - we can just as
well terminate the earlier one ASAP and call rcu_read_lock() again only
after having found a match (if any) in the in-lookup hash chain.

	That makes the entire thing easier to follow and the purpose
of those rcu_read_lock() calls easier to describe - the first scope is
for __d_lookup_rcu() + lockref_get_not_dead(), the second one bridges
over from the bitlock scope to the ->d_lock scope on the match found in
in-lookup hash.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/dcache.c b/fs/dcache.c
index 94749442fa6899e61f1ec0cd4df4e72e115ec880..beb0523c33047b094d79449e6459aec875a2ac07 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2743,38 +2743,33 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
        spin_unlock(&parent->d_lock);
 
 retry:
-       rcu_read_lock();
        seq = smp_load_acquire(&parent->d_inode->i_dir_seq);
        r_seq = read_seqbegin(&rename_lock);
+       rcu_read_lock();
        dentry = __d_lookup_rcu(parent, name, &d_seq);
        if (unlikely(dentry)) {
                if (!lockref_get_not_dead(&dentry->d_lockref)) {
                        rcu_read_unlock();
                        goto retry;
                }
+               rcu_read_unlock();
                if (read_seqcount_retry(&dentry->d_seq, d_seq)) {
-                       rcu_read_unlock();
                        dput(dentry);
                        goto retry;
                }
-               rcu_read_unlock();
                dput(new);
                return dentry;
        }
-       if (unlikely(read_seqretry(&rename_lock, r_seq))) {
-               rcu_read_unlock();
+       rcu_read_unlock();
+       if (unlikely(read_seqretry(&rename_lock, r_seq)))
                goto retry;
-       }
 
-       if (unlikely(seq & 1)) {
-               rcu_read_unlock();
+       if (unlikely(seq & 1))
                goto retry;
-       }
 
        hlist_bl_lock(b);
        if (unlikely(READ_ONCE(parent->d_inode->i_dir_seq) != seq)) {
                hlist_bl_unlock(b);
-               rcu_read_unlock();
                goto retry;
        }
        /*
@@ -2791,19 +2786,20 @@ retry:
                        continue;
                if (!d_same_name(dentry, parent, name))
                        continue;
+               rcu_read_lock();
                hlist_bl_unlock(b);
+               spin_lock(&dentry->d_lock);
+               rcu_read_unlock();
                /* now we can try to grab a reference */
-               if (!lockref_get_not_dead(&dentry->d_lockref)) {
-                       rcu_read_unlock();
+               if (unlikely(dentry->d_lockref.count < 0)) {
+                       spin_unlock(&dentry->d_lock);
                        goto retry;
                }
-
-               rcu_read_unlock();
                /*
                 * somebody is likely to be still doing lookup for it;
-                * wait for them to finish
+                * pin it and wait for them to finish
                 */
-               spin_lock(&dentry->d_lock);
+               dget_dlock(dentry);
                d_wait_lookup(dentry);
                /*
                 * it's not in-lookup anymore; in principle we should repeat
@@ -2824,7 +2820,6 @@ retry:
                dput(new);
                return dentry;
        }
-       rcu_read_unlock();
        hlist_bl_add_head(&new->d_in_lookup_hash, b);
        hlist_bl_unlock(b);
        return new;