illumos membar_producer emulation (zfs, dtrace)

Andriy Gapon avg at FreeBSD.org
Tue Oct 8 15:22:12 UTC 2019


I have got a passing interest in cleaning up of the atomic and related illumos
compatibility (and contrib) code.

At the moment I am looking at membar_producer.
>From Oracle Solaris documentation it follows that membar_producer is a store to
store barrier.
https://docs.oracle.com/cd/E88353_01/html/E37855/membar-producer-9f.html

I do not think that we have an MI abstraction for that, so it seems that
atomic_thread_fence_rel could be the closest thing that we have.  It also
affects loads which is redundant but not a big deal (IMO).

As of now, we have membar_producer implemented in assembly for several platforms
(aarch64, amd64, i386, powerpc64, sparc64) and it's a no-op for the rest.
I think that the latter is a problem as it can affect correctness of the code.

Also, the assembly implementations seem to be closer to wmb() although not
always the same.  E.g., on amd64 both are sfence, while on sparc64
membar_producer is "membar  #StoreStore" while wmb is "membar #MemIssue".
I think that those implementations, maybe except for sparc64, are a bit of an
overkill as the code that uses membar_producer only deals with "normal" memory.
Maybe illumos uses membar_producer for access ordering of both normal memory and
memory (or instructions) with special caching properties.

The relevant code is in:
- sys/cddl/compat/opensolaris/sys/atomic.h
- sys/cddl/compat/opensolaris/kern/opensolaris_atomic.c
- sys/cddl/contrib/opensolaris/common/atomic/*/opensolaris_atomic.S

So, what do you think, would it be safe to emulate membar_producer with
atomic_thread_fence_rel?
Or is it better to leave the current code (copied from illumos for amd64, i386,
sparc64 and home-grown for aarch64, powerpc64) alone?
In that case, we still need to add something for the rest of the platforms
(arm-s, mips-es, riscv).

P.S.
A bit offtopic.

I noticed a couple of uses of wmb() with stores to regular memory.
I think that they are either not needed or better be "release barriers".

vm_set_rendezvous_func() in sys/amd64/vmm/vmm.c
rendezvous_func is checked and modified under rendezvous_mtx anyway.

cpu_reset() in sys/x86/x86/cpu_machdep.c
I do not see a need for a store-store barrier here.

pmc_initialize() in sys/dev/hwpmc/hwpmc_mod.c
I think that a store_rel or fence_rel would be more appropriate.

Note sure about all uses of wmb in xen code, seems like at least some of them
should be release operations or fences as well.

-- 
Andriy Gapon


More information about the freebsd-arch mailing list