Messages in this thread | | | Date | Fri, 05 Apr 2013 16:26:27 -0400 | From | Waiman Long <> | Subject | Re: [PATCH v2 1/4] dcache: Don't take unnecessary lock in d_count update |
| |
On 04/05/2013 01:12 PM, Al Viro wrote: >> @@ -635,22 +640,14 @@ struct dentry *dget_parent(struct dentry *dentry) >> { >> struct dentry *ret; >> >> -repeat: >> - /* >> - * Don't need rcu_dereference because we re-check it was correct under >> - * the lock. >> - */ >> rcu_read_lock(); >> - ret = dentry->d_parent; >> - spin_lock(&ret->d_lock); >> - if (unlikely(ret != dentry->d_parent)) { >> - spin_unlock(&ret->d_lock); >> - rcu_read_unlock(); >> - goto repeat; >> - } >> + ret = rcu_dereference(dentry->d_parent); >> rcu_read_unlock(); >> + if (dcount_inc_cmpxchg(ret)) >> + return ret; >> + spin_lock(&ret->d_lock); > And WTF is going to protect your "ret" from being freed just as you'd done > rcu_read_unlock()?
I think I had made a mistake here. I should move the rcu_read_unlock() down to before the return statement as well as after the spin_lock(). Thank for pointing this out. I will fix that in the next version. Anything else that needs to be fixed?
Regards, Longman
| |