Page MenuHomeFreeBSD

Implement a translation from FreeBSD atomic API to gcc intrinsics
Needs ReviewPublic

Authored by mjg on Thu, Feb 13, 2:23 PM.

Details

Reviewers
kib
markj
jeff
jhb
Summary

Which are of course supported by clang.

Primary motivation is to eliminate problems like this (mtx lock):

0xffffffff80cb873b <+139>:	xor    %eax,%eax
0xffffffff80cb873d <+141>:	lock cmpxchg %r14,0x18(%rbx)
0xffffffff80cb8743 <+147>:	sete   %cl
0xffffffff80cb8746 <+150>:	test   %cl,%cl
0xffffffff80cb8748 <+152>:	je     0xffffffff80cb8798 <vfs_ref+232>

patched:

0xffffffff80caaa3b <+139>:	xor    %eax,%eax
0xffffffff80caaa3d <+141>:	lock cmpxchg %r14,0x18(%rbx)
0xffffffff80caaa43 <+147>:	jne    0xffffffff80caaa8e <vfs_ref+222>

Note that clang apparently now supports indicating the result in flags, meaning the hand-rolled routine can be patched to end up generating equivalent code. Howver, I don't think there is any good reason now to handroll said routines.

Note there is no equivalent for testandclear/readandclear so they are left be.

Also I did not alter load/store routines. In my opinion that part needs to be reworked to be type-friendly to passed arguments and goes beyond the scope of this patch.

The patch does not add routines which were not already present on amd64, but this can be changed later. The intent is to eventually remove all hand-rolled routines.

I provided powerpc64 people with a patch for their platform and asked that they test. Works for me on amd64 of course.

Kernel .text changed only a little bit:

    text      data       bss        dec         hex   filename
22234960   1778275   4505600   28518835   0x1b329b3   GENERIC-NODEBUG.patched/kernel
22340096   1778275   4505600   28623971   0x1b4c463   GENERIC-NODEBUG.stock/kernel

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 29344

Event Timeline

mjg created this revision.Thu, Feb 13, 2:23 PM
jhb added a comment.Mon, Feb 17, 6:54 PM

So in CheriBSD we just replaced all of atomic(9) with the __atomic API supported in both modern GCC and clang for RISC-V:

https://github.com/CTSRD-CHERI/cheribsd/commit/e9e9ff10652179eaeec911ed804326bee482a0b3#diff-dc47b5c2fe7ae88823267345a6d1ae5c

However, that header could likely just be used on any architecture as none of it is MD. It also does the full atomic API and works with both clang and gcc.

I do think it would be nice to also add untyped variants as part of doing this as well.

There is a risk of trusting the compiler (we had to fix a bug in RISC-V clang I'm not sure is yet upstream for the 32-bit cmpset).