On Mon, Sep 21, 2020 at 08:27:14AM -0700, Paul E. McKenney wrote:
On Mon, Sep 21, 2020 at 06:19:39PM +1000, Herbert Xu wrote:
On Thu, Sep 17, 2020 at 09:58:02AM -0700, Eric Biggers wrote:
smp_load_acquire() is obviously correct, whereas READ_ONCE() is an optimization that is difficult to tell whether it's correct or not. For trivial data structures it's "easy" to tell. But whenever there is a->b where b is an internal implementation detail of another kernel subsystem, the use of which could involve accesses to global or static data (for example, spin_lock() accessing lockdep stuff), a control dependency can slip in.
If we're going to follow this line of reasoning, surely you should be converting the RCU derference first and foremost, no?
...
And to Eric's point, it is also true that when you have pointers to static data, and when the compiler can guess this, you do need something like smp_load_acquire(). But this is a problem only when you are (1) using feedback-driven compiler optimization or (2) when you compare the pointer to the address of the static data.
Let me restate what I think Eric is saying. He is concerned about the case where a->b and b is some opaque object that may in turn dereference a global data structure unconnected to a. The case in question here is crng_node_pool in drivers/char/random.c which in turn contains a spin lock.
But this reasoning could apply to any data structure that contains a spin lock, in particular ones that are dereferenced through RCU.
So my question if this reasoning is valid, then why aren't we first converting rcu_dereference to use smp_load_acquire?
Cheers,