Is it intentional that these have the same value? I presume that the membarrier()'s "registration" verbs exist to handle races where the initiating thread fails to observe a concurrent pm_active modification by another thread that is switching on to a CPU.

This is functionally correct, but does not have the performance characteristics that are expected from the Linux documentation. The non-_EXPEDITED versions are allowed to block and are expected to wait until all of the other cores have done at least one context switch. Callers are likely to expect this to potentially take one scheduling quantum to return but not to disrupt the other threads' execution at all.

If I understand correctly (some comments would be nice so I could check my understanding), this is collecting the set of CPUs that are running threads in the same process. It would be worth a comment explaining the inherent optimisation. For functional correctness, this does not need to be an accurate snapshot of the set of CPUs: it can be a racy access because any thread that is [de]scheduled out from under you during the check is implicitly meeting the requirements (anything that does a context switch is sufficiently ordered).

It's confusing reading this that each command is a separate bit, yet the membarrier call requires that you provide a single one. Please can you add a comment that this is because MEMBARRIER_CMD_QUERY returns a bitmask of all supported commands.

I was going to ask if we can be better than Linux here and provide an enum so that these are somewhat type-safe, but I note that the Linux headers actually do define these as an enum, rather than as macros. Please can we do the same?

Source compatibility with what? Source code that relies on restartable sequences won't work on FreeBSD. glibc does not provide a wrapper around this, so any source code that actually uses it is using the system call via syscall.

This is unused and relates to a flag that is not supported. It is confusing for consuming code if flags are defined in headers for features that the OS doesn't support.

Note that until very recently this was a two-argument function on Linux. My Ubuntu 20.04 install with a 5.11 kernel has the two-argument version. I wouldn't be surprised if Linux changes the number of arguments in the future. If we want to avoid breakage then we may want to do an open-like thing here and make the libc wrapper membarrier(int, unsigned, ...) and forward to the correct kernel variant based on the second argument.

Please can we have comments that explain what each of these are for?

I don't see any references to this function in the diff, what calls it?

We could add pm_active to the pmap struct on arm64 at the cost of 2 atomic operations per-process switch and the extra used space in the struct. We may not want to MFC it if we consider pmap_t to be have stable size outside of pmap.c so would be better added later if so.

In theory this is unbounded sleep, but ok. I changed the _GLOBAL implementation to wait for each core to do at least one non-idle context switch.

Technically this is not just running threads, it is also e.g. rfork()ed siblings.

More correct explanation is the set of threads sharing the same address space. I believe this is identical to the set which is IPI-ed by linux.

I found it very strange that MEMBARRIER_CMDs are bitmask values but packed in enum. But I do not have strong opinion.

Source compatible with Linux. For instance I see a code in urcu which tests for presence of _EXPEDITED_RSEQ in _QUERY response. With this constant present in the header, the code just compiles and works without RSEQ support (well, it requires replacing __syscall with direct function call, but you should see what I mean)

It reserves the ABI values for future extensions, there is nothing unusual.

I will probably look at rseq stuff after this bits are landed.

My instance of the manual page states that the syscall takes three args. I certainly can go with libc wrapper as you suggested, but lets postpone it.

I want to do some more systematic review of consumers. I looked at urcu, which requires a patch anyway due to use of syscall, and I was not able to pace verona build system.

No, it is a stupidity.

It is actually pmap_active_cpus(), I did some rototiling for arm64 and did not rechecked afterward.

I do not see a point. This would make each context switch to pay two atomics for the benefit of rarely used API. It would be esp. saddle because armv8 indeed does not need to maintain pm_active for TLB invalidations due to availability of broadcast invl instructions.

Having looked at the other pmap implementations I noticed on powerpc pm_active is managed, but doesn't seem to be currently used (other than by this patch).

Which makes powerpc another candidate for the common (naive) implementation of pmap_active_cpus().

Please can you add the requested comment explaining *why* they are separate bits (i.e. so that MEMBARRIER_CMD_QUERY can return the set of supported commands as a bitfield)?

This is fine, but I'm somewhat inclined to add CAPENABLED to syscalls.master (or additionally). It's less consistent, but better matches kern/syscalls.master and people are less likely to forget.

I added CAPENABLED to freebsd32/syscalls.master both for membarrier and rseq. I prefer to keep the entry in capabilities.conf until this file exists.

[I will upload new patch when more changes accumulate]

Since this is only used in the smp_rendezvous case, never called directly, I think it can be a nop. 'rfi' and its many variants is a context synchronizing instruction.

I did not rechecked it, but most likely it is what Linux does. Do you mean that our rendezvous on ppc already does isync or stronger barrier?

Yes. Rendezvous does the following:

atomic_add_acq_int() (add+isync)
action
atomic_add_rel_int(lwsync or sync + add)

So the rendezvous is already a heavyweight barrier on its own.