I don't understand why the rel fence isn't adequate to provide reciprocal fence to the acq fence(s) on the userspace read side. Re: writing individual words, I think to match the existing read pattern, I would store MSW then LSW. But, ILP32 platforms all claim to implement atomic_store_rel_64(); if that can tear, it seeems like a violation of semantics.

This is nonsense, it cannot work this way. Even assuming sequential consistency, it is trivial to find executions which would allow the reader to see inconsistent high/low pair if write is teared.

I might misunderstand what "atomic_store" means, but how can an atomic store tear?

This raises the question, why do you need the generation to be 64-bit value ? You said that the update frequency is once in a hour. Then 32bit counter should stay unwrapped for 500 billions of years, it seems.

That's a good question. The whitepaper is very explicit about the value being 64-bit. I don't know if that's just an artifact of Windows being somewhat picky about the architectures it runs on (maybe they've truly abandoned IA-32 now and only run on arm64). The counter is also bumped in a handful of other circumstances but none that essentially change the hour math.

I believe you are correct that a 32-bit counter should be sufficient for a long time (I get "only" 456,000 years).

Are you sure about store_rel_64() on all ILP32 ? For instance, on PPC ? Even if they implement it, it would be with some global lock, which is probably fine for C11 semantic of atomic, but not for this use, where subwords of atomic are used, and which is undefined behavior for C11. And if the code uses two atomic stores to implement 64bit write, aba recheck does not help.

Yes, I miscalculated the years by using 1<<64 instead of 1<<32 as the number of hours. My bad.

So I think we have two options for ILP32 userspace, including e.g. compat32 on amd64:

1. Wrap the 64-bit seed version update in the shared page with an additional two-step 32-bit generation, as originally proposed.(A)
2. Drop the size of the seed version exposed to ILP32 userspace to 32 bits as you suggested, and don't worry about it too much. Even with the smaller 2^32 - 1 math, the rollover is approximately x00,000 years. If it occurs, stale readers will still — with (2^32 - 1) / (2^32) probability — see non-equality and update their local generator anyway. By point of comparison, i386 will run out of time_t much sooner than this will rollover.

I think (2) seems adequate, although I'm concerned that maybe I don't understand why the whitepaper chose a 64-bit seed version explicitly. (1) would also not be too difficult.

Does either (1) or (2) seem like a reasonable option to you, @kib? Is there a better approach I'm missing? Do you have a preference, and if you prefer something like (1), does the implementation in (A) below seem correct to you?

Thanks,
Conrad

(A). E.g.,

shpage structure: { uint64_t seed; uint32_t gen; }

writer (locked/serialized externally):
atomic_store_32(&shpage->gen, shpage->gen + 1);
atomic_thread_fence_rel();
store_64(&shpage->seed, new_seed_version);
atomic_store_rel_32(&shpage->gen, shpage->gen + 1);

reader:
u32 gen1, gen2;
u64 seedversion;
do {
  // odd generation denotes concurrent writer; spin until writer is done.
  do {
    gen1 = atomic_load_acq_32(&shpage->gen);
  } while (gen1 % 2 == 1);

  seedversion = load_64(&shpage->seed);
  atomic_thread_fence_rel();
  gen2 = atomic_load_32(&shpage->gen);
} while (gen1 != gen2);

(2) looks completely adequate to me, and after all, this is exactly what I agitated for before.

Custom seqlock you wrote should work as well, but IMO is overkill if plain 32bit seed is enough.

(2) it will be.

Sure, that would be fine.

I really don't know what you want in a disable mechanism. I don't see how it can possibly be useful for this feature. Just as a reminder, RANDOM_FENESTRASX will land completely compiled-out of the GENERIC kernel anyway. I've tried to add a disable mechanism because you want it, but now you are complaining that it can't be re-enabled for processes that saw it was disabled.

I don't understand the concern on either end. I don't think this needs a disable mechanism, nor do I think it is especially significant how quickly the feature is re-enabled if the user already turned it off once.

I've been running this code in kernel and libc on my workstation and test VMs for something like two years now without issue.

One year*

Normally disable in vdso causes all currently running FreeBSD-ABI processes to stop using corresponding vdso facility. In reverse, enable causes them to start using it at the moment of enablement. This is not true for your variant of disable, which only acts once for the program, at exec time.

I am sure that if somebody sees potential issues with the patch the issue is fixed in advance, the 'disable' knob is for emergency and debug. It helped me to save several user machines for timecounter.

Anyway, I do not want to block the feature on that disagreement. Please fix AT_FXRNG numeric value and go ahead.

Ok

Thanks, will investigate.