atomicbitops package cleanups

- Redocument memory ordering from "no ordering" to "acquire-release". (No
  functional change: both LOCK WHATEVER on x86, and LDAXR/STLXR loops on ARM64,
  already have this property.)

- Remove IncUnlessZeroInt32 and DecUnlessOneInt32, which were only faster than
  the equivalent loops using sync/atomic before the Go compiler inlined
  non-unsafe.Pointer atomics many releases ago.

PiperOrigin-RevId: 295811743
This commit is contained in:
gVisor bot
2020-02-18 13:43:28 -08:00
parent c841373013
commit 906eb6295d
8 changed files with 40 additions and 212 deletions
+4 -5
View File
@@ -5,10 +5,9 @@ package(licenses = ["notice"])
go_library(
name = "atomicbitops",
srcs = [
"atomic_bitops.go",
"atomic_bitops_amd64.s",
"atomic_bitops_arm64.s",
"atomic_bitops_common.go",
"atomicbitops.go",
"atomicbitops_amd64.s",
"atomicbitops_noasm.go",
],
visibility = ["//:sandbox"],
)
@@ -16,7 +15,7 @@ go_library(
go_test(
name = "atomicbitops_test",
size = "small",
srcs = ["atomic_bitops_test.go"],
srcs = ["atomicbitops_test.go"],
library = ":atomicbitops",
deps = ["//pkg/sync"],
)
@@ -14,47 +14,34 @@
// +build amd64 arm64
// Package atomicbitops provides basic bitwise operations in an atomic way.
// The implementation on amd64 leverages the LOCK prefix directly instead of
// relying on the generic cas primitives, and the arm64 leverages the LDAXR
// and STLXR pair primitives.
// Package atomicbitops provides extensions to the sync/atomic package.
//
// WARNING: the bitwise ops provided in this package doesn't imply any memory
// ordering. Using them to construct locks must employ proper memory barriers.
// All read-modify-write operations implemented by this package have
// acquire-release memory ordering (like sync/atomic).
package atomicbitops
// AndUint32 atomically applies bitwise and operation to *addr with val.
// AndUint32 atomically applies bitwise AND operation to *addr with val.
func AndUint32(addr *uint32, val uint32)
// OrUint32 atomically applies bitwise or operation to *addr with val.
// OrUint32 atomically applies bitwise OR operation to *addr with val.
func OrUint32(addr *uint32, val uint32)
// XorUint32 atomically applies bitwise xor operation to *addr with val.
// XorUint32 atomically applies bitwise XOR operation to *addr with val.
func XorUint32(addr *uint32, val uint32)
// CompareAndSwapUint32 is like sync/atomic.CompareAndSwapUint32, but returns
// the value previously stored at addr.
func CompareAndSwapUint32(addr *uint32, old, new uint32) uint32
// AndUint64 atomically applies bitwise and operation to *addr with val.
// AndUint64 atomically applies bitwise AND operation to *addr with val.
func AndUint64(addr *uint64, val uint64)
// OrUint64 atomically applies bitwise or operation to *addr with val.
// OrUint64 atomically applies bitwise OR operation to *addr with val.
func OrUint64(addr *uint64, val uint64)
// XorUint64 atomically applies bitwise xor operation to *addr with val.
// XorUint64 atomically applies bitwise XOR operation to *addr with val.
func XorUint64(addr *uint64, val uint64)
// CompareAndSwapUint64 is like sync/atomic.CompareAndSwapUint64, but returns
// the value previously stored at addr.
func CompareAndSwapUint64(addr *uint64, old, new uint64) uint64
// IncUnlessZeroInt32 increments the value stored at the given address and
// returns true; unless the value stored in the pointer is zero, in which case
// it is left unmodified and false is returned.
func IncUnlessZeroInt32(addr *int32) bool
// DecUnlessOneInt32 decrements the value stored at the given address and
// returns true; unless the value stored in the pointer is 1, in which case it
// is left unmodified and false is returned.
func DecUnlessOneInt32(addr *int32) bool
@@ -75,41 +75,3 @@ TEXT ·CompareAndSwapUint64(SB),$0-32
CMPXCHGQ DX, 0(DI)
MOVQ AX, ret+24(FP)
RET
TEXT ·IncUnlessZeroInt32(SB),NOSPLIT,$0-9
MOVQ addr+0(FP), DI
MOVL 0(DI), AX
retry:
TESTL AX, AX
JZ fail
LEAL 1(AX), DX
LOCK
CMPXCHGL DX, 0(DI)
JNZ retry
SETEQ ret+8(FP)
RET
fail:
MOVB AX, ret+8(FP)
RET
TEXT ·DecUnlessOneInt32(SB),NOSPLIT,$0-9
MOVQ addr+0(FP), DI
MOVL 0(DI), AX
retry:
LEAL -1(AX), DX
TESTL DX, DX
JZ fail
LOCK
CMPXCHGL DX, 0(DI)
JNZ retry
SETEQ ret+8(FP)
RET
fail:
MOVB DX, ret+8(FP)
RET
@@ -50,7 +50,6 @@ TEXT ·CompareAndSwapUint32(SB),$0-20
MOVD addr+0(FP), R0
MOVW old+8(FP), R1
MOVW new+12(FP), R2
again:
LDAXRW (R0), R3
CMPW R1, R3
@@ -95,7 +94,6 @@ TEXT ·CompareAndSwapUint64(SB),$0-32
MOVD addr+0(FP), R0
MOVD old+8(FP), R1
MOVD new+16(FP), R2
again:
LDAXR (R0), R3
CMP R1, R3
@@ -105,35 +103,3 @@ again:
done:
MOVD R3, prev+24(FP)
RET
TEXT ·IncUnlessZeroInt32(SB),NOSPLIT,$0-9
MOVD addr+0(FP), R0
again:
LDAXRW (R0), R1
CBZ R1, fail
ADDW $1, R1
STLXRW R1, (R0), R2
CBNZ R2, again
MOVW $1, R2
MOVB R2, ret+8(FP)
RET
fail:
MOVB ZR, ret+8(FP)
RET
TEXT ·DecUnlessOneInt32(SB),NOSPLIT,$0-9
MOVD addr+0(FP), R0
again:
LDAXRW (R0), R1
SUBSW $1, R1, R1
BEQ fail
STLXRW R1, (R0), R2
CBNZ R2, again
MOVW $1, R2
MOVB R2, ret+8(FP)
RET
fail:
MOVB ZR, ret+8(FP)
RET
@@ -20,7 +20,6 @@ import (
"sync/atomic"
)
// AndUint32 atomically applies bitwise and operation to *addr with val.
func AndUint32(addr *uint32, val uint32) {
for {
o := atomic.LoadUint32(addr)
@@ -31,7 +30,6 @@ func AndUint32(addr *uint32, val uint32) {
}
}
// OrUint32 atomically applies bitwise or operation to *addr with val.
func OrUint32(addr *uint32, val uint32) {
for {
o := atomic.LoadUint32(addr)
@@ -42,7 +40,6 @@ func OrUint32(addr *uint32, val uint32) {
}
}
// XorUint32 atomically applies bitwise xor operation to *addr with val.
func XorUint32(addr *uint32, val uint32) {
for {
o := atomic.LoadUint32(addr)
@@ -53,8 +50,6 @@ func XorUint32(addr *uint32, val uint32) {
}
}
// CompareAndSwapUint32 is like sync/atomic.CompareAndSwapUint32, but returns
// the value previously stored at addr.
func CompareAndSwapUint32(addr *uint32, old, new uint32) (prev uint32) {
for {
prev = atomic.LoadUint32(addr)
@@ -67,7 +62,6 @@ func CompareAndSwapUint32(addr *uint32, old, new uint32) (prev uint32) {
}
}
// AndUint64 atomically applies bitwise and operation to *addr with val.
func AndUint64(addr *uint64, val uint64) {
for {
o := atomic.LoadUint64(addr)
@@ -78,7 +72,6 @@ func AndUint64(addr *uint64, val uint64) {
}
}
// OrUint64 atomically applies bitwise or operation to *addr with val.
func OrUint64(addr *uint64, val uint64) {
for {
o := atomic.LoadUint64(addr)
@@ -89,7 +82,6 @@ func OrUint64(addr *uint64, val uint64) {
}
}
// XorUint64 atomically applies bitwise xor operation to *addr with val.
func XorUint64(addr *uint64, val uint64) {
for {
o := atomic.LoadUint64(addr)
@@ -100,8 +92,6 @@ func XorUint64(addr *uint64, val uint64) {
}
}
// CompareAndSwapUint64 is like sync/atomic.CompareAndSwapUint64, but returns
// the value previously stored at addr.
func CompareAndSwapUint64(addr *uint64, old, new uint64) (prev uint64) {
for {
prev = atomic.LoadUint64(addr)
@@ -113,35 +103,3 @@ func CompareAndSwapUint64(addr *uint64, old, new uint64) (prev uint64) {
}
}
}
// IncUnlessZeroInt32 increments the value stored at the given address and
// returns true; unless the value stored in the pointer is zero, in which case
// it is left unmodified and false is returned.
func IncUnlessZeroInt32(addr *int32) bool {
for {
v := atomic.LoadInt32(addr)
if v == 0 {
return false
}
if atomic.CompareAndSwapInt32(addr, v, v+1) {
return true
}
}
}
// DecUnlessOneInt32 decrements the value stored at the given address and
// returns true; unless the value stored in the pointer is 1, in which case it
// is left unmodified and false is returned.
func DecUnlessOneInt32(addr *int32) bool {
for {
v := atomic.LoadInt32(addr)
if v == 1 {
return false
}
if atomic.CompareAndSwapInt32(addr, v, v-1) {
return true
}
}
}
@@ -196,67 +196,3 @@ func TestCompareAndSwapUint64(t *testing.T) {
}
}
}
func TestIncUnlessZeroInt32(t *testing.T) {
for _, test := range []struct {
initial int32
final int32
ret bool
}{
{
initial: 0,
final: 0,
ret: false,
},
{
initial: 1,
final: 2,
ret: true,
},
{
initial: 2,
final: 3,
ret: true,
},
} {
val := test.initial
if got, want := IncUnlessZeroInt32(&val), test.ret; got != want {
t.Errorf("For initial value of %d: incorrect return value: got %v, wanted %v", test.initial, got, want)
}
if got, want := val, test.final; got != want {
t.Errorf("For initial value of %d: incorrect final value: got %d, wanted %d", test.initial, got, want)
}
}
}
func TestDecUnlessOneInt32(t *testing.T) {
for _, test := range []struct {
initial int32
final int32
ret bool
}{
{
initial: 0,
final: -1,
ret: true,
},
{
initial: 1,
final: 1,
ret: false,
},
{
initial: 2,
final: 1,
ret: true,
},
} {
val := test.initial
if got, want := DecUnlessOneInt32(&val), test.ret; got != want {
t.Errorf("For initial value of %d: incorrect return value: got %v, wanted %v", test.initial, got, want)
}
if got, want := val, test.final; got != want {
t.Errorf("For initial value of %d: incorrect final value: got %d, wanted %d", test.initial, got, want)
}
}
}
+18 -5
View File
@@ -18,7 +18,6 @@ import (
"fmt"
"sync/atomic"
"gvisor.dev/gvisor/pkg/atomicbitops"
"gvisor.dev/gvisor/pkg/sentry/platform"
"gvisor.dev/gvisor/pkg/usermem"
)
@@ -42,8 +41,15 @@ func (mm *MemoryManager) AddressSpace() platform.AddressSpace {
func (mm *MemoryManager) Activate() error {
// Fast path: the MemoryManager already has an active
// platform.AddressSpace, and we just need to indicate that we need it too.
if atomicbitops.IncUnlessZeroInt32(&mm.active) {
return nil
for {
active := atomic.LoadInt32(&mm.active)
if active == 0 {
// Fall back to the slow path.
break
}
if atomic.CompareAndSwapInt32(&mm.active, active, active+1) {
return nil
}
}
for {
@@ -118,8 +124,15 @@ func (mm *MemoryManager) Activate() error {
func (mm *MemoryManager) Deactivate() {
// Fast path: this is not the last goroutine to deactivate the
// MemoryManager.
if atomicbitops.DecUnlessOneInt32(&mm.active) {
return
for {
active := atomic.LoadInt32(&mm.active)
if active == 1 {
// Fall back to the slow path.
break
}
if atomic.CompareAndSwapInt32(&mm.active, active, active-1) {
return
}
}
mm.activeMu.Lock()
+9 -2
View File
@@ -18,7 +18,6 @@ import (
"fmt"
"sync/atomic"
"gvisor.dev/gvisor/pkg/atomicbitops"
"gvisor.dev/gvisor/pkg/context"
"gvisor.dev/gvisor/pkg/sentry/arch"
"gvisor.dev/gvisor/pkg/sentry/limits"
@@ -229,7 +228,15 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
// IncUsers increments mm's user count and returns true. If the user count is
// already 0, IncUsers does nothing and returns false.
func (mm *MemoryManager) IncUsers() bool {
return atomicbitops.IncUnlessZeroInt32(&mm.users)
for {
users := atomic.LoadInt32(&mm.users)
if users == 0 {
return false
}
if atomic.CompareAndSwapInt32(&mm.users, users, users+1) {
return true
}
}
}
// DecUsers decrements mm's user count. If the user count reaches 0, all