This change makes 3-4 assumptions:

1. Pointer-sized and natively aligned stores are atomic on all FreeBSD arch.
2. Pointer-sized and natively aligned loads are similarly atomic.
3. If the arbitrary order of function pointer initialization is significant to the loadable implementation, the loadable random module will invoke random_infra_init before callers.

Are 1-2 true, or should we instead use the atomic_store_rel / atomic_load_acq wrappers? Maybe that would show intent better even if the assumption is true.

My 4th assumption is this: that we care about being able to load a random.ko from kldload in userspace (for example, because the architecture does not use loader(8), or some other reason). If we don't need to be able to kldload from userspace, we could simplify this a *ton* by removing all the wrappers and having the loadable random provide the read_random_foo and is_random_seeded symbols directly. That would remove all of the assumptions above, and still remove the need for any configuration lock.

Thoughts?

I don't know what the consumers of random_loadable actually want from the interface. I'd prefer to just remove support entirely and let downstreams hack their proprietary random modules into their own downstream source trees in a way that meets their needs. Does anyone else consider that an option?

On second thought, this unrelated change may break userspace load (arc4random will be fine, but there are many direct read_random callers and I have not audited all of them for is_random_seeded checks). So I will drop it from a subsequent revisions of this diff.

When I did the last load of major code updates in this area, there was a LOT of pushback, some of it idealistic (the people who wanted every choice to be made by the owner/operator, at least by high principle), and some of it from the folks who wanted/needed *small* systems with most components unloadable if not deletable. Most of those objections seem to have quietened down now.

I'd be *delighted* to see the RANDOM_LOADABLE facility gone. Keep the KLD ability, please - it's nice to be able to test e.g. the NIST algorithms.

I think it may be possible. Here is my idea (untested): the (LOADABLE) function pointers already have non-conflicting names; leaving the existing (LOADABLE) sys/random.h macros. Then just define the (LOADABLE) version of these symbols with the ordinary names, rather than the suffixed names; they won't conflict. The macro can be avoided with void (read_random)( ... ) { } in the declarations/definitions, and function pointer assignment. I'll give it a shot. I will have intermittent availability for the next handful of days, so hopefully no rush :-).

No, unfortunately. The idea is that these are the standard definitions everyone gets -- so "read_random(9)" just works regardless of LOADABLE. The undefs in randomdev were an implementation detail for that file only. But I think it may be possible to avoid them, so I'll give it a shot.

Tiny nit. Update the comment to match the new function declaration.

Do we have any notion of how much faster this will be without the locking?

Are there any potential problems with this being reordered to THIRD that breaks assumptions about availability?

static? Maybe just remove the comment entirely since it purely duplicates the code?

Honestly, speed wasn't a factor I'd considered at all. That's not the motivation for the change. GENERIC sets nooption RANDOM_LOADABLE, so it didn't have these locks before this change.

I will double check, but I don't believe so.

I don't believe this one is problematic. The most problematic ordering is that algorithms (i.e., Fortuna), and randomdev.c both initialize at SI_SUB_RANDOM:SI_ORDER_SECOND. However, neither actually depends on the other, and SI_SUB_RANDOM is long before SI_SUB_SMP or any concurrency kicks off. So I think we're fine in practice.

Ok, fixed!