mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 01:55:51 +02:00
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>
This commit is contained in:
parent
3ef89feb38
commit
92cba84470
29
fs/dcache.c
29
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 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
|
|||
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 @@ struct dentry *d_alloc_parallel(struct dentry *parent,
|
|||
dput(new);
|
||||
return dentry;
|
||||
}
|
||||
rcu_read_unlock();
|
||||
hlist_bl_add_head(&new->d_in_lookup_hash, b);
|
||||
hlist_bl_unlock(b);
|
||||
return new;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user