Re: Should list_entry_rcu use rcu_dereference ?

["Paul E. McKenney" <paulmck@linux.ibm.com>]

On Sat, May 04, 2019 at 02:59:44PM -0400, Joel Fernandes wrote:
> Hi,
> Sorry if this is a silly question.

Not at all silly!

> Looking at the list_entry_rcu primitive, I see it does direct READ_ONCE
> on ptr. That's Ok, but rcu_dereference also does additional lockdep and
> sparse checking.  Why not call rcu_dereference instead of READ_ONCE? The
> pointer may be dereference by the caller so IMO makes sense to check.
> 
> Here is the definition of list_entry_rcu:
> /**
>  * list_entry_rcu - get the struct for this entry
>  [snip]
> * This primitive may safely run concurrently with the _rcu list-mutation
>  * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock().
>  */
> #define list_entry_rcu(ptr, type, member) \
> 	container_of(READ_ONCE(ptr), type, member)
> 
> Also, I was curious why hlist_for_each_entry_rcu() uses rcu_dereference_raw()
> while __hlist_for_each_rcu)_ uses rcu_dereference(). I feel both should use
> rcu_dereference to have the lockdep checking. Is this not done due to
> performance reasons?
> 
> thanks!
> 
>   - Joel
> 
> https://cdn-images-1.medium.com/max/1600/0*XTYvSNj_rT2UZwfm.png

The issue is that most of the RCU list macros are generic over the RCU
read-side flavors.  We could have created _bh and _sched variants of all
of these, but that seemed like way too much RCU API expansion at the time,
and still does.  This shows up in the sparse checking as well, so that
there is just __rcu instead of also being __rcu_bh and __rcu_sched.

Or are do you have a trick in mind that would allow lockdep checking
without RCU API expansion?

							Thanx, Paul