On Tue, Sep 22, 2020 at 01:56:28PM -0700, Paul E. McKenney wrote:
You're missing the point here. b and c could easily be allocated by a function alloc_b() that's in another file.
I am still missing something.
If by "allocated" you mean something like kmalloc(), the compiler doesn't know the address. If you instead mean that there is a function that returns the address of another translation unit's static variable, then any needed ordering should preferably be built into that function's API. Either way, one would hope for some documentation of anything the caller needed to be careful of.
Besides which, control dependencies should be used only by LKMM experts at this point.
What does that even mean? Control dependencies are everywhere.
Does the following work better for you?
"... the non-local ordering properties of control dependencies should be relied on only by LKMM experts ...".
No. I don't know what that means. And I think very few people would know.
I just want to know if I use the one-time init pattern with a pointer to a data structure foo, are the readers using foo_use() supposed to use READ_ONCE() or are they supposed to use smp_load_acquire().
It seems the answer is that smp_load_acquire() is the only safe choice, since foo_use() *might* involve a control dependency, or might in the future since it's part of another kernel subsystem and its implementation could change.
If this control dependency's non-local ordering places any requirements on the users of that code, those requirements need to be clearly documented. It is of course better if the control dependency's non-local ordering properties are local to the code containing those control dependencies so that the callers don't need to worry about the resulting non-local ordering.
But in the LKMM documentation, you are likely to find LKMM experts who want to optimize all the way, particularly in cases like the one-time init pattern where all the data is often local. And the best basis for READ_ONCE() in one-time init is not a control dependency, but rather ordering of accesses to a single variable from a single task combined with locking, both of which are quite robust and much easier to use, especially in comparison to control dependencies.
My goal for LKMM is not that each and every developer have a full understanding of every nook and cranny of that model, but instead that people can find the primitives supporting the desired point in the performance/simplicity tradoff space. And yes, I have more writing to do to make more progress towards that goal.
So are you saying people should use smp_load_acquire(), or are you saying people should use READ_ONCE()?
C'mon, you know the answer to that! ;-)
The answer is that it depends on both the people and the situation.
In the specific case of crng, where you need address dependency ordering but the pointed-to data is dynamically allocated and never deallocated, READ_ONCE() now suffices [1]. Of course, smp_load_acquire() also suffices, at the cost of extra/expensive instructions on some architectures. The cmpxchg() needs at least release semantics, but presumably no one cares if this operation is a bit more expensive than it needs to be.
So, is select_crng() used on a fastpath? If so, READ_ONCE() might be necessary. If not, why bother with anything stronger than smp_load_acquire()? The usual approach is to run this both ways on ARM or PowerPC and see if it makes a significant difference. If there is no significant difference, keep it simple and just use smp_load_acquire().
If the code was sufficiently performance-insensitive, even better would be to just use locking. My hope is that no one bothered with the atomics without a good reason, but you never know.
I confess some uncertainty as to how the transition from the global primary_crng and the per-NUMA-node locks is handled. I hope that the global primary_crng guards global state that is disjoint from the state being allocated by do_numa_crng_init()!
crng_node_pool just uses the one-time init pattern. It's nothing unusual; lots of other places in the kernel want to do one-time initialization too. It seems to be one of the more common cases where people run into the LKMM at all. I tried to document it in https://lkml.kernel.org/lkml/20200717044427.68747-1-ebiggers@kernel.org/T/#u, but people complained it was still too complicated.
I hope that people can at least reach some general recommendation about READ_ONCE() vs. smp_load_acquire(), so that every kernel developer doesn't have to understand the detailed difference, and so that we don't need to have a long discussion (potentially requiring LWN coverage) about every patch.
Use the simplest thing that gets the job done. Which in the Linux kernel often won't be all that simple, but life is like that sometimes.
Thanx, Paul[1] It used to be that READ_ONCE() did -not- suffice on DEC Alpha, but this has thankfully changed, so that lockless_dereference() is no more.
Let me give an example using spinlock_t, since that's used in crng_node_pool. However, it could be any other data structure too; this is *just an example*. And it doesn't matter if the implementation is currently different; the point is that it's an *implementation*.
The allocation side uses spin_lock_init(), while the read side uses spin_lock(). Let's say that some debugging feature is enabled where spin locks use some global debugging information (say, a list of all locks) that gets allocated the first time a spin lock is initialized:
static struct spin_lock_debug_info *debug_info; static DEFINE_MUTEX(debug_info_alloc_mutex);
void spin_lock_init(spinlock_t *lock) { #ifdef CONFIG_DEBUG_SPIN_LOCKS mutex_lock(&debug_info_alloc_mutex); if (!debug_info) debug_info = alloc_debug_info(); add_lock(debug_info, lock); mutex_unlock(&debug_info_alloc_mutex); #endif real_spin_lock_init(lock); }
void spin_lock(spinlock_t *lock) { #ifdef CONFIG_DEBUG_SPIN_LOCKS debug_info->...; # use the debug info #endif real_spin_lock(lock); }
In that case, readers would have a control dependency between the condition of the data struct containing the spinlock_t being non-NULL, and the dereference of debug_info by spin_lock(). So anyone "receiving" a data structure containing a spinlock_t would need to use smp_load_acquire(), not READ_ONCE().
Point is, whether it's safe to use READ_ONCE() with a data structure or not is an implementation detail, not an API guarantee.
- Eric