Use an atomic instead of a reference to be domain-safe

[dinosaure]

No description provided.

[hannesm]

CI is failing, is this expected?

[dinosaure]

CI is failing, is this expected?

Yes, the CI fails only for OCaml < 4.13 (backoff requires OCaml 4.13 at least) but it works fine for OCaml >= 4.14.

[hannesm]

CI is failing, is this expected?

Yes, the CI fails only for OCaml < 4.13 (backoff requires OCaml 4.13 at least) but it works fine for OCaml >= 4.14.

Hmm, ok. I'd prefer to keep OCaml 4.09 support. Any chance to get that?

[reynir]

Is it maybe worth it to not use backoff? I don't imagine there will be much contention so I think it should be perfectly fine without.

[dinosaure]

Hmm, ok. I'd prefer to keep OCaml 4.09 support. Any chance to get that?

Is it maybe worth it to not use backoff?

Atomic are introduced only since 4.12. backoff requires at least 4.13. So, if we would like to solve our initial issue, we have different possibilities:

• lost the support of OCaml < 4.12
• do our own atomic_shims and keep the support of OCaml 4.09
• vendor backoff in mirage-crypto if we want to have a sain way to add a new entropy source.

[hannesm]

ok, so in the end I personally don't care much about OCaml < 4.14. Should we just put a lower bound? Or is there anyone hesitant to get their applications to work with older OCaml versions (/cc @mirage/core)

[hannesm]

I've not much clue -- but from what I can tell backoff requires on OCaml 4 the threads -- and I wonder whether this is available on MirageOS. Any hints welcome. I also don't know anything about the Atomic stuff, does this require threads or something on OCaml 4?

[palainp]

I recall pthread* being introduced in the pending ocaml-solo5 PR, so I guess threads are currently not available with solo5 targets :(

[hannesm]

so I took the initiative to not use backoff (unclear about the threads and MirageOS) -- instead use Atomic and compare_and_set in a tight loop.

Feels a bit strange since this may now not terminate, but given the amount of entropy sources that register themselves (at Rng.initialization time), I think it is safe (and/or once we observe infinite loop, we can reconsider).

/cc @dinosaure @reynir for review

[reynir]

It will always terminate since (at least) one domain will succeed in updating with compare_and_set* (think: if all domains fail every compare_and_set call then the value must have changed since they last read it => we have liveness). It can be beneficial to use backoff if there is a lot of contention to ease the CPU cache lines. Since we're probably going to add about a handful entropy sources it's IMO not worth it to add backoff.