prohibit direct use of sync/atomic (u)int64 functions

All atomic 64 bit ints are changed to atomicbitops.(Ui|I)nt64. A nogo checker
enforces that sync/atomic 64 bit functions are not called.

For reviewers: the interesting changes are in the atomicbitops and checkaligned
packages.

Why do this?
- It is very easy to accidentally use atomic values without sync/atomic funcs.
- We have checkatomics, but this is optional and is forgotten in several places.
  - Using a type+checker to enforce this seems less error prone and simpler.
- We get NoCopy protection.
- Use of 64 bit atomics can break 32 bit builds. We have types to handle this
  without any runtime cost, so we might as well use them.

PiperOrigin-RevId: 440473398

pkg/atomicbitops/BUILD

@@ -13,6 +13,7 @@ go_library(
         "atomicbitops_noasm.go",
     ],
     visibility = ["//:sandbox"],
+    deps = ["//pkg/sync"],
 )
 
 go_test(

pkg/atomicbitops/aligned_32bit_unsafe.go

@@ -20,9 +20,11 @@ package atomicbitops
 import (
 	"sync/atomic"
 	"unsafe"
+
+	"gvisor.dev/gvisor/pkg/sync"
 )
 
-// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit
+// Int64 is an atomic int64 that is guaranteed to be 64-bit
 // aligned, even on 32-bit systems.
 //
 // Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG:
@@ -33,34 +35,90 @@ import (
 // be 64-bit aligned."
 //
 // +stateify savable
-type AlignedAtomicInt64 struct {
+type Int64 struct {
+	_       sync.NoCopy
 	value   int64
 	value32 int32
 }
 
-func (aa *AlignedAtomicInt64) ptr() *int64 {
-	// On 32-bit systems, aa.value is guaranteed to be 32-bit aligned. It means
-	// that in the 12-byte aa.value, there are guaranteed to be 8 contiguous bytes
+//go:nosplit
+func (i *Int64) ptr() *int64 {
+	// On 32-bit systems, i.value is guaranteed to be 32-bit aligned. It means
+	// that in the 12-byte i.value, there are guaranteed to be 8 contiguous bytes
 	// with 64-bit alignment.
-	return (*int64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 4) &^ 7))
+	return (*int64)(unsafe.Pointer((uintptr(unsafe.Pointer(&i.value)) + 4) &^ 7))
 }
 
-// Load is analagous to atomic.LoadInt64.
-func (aa *AlignedAtomicInt64) Load() int64 {
-	return atomic.LoadInt64(aa.ptr())
+// FromInt64 returns an Int64 initialized to value v.
+//go:nosplit
+func FromInt64(v int64) Int64 {
+	var i Int64
+	*i.ptr() = v
+	return i
 }
 
-// Store is analagous to atomic.StoreInt64.
-func (aa *AlignedAtomicInt64) Store(v int64) {
-	atomic.StoreInt64(aa.ptr(), v)
+// Load is analogous to atomic.LoadInt64.
+//go:nosplit
+func (i *Int64) Load() int64 {
+	return atomic.LoadInt64(i.ptr())
 }
 
-// Add is analagous to atomic.AddInt64.
-func (aa *AlignedAtomicInt64) Add(v int64) int64 {
-	return atomic.AddInt64(aa.ptr(), v)
+// RacyLoad is analogous to reading an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyLoad() int64 {
+	return *i.ptr()
 }
 
-// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit
+// Store is analogous to atomic.StoreInt64.
+//go:nosplit
+func (i *Int64) Store(v int64) {
+	atomic.StoreInt64(i.ptr(), v)
+}
+
+// RacyStore is analogous to setting an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyStore(v int64) {
+	*i.ptr() = v
+}
+
+// Add is analogous to atomic.AddInt64.
+//go:nosplit
+func (i *Int64) Add(v int64) int64 {
+	return atomic.AddInt64(i.ptr(), v)
+}
+
+// RacyAdd is analogous to adding to an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyAdd(v int64) int64 {
+	*i.ptr() += v
+	return *i.ptr()
+}
+
+// Swap is analogous to atomic.SwapInt64.
+//go:nosplit
+func (i *Int64) Swap(v int64) int64 {
+	return atomic.SwapInt64(i.ptr(), v)
+}
+
+// CompareAndSwap is analogous to atomic.CompareAndSwapInt64.
+//go:nosplit
+func (i *Int64) CompareAndSwap(oldVal, newVal int64) bool {
+	return atomic.CompareAndSwapInt64(&i.value, oldVal, newVal)
+}
+
+// Uint64 is an atomic uint64 that is guaranteed to be 64-bit
 // aligned, even on 32-bit systems.
 //
 // Per https://golang.org/pkg/sync/atomic/#pkg-note-BUG:
@@ -71,29 +129,85 @@ func (aa *AlignedAtomicInt64) Add(v int64) int64 {
 // be 64-bit aligned."
 //
 // +stateify savable
-type AlignedAtomicUint64 struct {
+type Uint64 struct {
+	_       sync.NoCopy
 	value   uint64
 	value32 uint32
 }
 
-func (aa *AlignedAtomicUint64) ptr() *uint64 {
-	// On 32-bit systems, aa.value is guaranteed to be 32-bit aligned. It means
-	// that in the 12-byte aa.value, there are guaranteed to be 8 contiguous bytes
+//go:nosplit
+func (u *Uint64) ptr() *uint64 {
+	// On 32-bit systems, i.value is guaranteed to be 32-bit aligned. It means
+	// that in the 12-byte i.value, there are guaranteed to be 8 contiguous bytes
 	// with 64-bit alignment.
-	return (*uint64)(unsafe.Pointer((uintptr(unsafe.Pointer(&aa.value)) + 4) &^ 7))
+	return (*uint64)(unsafe.Pointer((uintptr(unsafe.Pointer(&u.value)) + 4) &^ 7))
 }
 
-// Load is analagous to atomic.LoadUint64.
-func (aa *AlignedAtomicUint64) Load() uint64 {
-	return atomic.LoadUint64(aa.ptr())
+// FromUint64 returns an Uint64 initialized to value v.
+//go:nosplit
+func FromUint64(v uint64) Uint64 {
+	var u Uint64
+	*u.ptr() = v
+	return u
 }
 
-// Store is analagous to atomic.StoreUint64.
-func (aa *AlignedAtomicUint64) Store(v uint64) {
-	atomic.StoreUint64(aa.ptr(), v)
+// Load is analogous to atomic.LoadUint64.
+//go:nosplit
+func (u *Uint64) Load() uint64 {
+	return atomic.LoadUint64(u.ptr())
 }
 
-// Add is analagous to atomic.AddUint64.
-func (aa *AlignedAtomicUint64) Add(v uint64) uint64 {
-	return atomic.AddUint64(aa.ptr(), v)
+// RacyLoad is analogous to reading an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyLoad() uint64 {
+	return *u.ptr()
+}
+
+// Store is analogous to atomic.StoreUint64.
+//go:nosplit
+func (u *Uint64) Store(v uint64) {
+	atomic.StoreUint64(u.ptr(), v)
+}
+
+// RacyStore is analogous to setting an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyStore(v uint64) {
+	*u.ptr() = v
+}
+
+// Add is analogous to atomic.AddUint64.
+//go:nosplit
+func (u *Uint64) Add(v uint64) uint64 {
+	return atomic.AddUint64(u.ptr(), v)
+}
+
+// RacyAdd is analogous to adding to an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyAdd(v uint64) uint64 {
+	*u.ptr() += v
+	return *u.ptr()
+}
+
+// Swap is analogous to atomic.SwapUint64.
+//go:nosplit
+func (u *Uint64) Swap(v uint64) uint64 {
+	return atomic.SwapUint64(u.ptr(), v)
+}
+
+// CompareAndSwap is analogous to atomic.CompareAndSwapUint64.
+//go:nosplit
+func (u *Uint64) CompareAndSwap(oldVal, newVal uint64) bool {
+	return atomic.CompareAndSwapUint64(u.ptr(), oldVal, newVal)
 }

pkg/atomicbitops/aligned_64bit.go

@@ -17,56 +17,178 @@
 
 package atomicbitops
 
-import "sync/atomic"
+import (
+	"sync/atomic"
 
-// AlignedAtomicInt64 is an atomic int64 that is guaranteed to be 64-bit
+	"gvisor.dev/gvisor/pkg/sync"
+)
+
+// Int64 is an atomic int64 that is guaranteed to be 64-bit
 // aligned, even on 32-bit systems. On most architectures, it's just a regular
 // int64.
 //
-// See aligned_unsafe.go in this directory for justification.
+// The default value is zero.
+//
+// See aligned_32bit_unsafe.go in this directory for justification.
 //
 // +stateify savable
-type AlignedAtomicInt64 struct {
+type Int64 struct {
+	_     sync.NoCopy
 	value int64
 }
 
-// Load is analagous to atomic.LoadInt64.
-func (aa *AlignedAtomicInt64) Load() int64 {
-	return atomic.LoadInt64(&aa.value)
+// FromInt64 returns an Int64 initialized to value v.
+//go:nosplit
+func FromInt64(v int64) Int64 {
+	return Int64{value: v}
 }
 
-// Store is analagous to atomic.StoreInt64.
-func (aa *AlignedAtomicInt64) Store(v int64) {
-	atomic.StoreInt64(&aa.value, v)
+// Load is analogous to atomic.LoadInt64.
+//go:nosplit
+func (i *Int64) Load() int64 {
+	return atomic.LoadInt64(&i.value)
 }
 
-// Add is analagous to atomic.AddInt64.
-func (aa *AlignedAtomicInt64) Add(v int64) int64 {
-	return atomic.AddInt64(&aa.value, v)
+// RacyLoad is analogous to reading an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyLoad() int64 {
+	return i.value
 }
 
-// AlignedAtomicUint64 is an atomic uint64 that is guaranteed to be 64-bit
+// Store is analogous to atomic.StoreInt64.
+//go:nosplit
+func (i *Int64) Store(v int64) {
+	atomic.StoreInt64(&i.value, v)
+}
+
+// RacyStore is analogous to setting an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyStore(v int64) {
+	i.value = v
+}
+
+// Add is analogous to atomic.AddInt64.
+//go:nosplit
+func (i *Int64) Add(v int64) int64 {
+	return atomic.AddInt64(&i.value, v)
+}
+
+// RacyAdd is analogous to adding to an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (i *Int64) RacyAdd(v int64) int64 {
+	i.value += v
+	return i.value
+}
+
+// Swap is analogous to atomic.SwapInt64.
+//go:nosplit
+func (i *Int64) Swap(v int64) int64 {
+	return atomic.SwapInt64(&i.value, v)
+}
+
+// CompareAndSwap is analogous to atomic.CompareAndSwapInt64.
+//go:nosplit
+func (i *Int64) CompareAndSwap(oldVal, newVal int64) bool {
+	return atomic.CompareAndSwapInt64(&i.value, oldVal, newVal)
+}
+
+//go:nosplit
+func (i *Int64) ptr() *int64 {
+	return &i.value
+}
+
+// Uint64 is an atomic uint64 that is guaranteed to be 64-bit
 // aligned, even on 32-bit systems. On most architectures, it's just a regular
 // uint64.
 //
 // See aligned_unsafe.go in this directory for justification.
 //
 // +stateify savable
-type AlignedAtomicUint64 struct {
+type Uint64 struct {
+	_     sync.NoCopy
 	value uint64
 }
 
-// Load is analagous to atomic.LoadUint64.
-func (aa *AlignedAtomicUint64) Load() uint64 {
-	return atomic.LoadUint64(&aa.value)
+// FromUint64 returns an Uint64 initialized to value v.
+//go:nosplit
+func FromUint64(v uint64) Uint64 {
+	return Uint64{value: v}
 }
 
-// Store is analagous to atomic.StoreUint64.
-func (aa *AlignedAtomicUint64) Store(v uint64) {
-	atomic.StoreUint64(&aa.value, v)
+// Load is analogous to atomic.LoadUint64.
+//go:nosplit
+func (u *Uint64) Load() uint64 {
+	return atomic.LoadUint64(&u.value)
 }
 
-// Add is analagous to atomic.AddUint64.
-func (aa *AlignedAtomicUint64) Add(v uint64) uint64 {
-	return atomic.AddUint64(&aa.value, v)
+// RacyLoad is analogous to reading an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyLoad() uint64 {
+	return u.value
+}
+
+// Store is analogous to atomic.StoreUint64.
+//go:nosplit
+func (u *Uint64) Store(v uint64) {
+	atomic.StoreUint64(&u.value, v)
+}
+
+// RacyStore is analogous to setting an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyStore(v uint64) {
+	u.value = v
+}
+
+// Add is analogous to atomic.AddUint64.
+//go:nosplit
+func (u *Uint64) Add(v uint64) uint64 {
+	return atomic.AddUint64(&u.value, v)
+}
+
+// RacyAdd is analogous to adding to an atomic value without using
+// synchronization.
+//
+// It may be helpful to document why a racy operation is permitted.
+//
+//go:nosplit
+func (u *Uint64) RacyAdd(v uint64) uint64 {
+	u.value += v
+	return u.value
+}
+
+// Swap is analogous to atomic.SwapUint64.
+//go:nosplit
+func (u *Uint64) Swap(v uint64) uint64 {
+	return atomic.SwapUint64(&u.value, v)
+}
+
+// CompareAndSwap is analogous to atomic.CompareAndSwapUint64.
+//go:nosplit
+func (u *Uint64) CompareAndSwap(oldVal, newVal uint64) bool {
+	return atomic.CompareAndSwapUint64(&u.value, oldVal, newVal)
+}
+
+//go:nosplit
+func (u *Uint64) ptr() *uint64 {
+	return &u.value
 }

pkg/atomicbitops/aligned_test.go

@@ -21,7 +21,7 @@ import (
 func TestAtomiciInt64(t *testing.T) {
 	v := struct {
 		v8  int8
-		v64 AlignedAtomicInt64
+		v64 Int64
 	}{}
 	v.v64.Add(1)
 }
@@ -29,7 +29,7 @@ func TestAtomiciInt64(t *testing.T) {
 func TestAtomicUint64(t *testing.T) {
 	v := struct {
 		v8  uint8
-		v64 AlignedAtomicUint64
+		v64 Uint64
 	}{}
 	v.v64.Add(1)
 }

pkg/atomicbitops/atomicbitops.go

@@ -35,14 +35,30 @@ func XorUint32(addr *uint32, val uint32)
 func CompareAndSwapUint32(addr *uint32, old, new uint32) uint32
 
 // AndUint64 atomically applies bitwise AND operation to *addr with val.
-func AndUint64(addr *uint64, val uint64)
+func AndUint64(addr *Uint64, val uint64) {
+	andUint64(&addr.value, val)
+}
+
+func andUint64(addr *uint64, val uint64)
 
 // OrUint64 atomically applies bitwise OR operation to *addr with val.
-func OrUint64(addr *uint64, val uint64)
+func OrUint64(addr *Uint64, val uint64) {
+	orUint64(&addr.value, val)
+}
+
+func orUint64(addr *uint64, val uint64)
 
 // XorUint64 atomically applies bitwise XOR operation to *addr with val.
-func XorUint64(addr *uint64, val uint64)
+func XorUint64(addr *Uint64, val uint64) {
+	xorUint64(&addr.value, 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
+func CompareAndSwapUint64(addr *Uint64, old, new uint64) uint64 {
+	return compareAndSwapUint64(&addr.value, old, new)
+}
+
+func compareAndSwapUint64(addr *uint64, old, new uint64) uint64

pkg/atomicbitops/atomicbitops_amd64.s

@@ -46,28 +46,28 @@ TEXT ·CompareAndSwapUint32(SB),NOSPLIT,$0-20
   MOVL  AX, ret+16(FP)
   RET
 
-TEXT ·AndUint64(SB),NOSPLIT,$0-16
+TEXT ·andUint64(SB),NOSPLIT,$0-16
   MOVQ  addr+0(FP), BX
   MOVQ  val+8(FP), AX
   LOCK
   ANDQ   AX, 0(BX)
   RET
 
-TEXT ·OrUint64(SB),NOSPLIT,$0-16
+TEXT ·orUint64(SB),NOSPLIT,$0-16
   MOVQ  addr+0(FP), BX
   MOVQ  val+8(FP), AX
   LOCK
   ORQ   AX, 0(BX)
   RET
 
-TEXT ·XorUint64(SB),NOSPLIT,$0-16
+TEXT ·xorUint64(SB),NOSPLIT,$0-16
   MOVQ  addr+0(FP), BX
   MOVQ  val+8(FP), AX
   LOCK
   XORQ   AX, 0(BX)
   RET
 
-TEXT ·CompareAndSwapUint64(SB),NOSPLIT,$0-32
+TEXT ·compareAndSwapUint64(SB),NOSPLIT,$0-32
   MOVQ  addr+0(FP), DI
   MOVQ  old+8(FP), AX
   MOVQ  new+16(FP), DX

pkg/atomicbitops/atomicbitops_arm64.s

@@ -60,7 +60,7 @@ done:
   MOVW R3, prev+16(FP)
   RET
 
-TEXT ·AndUint64(SB),NOSPLIT,$0-16
+TEXT ·andUint64(SB),NOSPLIT,$0-16
   MOVD    ptr+0(FP), R0
   MOVD    val+8(FP), R1
 again:
@@ -70,7 +70,7 @@ again:
   CBNZ    R3, again
   RET
 
-TEXT ·OrUint64(SB),NOSPLIT,$0-16
+TEXT ·orUint64(SB),NOSPLIT,$0-16
   MOVD    ptr+0(FP), R0
   MOVD    val+8(FP), R1
 again:
@@ -80,7 +80,7 @@ again:
   CBNZ    R3, again
   RET
 
-TEXT ·XorUint64(SB),NOSPLIT,$0-16
+TEXT ·xorUint64(SB),NOSPLIT,$0-16
   MOVD    ptr+0(FP), R0
   MOVD    val+8(FP), R1
 again:
@@ -90,7 +90,7 @@ again:
   CBNZ    R3, again
   RET
 
-TEXT ·CompareAndSwapUint64(SB),NOSPLIT,$0-32
+TEXT ·compareAndSwapUint64(SB),NOSPLIT,$0-32
   MOVD addr+0(FP), R0
   MOVD old+8(FP), R1
   MOVD new+16(FP), R2

pkg/atomicbitops/atomicbitops_noasm.go

@@ -68,46 +68,46 @@ func CompareAndSwapUint32(addr *uint32, old, new uint32) (prev uint32) {
 }
 
 //go:nosplit
-func AndUint64(addr *uint64, val uint64) {
+func AndUint64(addr *Uint64, val uint64) {
 	for {
-		o := atomic.LoadUint64(addr)
+		o := atomic.LoadUint64(addr.ptr())
 		n := o & val
-		if atomic.CompareAndSwapUint64(addr, o, n) {
+		if atomic.CompareAndSwapUint64(addr.ptr(), o, n) {
 			break
 		}
 	}
 }
 
 //go:nosplit
-func OrUint64(addr *uint64, val uint64) {
+func OrUint64(addr *Uint64, val uint64) {
 	for {
-		o := atomic.LoadUint64(addr)
+		o := atomic.LoadUint64(addr.ptr())
 		n := o | val
-		if atomic.CompareAndSwapUint64(addr, o, n) {
+		if atomic.CompareAndSwapUint64(addr.ptr(), o, n) {
 			break
 		}
 	}
 }
 
 //go:nosplit
-func XorUint64(addr *uint64, val uint64) {
+func XorUint64(addr *Uint64, val uint64) {
 	for {
-		o := atomic.LoadUint64(addr)
+		o := atomic.LoadUint64(addr.ptr())
 		n := o ^ val
-		if atomic.CompareAndSwapUint64(addr, o, n) {
+		if atomic.CompareAndSwapUint64(addr.ptr(), o, n) {
 			break
 		}
 	}
 }
 
 //go:nosplit
-func CompareAndSwapUint64(addr *uint64, old, new uint64) (prev uint64) {
+func CompareAndSwapUint64(addr *Uint64, old, new uint64) (prev uint64) {
 	for {
-		prev = atomic.LoadUint64(addr)
+		prev = atomic.LoadUint64(addr.ptr())
 		if prev != old {
 			return
 		}
-		if atomic.CompareAndSwapUint64(addr, old, new) {
+		if atomic.CompareAndSwapUint64(addr.ptr(), old, new) {
 			return
 		}
 	}

pkg/atomicbitops/atomicbitops_test.go

@@ -43,20 +43,20 @@ func detectRaces32(val, target uint32, fn func(*uint32, uint32)) bool {
 	return false
 }
 
-func detectRaces64(val, target uint64, fn func(*uint64, uint64)) bool {
+func detectRaces64(val, target uint64, fn func(*Uint64, uint64)) bool {
 	runtime.GOMAXPROCS(100)
 	for n := 0; n < iterations; n++ {
-		x := val
+		x := FromUint64(val)
 		var wg sync.WaitGroup
 		for i := uint64(0); i < 64; i++ {
 			wg.Add(1)
-			go func(a *uint64, i uint64) {
+			go func(a *Uint64, i uint64) {
 				defer wg.Done()
 				fn(a, uint64(1<<i))
 			}(&x, i)
 		}
 		wg.Wait()
-		if x != target {
+		if x != FromUint64(target) {
 			return true
 		}
 	}
@@ -186,12 +186,12 @@ func TestCompareAndSwapUint64(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
-		val := test.prev
+		val := FromUint64(test.prev)
 		prev := CompareAndSwapUint64(&val, test.old, test.new)
 		if got, want := prev, test.prev; got != want {
 			t.Errorf("%s: incorrect returned previous value: got %d, expected %d", test.name, got, want)
 		}
-		if got, want := val, test.next; got != want {
+		if got, want := val.Load(), test.next; got != want {
 			t.Errorf("%s: incorrect value stored in val: got %d, expected %d", test.name, got, want)
 		}
 	}

pkg/fd/BUILD

@@ -6,7 +6,10 @@ go_library(
     name = "fd",
     srcs = ["fd.go"],
     visibility = ["//visibility:public"],
-    deps = ["@org_golang_x_sys//unix:go_default_library"],
+    deps = [
+        "//pkg/atomicbitops",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
 )
 
 go_test(
@@ -14,5 +17,8 @@ go_test(
     size = "small",
     srcs = ["fd_test.go"],
     library = ":fd",
-    deps = ["@org_golang_x_sys//unix:go_default_library"],
+    deps = [
+        "//pkg/atomicbitops",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
 )

pkg/fd/fd.go

@@ -20,16 +20,16 @@ import (
 	"io"
 	"os"
 	"runtime"
-	"sync/atomic"
 
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 )
 
 // ReadWriter implements io.ReadWriter, io.ReaderAt, and io.WriterAt for fd. It
 // does not take ownership of fd.
 type ReadWriter struct {
 	// fd is accessed atomically so FD.Close/Release can swap it.
-	fd int64
+	fd atomicbitops.Int64
 }
 
 var _ io.ReadWriter = (*ReadWriter)(nil)
@@ -38,7 +38,9 @@ var _ io.WriterAt = (*ReadWriter)(nil)
 
 // NewReadWriter creates a ReadWriter for fd.
 func NewReadWriter(fd int) *ReadWriter {
-	return &ReadWriter{int64(fd)}
+	return &ReadWriter{
+		fd: atomicbitops.FromInt64(int64(fd)),
+	}
 }
 
 func fixCount(n int, err error) (int, error) {
@@ -124,7 +126,7 @@ func (r *ReadWriter) WriteAt(b []byte, off int64) (c int, err error) {
 
 // FD returns the owned file descriptor. Ownership remains unchanged.
 func (r *ReadWriter) FD() int {
-	return int(atomic.LoadInt64(&r.fd))
+	return int(r.fd.Load())
 }
 
 // String implements Stringer.String().
@@ -152,9 +154,17 @@ type FD struct {
 // New takes ownership of fd.
 func New(fd int) *FD {
 	if fd < 0 {
-		return &FD{ReadWriter{-1}}
+		return &FD{
+			ReadWriter: ReadWriter{
+				fd: atomicbitops.FromInt64(-1),
+			},
+		}
+	}
+	f := &FD{
+		ReadWriter: ReadWriter{
+			fd: atomicbitops.FromInt64(int64(fd)),
+		},
 	}
-	f := &FD{ReadWriter{int64(fd)}}
 	runtime.SetFinalizer(f, (*FD).Close)
 	return f
 }
@@ -173,7 +183,11 @@ func NewFromFile(file *os.File) (*FD, error) {
 	// Fd() returns.
 	runtime.KeepAlive(file)
 	if err != nil {
-		return &FD{ReadWriter{-1}}, err
+		return &FD{
+			ReadWriter: ReadWriter{
+				fd: atomicbitops.FromInt64(-1),
+			},
+		}, err
 	}
 	return New(fd), nil
 }
@@ -221,7 +235,7 @@ func OpenAt(dir *FD, path string, flags int, mode uint32) (*FD, error) {
 // Concurrently calling Close and any other method is undefined.
 func (f *FD) Close() error {
 	runtime.SetFinalizer(f, nil)
-	return unix.Close(int(atomic.SwapInt64(&f.fd, -1)))
+	return unix.Close(int(f.fd.Swap(-1)))
 }
 
 // Release relinquishes ownership of the contained file descriptor.
@@ -229,7 +243,7 @@ func (f *FD) Close() error {
 // Concurrently calling Release and any other method is undefined.
 func (f *FD) Release() int {
 	runtime.SetFinalizer(f, nil)
-	return int(atomic.SwapInt64(&f.fd, -1))
+	return int(f.fd.Swap(-1))
 }
 
 // File converts the FD to an os.File.

pkg/fd/fd_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 )
 
 func TestSetNegOne(t *testing.T) {
@@ -109,7 +110,7 @@ func TestFileDotFile(t *testing.T) {
 }
 
 func TestFileDotFileError(t *testing.T) {
-	f := &FD{ReadWriter{-2}}
+	f := &FD{ReadWriter{atomicbitops.FromInt64(-2)}}
 
 	if of, err := f.File(); err == nil {
 		t.Errorf("File %v got nil err want non-nil", of)

pkg/metric/BUILD

@@ -11,6 +11,7 @@ go_library(
     visibility = ["//:sandbox"],
     deps = [
         ":metric_go_proto",
+        "//pkg/atomicbitops",
         "//pkg/eventchannel",
         "//pkg/gohacks",
         "//pkg/log",

pkg/metric/metric.go

@@ -21,10 +21,10 @@ import (
 	"math"
 	"sort"
 	"strings"
-	"sync/atomic"
 	"time"
 
 	"google.golang.org/protobuf/types/known/timestamppb"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/eventchannel"
 	"gvisor.dev/gvisor/pkg/log"
 	pb "gvisor.dev/gvisor/pkg/metric/metric_go_proto"
@@ -91,7 +91,7 @@ var (
 // TODO(b/67298427): Support metric fields.
 type Uint64Metric struct {
 	// value is the actual value of the metric. It must be accessed atomically.
-	value uint64
+	value atomicbitops.Uint64
 
 	// numFields is the number of metric fields. It is immutable once
 	// initialized.
@@ -432,7 +432,7 @@ func (m *Uint64Metric) Value(fieldValues ...string) uint64 {
 
 	switch m.numFields {
 	case 0:
-		return atomic.LoadUint64(&m.value)
+		return m.value.Load()
 	case 1:
 		m.mu.RLock()
 		defer m.mu.RUnlock()
@@ -460,7 +460,7 @@ func (m *Uint64Metric) IncrementBy(v uint64, fieldValues ...string) {
 
 	switch m.numFields {
 	case 0:
-		atomic.AddUint64(&m.value, v)
+		m.value.Add(v)
 		return
 	case 1:
 		fieldValue := fieldValues[0]
@@ -643,7 +643,7 @@ type DistributionMetric struct {
 	// (i-1)-th finite bucket.
 	// The last value is the number of samples that fell into the bucketer's
 	// last (i.e. infinite) bucket.
-	samples map[string][]uint64
+	samples map[string][]atomicbitops.Uint64
 }
 
 // NewDistributionMetric creates and registers a new distribution metric.
@@ -669,10 +669,10 @@ func NewDistributionMetric(name string, sync bool, bucketer Bucketer, unit pb.Me
 		return nil, err
 	}
 	allKeys := fieldsToKey.all()
-	samples := make(map[string][]uint64, len(allKeys))
+	samples := make(map[string][]atomicbitops.Uint64, len(allKeys))
 	numFiniteBuckets := bucketer.NumFiniteBuckets()
 	for _, key := range allKeys {
-		samples[key] = make([]uint64, numFiniteBuckets+2)
+		samples[key] = make([]atomicbitops.Uint64, numFiniteBuckets+2)
 	}
 	protoFields := make([]*pb.MetricMetadata_Field, len(fields))
 	for i, f := range fields {
@@ -723,7 +723,7 @@ func (d *DistributionMetric) AddSample(sample int64, fields ...string) {
 //go:nosplit
 func (d *DistributionMetric) addSampleByKey(sample int64, key string) {
 	bucket := d.exponentialBucketer.BucketIndex(sample)
-	atomic.AddUint64(&d.samples[key][bucket+1], 1)
+	d.samples[key][bucket+1].Add(1)
 }
 
 // Minimum number of buckets for NewDurationBucket.

pkg/metric/metric_unsafe.go

@@ -17,6 +17,7 @@ package metric
 import (
 	"unsafe"
 
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/gohacks"
 	"gvisor.dev/gvisor/pkg/sync"
 )
@@ -27,7 +28,7 @@ import (
 // inconsistency (i.e. increments that race with the snapshot) will simply be
 // detected during the next snapshot instead. Reading them consistently would
 // require more synchronization during increments, which we need to be cheap.
-func snapshotDistribution(samples []uint64) []uint64 {
+func snapshotDistribution(samples []atomicbitops.Uint64) []uint64 {
 	// The number of buckets within a distribution never changes, so there is
 	// no race condition from getting the number of buckets upfront.
 	numBuckets := len(samples)
@@ -40,8 +41,9 @@ func snapshotDistribution(samples []uint64) []uint64 {
 		// not instrumented by the race detector.
 		gohacks.Memmove(snapshotHeader.Data, samplesHeader.Data, unsafe.Sizeof(uint64(0))*uintptr(numBuckets))
 	} else {
-		// Just use copy.
-		copy(snapshot, samples)
+		for i := range samples {
+			snapshot[i] = samples[i].RacyLoad()
+		}
 	}
 	return snapshot
 }

pkg/p9/BUILD

@@ -23,6 +23,7 @@ go_library(
     ],
     deps = [
         "//pkg/abi/linux",
+        "//pkg/atomicbitops",
         "//pkg/errors",
         "//pkg/errors/linuxerr",
         "//pkg/fd",

pkg/p9/handlers.go

@@ -24,6 +24,7 @@ import (
 	"sync/atomic"
 
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/errors"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/fd"
@@ -299,7 +300,7 @@ func (t *Tattach) handle(cs *connState) message {
 		server:   cs.server,
 		parent:   nil,
 		file:     sf,
-		refs:     1,
+		refs:     atomicbitops.FromInt64(1),
 		mode:     attr.Mode.FileType(),
 		pathNode: cs.server.pathTree,
 	}

pkg/p9/p9.go

@@ -20,11 +20,11 @@ import (
 	"math"
 	"os"
 	"strings"
-	"sync/atomic"
 	"syscall"
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 )
 
 // OpenFlags is the mode passed to Open and Create operations.
@@ -511,7 +511,7 @@ func (q *QID) encode(b *buffer) {
 type QIDGenerator struct {
 	// uids is an ever increasing value that can be atomically incremented
 	// to provide unique Path values for QIDs.
-	uids uint64
+	uids atomicbitops.Uint64
 }
 
 // Get returns a new 9P unique ID with a unique Path given a QID type.
@@ -523,7 +523,7 @@ func (q *QIDGenerator) Get(t QIDType) QID {
 	return QID{
 		Type:    t,
 		Version: 0,
-		Path:    atomic.AddUint64(&q.uids, 1),
+		Path:    q.uids.Add(1),
 	}
 }
 

pkg/p9/p9test/BUILD

@@ -65,6 +65,7 @@ go_library(
     ],
     visibility = ["//:sandbox"],
     deps = [
+        "//pkg/atomicbitops",
         "//pkg/fd",
         "//pkg/log",
         "//pkg/p9",

pkg/p9/p9test/p9test.go

@@ -17,11 +17,11 @@ package p9test
 
 import (
 	"fmt"
-	"sync/atomic"
 	"testing"
 
 	"github.com/golang/mock/gomock"
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/p9"
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/unet"
@@ -39,11 +39,11 @@ type Harness struct {
 }
 
 // globalPath is a QID.Path Generator.
-var globalPath uint64
+var globalPath atomicbitops.Uint64
 
 // MakePath returns a globally unique path.
 func MakePath() uint64 {
-	return atomic.AddUint64(&globalPath, 1)
+	return globalPath.Add(1)
 }
 
 // Generator is a function that generates a new file.