Paul E. McKenney <[email protected]> wrote: > > Yes, preempt_disable() does indeed start an RCU read-side critical section, > just as surely as rcu_read_lock() does. > > However, this is a lockdep check inside of __rhashtable_lookup(): > > rht_dereference_rcu(ht->tbl, ht) > > Which is defined as: > > rcu_dereference_check(p, lockdep_rht_mutex_is_held(ht)); > > This is explicitly telling lockdep that rcu_read_lock() is OK and > holding ht->mutex is OK, but nothing else is.
I think that's a deficiency in rcu_dereference_check. Yes I could certainly add a preemption check to rht_dereference_rcu, but that makes zero sense because this is an implementation detail of RCU and there is no reason why this logic should be added to rhashtable. rhashtable never relies on the fact that turning off preemption creates is safe for RCU reads, so it makes no sense to add this logic to rht_dereference_rcu since RCU could conceivably (even if it is unlikely) be changed on day so that turning off preemption is no longer safe for RCU reads. My preference would be to add the preemption test to rcu_dereference_check, or if that is not possible for some reason, create a new RCU helper that includes the preemption test. Of course just adding RCU read locks as this patch does is also fine. Thanks, -- Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
