kern: random: use a spinlock for the reseed lock

[kevans]

Summary

read_random() will often induce a reseed in fortuna; make the reseed
lock a spinlock to allow random consumers in non-sleepable contexts.

The fX implementation has the same issue in the standard read path with
the brng lock, but that's out of scope for the time being as I've not
done any testing with fX.

X-NetApp-PR: #69
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.

[markm]

This looks safe enough to me.

[cem]

I don't believe anything should be consuming random in interrupt contexts. Could you elaborate on the scenario / bug?

[kevans]

In D32478#732631, @cem wrote:

I don't believe anything should be consuming random in interrupt contexts. Could you elaborate on the scenario / bug?

I'll double check tomorrow, but it's some downstream code that's iirc trying to consume random in the net epoch... but I might be crossing memories on that one. It's definitely downstream stuff, at least. :-)

[mjg]

but net epoch is allowed to take a non-spin mutex

[kevans]

In D32478#732634, @mjg wrote:

but net epoch is allowed to take a non-spin mutex

OK, so definitely crossing memories on that one.

[kevans]

OK, without going into too much detail, the downstream code is periodically polling random in what's effectively a swi.

[kevans]

In D32478#732631, @cem wrote:

I don't believe anything should be consuming random in interrupt contexts. Could you elaborate on the scenario / bug?

Anything else I can do to better elucidate on NetApp's needs? When you say "should be" here, do you consider it a fundamentally bad idea or just that it's not a scenario that should be happening in stock FreeBSD?

[cem]

I meant fundamentally bad idea. Very little work should happen in interrupt context. Taking a global lock and running AES in software is somewhat expensive.

Also, I am concerned about the effect — of making this a spinlock — on all the non-SWI code that formerly blocked but would now spin on the lock.

[kevans]

In D32478#736882, @cem wrote:

I meant fundamentally bad idea. Very little work should happen in interrupt context. Taking a global lock and running AES in software is somewhat expensive.

Sure, fair enough; I think it's a not-quite-interrupt-context but they treat it as such for 'other reasons', but I don't fully understand the context (it's a bit of an atypical application). But given that:

Also, I am concerned about the effect — of making this a spinlock — on all the non-SWI code that formerly blocked but would now spin on the lock.

I'm going to push it back downstream and abandon this, since it's a low-maintenance diff for them to maintain. Thanks!