On Mon, Dec 20, 2021 at 10:45:15PM +0100, Jason A. Donenfeld wrote:
Hi Paul,
On Mon, Dec 20, 2021 at 8:00 PM Paul E. McKenney paulmck@kernel.org wrote:
This assumes that the various crng_node_pool[i] pointers never change while accessible to readers (and that some sort of synchronization applies to the values in the pointed-to structure). If these pointers do change, then there also needs to be a READ_ONCE(pool[nid]) in select_crng(), where the value returned from this READ_ONCE() is both tested and returned. (As in assign this value to a temporary.)
But if the various crng_node_pool[i] pointers really are constant while readers can access them, then the cmpxchg_release() suffices. The loads from pool[nid] are then data-race free, and because they are unmarked, the compiler is prohibited from hoisting them out from within the "if" statement. The address dependency prohibits the CPU from reordering them.
Right, this is just an initialization-time allocation and assignment, never updated or freed again after.
So READ_ONCE() should be just fine. Which answers Jason's question. ;-)
Great. So v2 of this patch can use READ_ONCE then. Thanks!
Sure, I really don't care anymore. If people want READ_ONCE() here, I'll use it. It seems that the people who really prefer smp_load_acquire() aren't on this thread (unlike on https://lore.kernel.org/linux-fsdevel/20200713033330.205104-1-ebiggers@kerne... for example, where READ_ONCE() was rejected), so I guess that is what people are going to agree on in this particular case.
- Eric