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@xxxxxxxxxx> 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@xxxxxxxxxx/T/#u for example, where READ_ONCE() was rejected), so I guess that is what people are going to agree on in this particular case. - Eric
