diff --git a/pkg/atomicbitops/BUILD b/pkg/atomicbitops/BUILD index 02c0e52b9..10abe2451 100644 --- a/pkg/atomicbitops/BUILD +++ b/pkg/atomicbitops/BUILD @@ -13,6 +13,7 @@ go_library( "atomicbitops_noasm.go", ], visibility = ["//:sandbox"], + deps = ["//pkg/sync"], ) go_test( diff --git a/pkg/atomicbitops/aligned_32bit_unsafe.go b/pkg/atomicbitops/aligned_32bit_unsafe.go index a143c027d..65f02b0be 100644 --- a/pkg/atomicbitops/aligned_32bit_unsafe.go +++ b/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) } diff --git a/pkg/atomicbitops/aligned_64bit.go b/pkg/atomicbitops/aligned_64bit.go index 634f0ed2c..f04d7a64b 100644 --- a/pkg/atomicbitops/aligned_64bit.go +++ b/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 } diff --git a/pkg/atomicbitops/aligned_test.go b/pkg/atomicbitops/aligned_test.go index e7123d2b8..886efb772 100644 --- a/pkg/atomicbitops/aligned_test.go +++ b/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) } diff --git a/pkg/atomicbitops/atomicbitops.go b/pkg/atomicbitops/atomicbitops.go index 4c4606a58..63ab8c3cc 100644 --- a/pkg/atomicbitops/atomicbitops.go +++ b/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 diff --git a/pkg/atomicbitops/atomicbitops_amd64.s b/pkg/atomicbitops/atomicbitops_amd64.s index 6b9a67adc..5df9134ea 100644 --- a/pkg/atomicbitops/atomicbitops_amd64.s +++ b/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 diff --git a/pkg/atomicbitops/atomicbitops_arm64.s b/pkg/atomicbitops/atomicbitops_arm64.s index 644a6bca5..cffa38347 100644 --- a/pkg/atomicbitops/atomicbitops_arm64.s +++ b/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 diff --git a/pkg/atomicbitops/atomicbitops_noasm.go b/pkg/atomicbitops/atomicbitops_noasm.go index af6b1362e..6ed68ebec 100644 --- a/pkg/atomicbitops/atomicbitops_noasm.go +++ b/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 } } diff --git a/pkg/atomicbitops/atomicbitops_test.go b/pkg/atomicbitops/atomicbitops_test.go index 73af71bb4..0f908175b 100644 --- a/pkg/atomicbitops/atomicbitops_test.go +++ b/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< 0 && !f.flags.NonSeekable { - atomic.AddInt64(&f.offset, n) + f.offset.Add(n) } return n, err } @@ -277,7 +277,7 @@ func (f *File) Writev(ctx context.Context, src usermem.IOSequence) (int64, error } // Enforce file limits. - limit, ok := f.checkLimit(ctx, f.offset) + limit, ok := f.checkLimit(ctx, f.offset.RacyLoad()) switch { case ok && limit == 0: unlockAppendMu() @@ -287,9 +287,9 @@ func (f *File) Writev(ctx context.Context, src usermem.IOSequence) (int64, error } // We must hold the lock during the write. - n, err := f.FileOperations.Write(ctx, f, src, f.offset) + n, err := f.FileOperations.Write(ctx, f, src, f.offset.RacyLoad()) if n >= 0 && !f.flags.NonSeekable { - atomic.StoreInt64(&f.offset, f.offset+n) + f.offset.Store(f.offset.RacyLoad() + n) } unlockAppendMu() return n, err @@ -309,7 +309,8 @@ func (f *File) Pwritev(ctx context.Context, src usermem.IOSequence, offset int64 unlockAppendMu := f.Dirent.Inode.lockAppendMu(f.Flags().Append) defer unlockAppendMu() if f.Flags().Append { - if err := f.offsetForAppend(ctx, &offset); err != nil { + off := atomicbitops.FromInt64(offset) + if err := f.offsetForAppend(ctx, &off); err != nil { return 0, err } } @@ -330,7 +331,7 @@ func (f *File) Pwritev(ctx context.Context, src usermem.IOSequence, offset int64 // // Precondition: the file.Dirent.Inode.appendMu mutex should be held for // writing. -func (f *File) offsetForAppend(ctx context.Context, offset *int64) error { +func (f *File) offsetForAppend(ctx context.Context, offset *atomicbitops.Int64) error { uattr, err := f.Dirent.Inode.UnstableAttr(ctx) if err != nil { // This is an odd error, we treat it as evidence that @@ -339,7 +340,7 @@ func (f *File) offsetForAppend(ctx context.Context, offset *int64) error { } // Update the offset. - atomic.StoreInt64(offset, uattr.Size) + offset.Store(uattr.Size) return nil } diff --git a/pkg/sentry/fs/host/BUILD b/pkg/sentry/fs/host/BUILD index 921612e9c..27feec9ab 100644 --- a/pkg/sentry/fs/host/BUILD +++ b/pkg/sentry/fs/host/BUILD @@ -27,6 +27,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fd", diff --git a/pkg/sentry/fs/host/socket.go b/pkg/sentry/fs/host/socket.go index 37c876505..d354f8d6c 100644 --- a/pkg/sentry/fs/host/socket.go +++ b/pkg/sentry/fs/host/socket.go @@ -16,10 +16,10 @@ package host import ( "fmt" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fd" @@ -62,7 +62,7 @@ type ConnectedEndpoint struct { // GetSockOpt and message splitting/rejection in SendMsg, but do not // prevent lots of small messages from filling the real send buffer // size on the host. - sndbuf int64 `state:"nosave"` + sndbuf atomicbitops.Int64 `state:"nosave"` // mu protects the fields below. mu sync.RWMutex `state:"nosave"` @@ -100,7 +100,7 @@ func (c *ConnectedEndpoint) init() *syserr.Error { } c.stype = linux.SockType(stype) - c.sndbuf = int64(sndbuf) + c.sndbuf = atomicbitops.FromInt64(int64(sndbuf)) return nil } @@ -363,14 +363,14 @@ func (c *ConnectedEndpoint) RecvQueuedSize() int64 { // SendMaxQueueSize implements transport.Receiver.SendMaxQueueSize. func (c *ConnectedEndpoint) SendMaxQueueSize() int64 { - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // RecvMaxQueueSize implements transport.Receiver.RecvMaxQueueSize. func (c *ConnectedEndpoint) RecvMaxQueueSize() int64 { // N.B. Unix sockets don't use the receive buffer. We'll claim it is // the same size as the send buffer. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // Release implements transport.ConnectedEndpoint.Release and transport.Receiver.Release. @@ -385,7 +385,7 @@ func (c *ConnectedEndpoint) CloseUnread() {} func (c *ConnectedEndpoint) SetSendBufferSize(v int64) (newSz int64) { // gVisor does not permit setting of SO_SNDBUF for host backed unix // domain sockets. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // SetReceiveBufferSize implements transport.ConnectedEndpoint.SetReceiveBufferSize. @@ -393,7 +393,7 @@ func (c *ConnectedEndpoint) SetReceiveBufferSize(v int64) (newSz int64) { // gVisor does not permit setting of SO_RCVBUF for host backed unix // domain sockets. Receive buffer does not have any effect for unix // sockets and we claim to be the same as send buffer. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // LINT.ThenChange(../../socket/unix/transport/host.go) diff --git a/pkg/sentry/fs/mount.go b/pkg/sentry/fs/mount.go index ee69b10e8..0f380e4e8 100644 --- a/pkg/sentry/fs/mount.go +++ b/pkg/sentry/fs/mount.go @@ -17,8 +17,8 @@ package fs import ( "bytes" "fmt" - "sync/atomic" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/refs" ) @@ -124,7 +124,7 @@ type MountSource struct { // walks to Dirents in this MountSource. // // direntRefs must be atomically changed. - direntRefs uint64 + direntRefs atomicbitops.Uint64 } // DefaultDirentCacheSize is the number of Dirents that the VFS can hold an @@ -150,17 +150,17 @@ func NewMountSource(ctx context.Context, mops MountSourceOperations, filesystem // DirentRefs returns the current mount direntRefs. func (msrc *MountSource) DirentRefs() uint64 { - return atomic.LoadUint64(&msrc.direntRefs) + return msrc.direntRefs.Load() } // IncDirentRefs increases direntRefs. func (msrc *MountSource) IncDirentRefs() { - atomic.AddUint64(&msrc.direntRefs, 1) + msrc.direntRefs.Add(1) } // DecDirentRefs decrements direntRefs. func (msrc *MountSource) DecDirentRefs() { - if atomic.AddUint64(&msrc.direntRefs, ^uint64(0)) == ^uint64(0) { + if msrc.direntRefs.Add(^uint64(0)) == ^uint64(0) { panic("Decremented zero mount reference direntRefs") } } diff --git a/pkg/sentry/fs/proc/task.go b/pkg/sentry/fs/proc/task.go index 03f2a882d..c7f7aed50 100644 --- a/pkg/sentry/fs/proc/task.go +++ b/pkg/sentry/fs/proc/task.go @@ -774,13 +774,13 @@ func (i *ioData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) ([]se io.Accumulate(i.IOUsage()) var buf bytes.Buffer - fmt.Fprintf(&buf, "rchar: %d\n", io.CharsRead) - fmt.Fprintf(&buf, "wchar: %d\n", io.CharsWritten) - fmt.Fprintf(&buf, "syscr: %d\n", io.ReadSyscalls) - fmt.Fprintf(&buf, "syscw: %d\n", io.WriteSyscalls) - fmt.Fprintf(&buf, "read_bytes: %d\n", io.BytesRead) - fmt.Fprintf(&buf, "write_bytes: %d\n", io.BytesWritten) - fmt.Fprintf(&buf, "cancelled_write_bytes: %d\n", io.BytesWriteCancelled) + fmt.Fprintf(&buf, "rchar: %d\n", io.CharsRead.Load()) + fmt.Fprintf(&buf, "wchar: %d\n", io.CharsWritten.Load()) + fmt.Fprintf(&buf, "syscr: %d\n", io.ReadSyscalls.Load()) + fmt.Fprintf(&buf, "syscw: %d\n", io.WriteSyscalls.Load()) + fmt.Fprintf(&buf, "read_bytes: %d\n", io.BytesRead.Load()) + fmt.Fprintf(&buf, "write_bytes: %d\n", io.BytesWritten.Load()) + fmt.Fprintf(&buf, "cancelled_write_bytes: %d\n", io.BytesWriteCancelled.Load()) return []seqfile.SeqData{{Buf: buf.Bytes(), Handle: (*ioData)(nil)}}, 0 } diff --git a/pkg/sentry/fs/splice.go b/pkg/sentry/fs/splice.go index 474b8ddde..b6e1788b5 100644 --- a/pkg/sentry/fs/splice.go +++ b/pkg/sentry/fs/splice.go @@ -16,8 +16,8 @@ package fs import ( "io" - "sync/atomic" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" ) @@ -67,16 +67,16 @@ func Splice(ctx context.Context, dst *File, src *File, opts SpliceOpts) (int64, srcLock = false } // Use both offsets (locked). - opts.DstStart = dst.offset - opts.SrcStart = src.offset + opts.DstStart = dst.offset.RacyLoad() + opts.SrcStart = src.offset.RacyLoad() case dstLock: // Acquire only dst. dst.mu.Lock() - opts.DstStart = dst.offset // Safe: locked. + opts.DstStart = dst.offset.RacyLoad() // Safe: locked. case srcLock: // Acquire only src. src.mu.Lock() - opts.SrcStart = src.offset // Safe: locked. + opts.SrcStart = src.offset.RacyLoad() // Safe: locked. } var err error @@ -85,7 +85,10 @@ func Splice(ctx context.Context, dst *File, src *File, opts SpliceOpts) (int64, defer unlock() // Figure out the appropriate offset to use. - err = dst.offsetForAppend(ctx, &opts.DstStart) + + dstStart := atomicbitops.FromInt64(opts.DstStart) + err = dst.offsetForAppend(ctx, &dstStart) + opts.DstStart = dstStart.RacyLoad() } if err == nil && !dstPipe { // Enforce file limits. @@ -147,10 +150,10 @@ func Splice(ctx context.Context, dst *File, src *File, opts SpliceOpts) (int64, // Update offsets, if required. if n > 0 { if !dstPipe && !opts.DstOffset { - atomic.StoreInt64(&dst.offset, dst.offset+n) + dst.offset.Add(n) } if !srcPipe && !opts.SrcOffset { - atomic.StoreInt64(&src.offset, src.offset+n) + src.offset.Add(n) } } diff --git a/pkg/sentry/fs/timerfd/BUILD b/pkg/sentry/fs/timerfd/BUILD index e61115932..13d2c4106 100644 --- a/pkg/sentry/fs/timerfd/BUILD +++ b/pkg/sentry/fs/timerfd/BUILD @@ -7,6 +7,7 @@ go_library( srcs = ["timerfd.go"], visibility = ["//pkg/sentry:internal"], deps = [ + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/fs/timerfd/timerfd.go b/pkg/sentry/fs/timerfd/timerfd.go index 4c0744d8c..5b561828f 100644 --- a/pkg/sentry/fs/timerfd/timerfd.go +++ b/pkg/sentry/fs/timerfd/timerfd.go @@ -17,8 +17,7 @@ package timerfd import ( - "sync/atomic" - + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -49,7 +48,7 @@ type TimerOperations struct { // val is the number of timer expirations since the last successful call to // Readv, Preadv, or SetTime. val is accessed using atomic memory // operations. - val uint64 + val atomicbitops.Uint64 } // NewFile returns a timerfd File that receives time from c. @@ -95,13 +94,13 @@ func (t *TimerOperations) GetTime() (ktime.Time, ktime.Setting) { // of expirations to 0, and returns the previous setting and the time at which // it was observed. func (t *TimerOperations) SetTime(s ktime.Setting) (ktime.Time, ktime.Setting) { - return t.timer.SwapAnd(s, func() { atomic.StoreUint64(&t.val, 0) }) + return t.timer.SwapAnd(s, func() { t.val.Store(0) }) } // Readiness implements waiter.Waitable.Readiness. func (t *TimerOperations) Readiness(mask waiter.EventMask) waiter.EventMask { var ready waiter.EventMask - if atomic.LoadUint64(&t.val) != 0 { + if t.val.Load() != 0 { ready |= waiter.ReadableEvents } return ready @@ -124,7 +123,7 @@ func (t *TimerOperations) Read(ctx context.Context, file *fs.File, dst usermem.I if dst.NumBytes() < sizeofUint64 { return 0, linuxerr.EINVAL } - if val := atomic.SwapUint64(&t.val, 0); val != 0 { + if val := t.val.Swap(0); val != 0 { var buf [sizeofUint64]byte hostarch.ByteOrder.PutUint64(buf[:], val) if _, err := dst.CopyOut(ctx, buf[:]); err != nil { @@ -144,7 +143,7 @@ func (t *TimerOperations) Write(context.Context, *fs.File, usermem.IOSequence, i // NotifyTimer implements ktime.TimerListener.NotifyTimer. func (t *TimerOperations) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { - atomic.AddUint64(&t.val, exp) + t.val.Add(exp) t.events.Notify(waiter.ReadableEvents) return ktime.Setting{}, false } diff --git a/pkg/sentry/fsimpl/cgroupfs/base.go b/pkg/sentry/fsimpl/cgroupfs/base.go index 9d039ac11..c4c3b2a14 100644 --- a/pkg/sentry/fsimpl/cgroupfs/base.go +++ b/pkg/sentry/fsimpl/cgroupfs/base.go @@ -20,7 +20,6 @@ import ( "sort" "strconv" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -75,7 +74,7 @@ func (c *controllerCommon) HierarchyID() uint32 { // NumCgroups implements kernel.CgroupController.NumCgroups. func (c *controllerCommon) NumCgroups() uint64 { - return atomic.LoadUint64(&c.fs.numCgroups) + return c.fs.numCgroups.Load() } // Enabled implements kernel.CgroupController.Enabled. @@ -194,7 +193,7 @@ func (fs *filesystem) newCgroupInode(ctx context.Context, creds *auth.Credential c.dir.OrderedChildren.Init(kernfs.OrderedChildrenOptions{Writable: true}) c.dir.IncLinks(c.dir.OrderedChildren.Populate(contents)) - atomic.AddUint64(&fs.numCgroups, 1) + fs.numCgroups.Add(1) return c } diff --git a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go index 5ba44d6e0..1a4124b7d 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go +++ b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go @@ -62,9 +62,9 @@ import ( "sort" "strconv" "strings" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -140,7 +140,7 @@ type filesystem struct { controllers []controller kcontrollers []kernel.CgroupController - numCgroups uint64 // Protected by atomic ops. + numCgroups atomicbitops.Uint64 // Protected by atomic ops. root *kernfs.Dentry // effectiveRoot is the initial cgroup new tasks are created in. Unless @@ -610,12 +610,12 @@ type stubControllerFile struct { controllerFile // data is accessed through atomic ops. - data *int64 + data *atomicbitops.Int64 } // Generate implements vfs.DynamicBytesSource.Generate. func (f *stubControllerFile) Generate(ctx context.Context, buf *bytes.Buffer) error { - fmt.Fprintf(buf, "%d\n", atomic.LoadInt64(f.data)) + fmt.Fprintf(buf, "%d\n", f.data.Load()) return nil } @@ -625,13 +625,13 @@ func (f *stubControllerFile) Write(ctx context.Context, _ *vfs.FileDescription, if err != nil { return 0, err } - atomic.StoreInt64(f.data, val) + f.data.Store(val) return n, nil } -// newStubControllerFile creates a new stub controller file tbat loads and +// newStubControllerFile creates a new stub controller file that loads and // stores a control value from data. -func (fs *filesystem) newStubControllerFile(ctx context.Context, creds *auth.Credentials, data *int64) kernfs.Inode { +func (fs *filesystem) newStubControllerFile(ctx context.Context, creds *auth.Credentials, data *atomicbitops.Int64) kernfs.Inode { f := &stubControllerFile{ data: data, } diff --git a/pkg/sentry/fsimpl/cgroupfs/cpu.go b/pkg/sentry/fsimpl/cgroupfs/cpu.go index 5a8f1c9e5..e7ab6192e 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cpu.go +++ b/pkg/sentry/fsimpl/cgroupfs/cpu.go @@ -15,6 +15,7 @@ package cgroupfs import ( + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -26,11 +27,11 @@ type cpuController struct { controllerStateless // CFS bandwidth control parameters, values in microseconds. - cfsPeriod int64 - cfsQuota int64 + cfsPeriod atomicbitops.Int64 + cfsQuota atomicbitops.Int64 // CPU shares, values should be (num core * 1024). - shares int64 + shares atomicbitops.Int64 } var _ controller = (*cpuController)(nil) @@ -38,21 +39,21 @@ var _ controller = (*cpuController)(nil) func newCPUController(fs *filesystem, defaults map[string]int64) *cpuController { // Default values for controller parameters from Linux. c := &cpuController{ - cfsPeriod: 100000, - cfsQuota: -1, - shares: 1024, + cfsPeriod: atomicbitops.FromInt64(100000), + cfsQuota: atomicbitops.FromInt64(-1), + shares: atomicbitops.FromInt64(1024), } if val, ok := defaults["cpu.cfs_period_us"]; ok { - c.cfsPeriod = val + c.cfsPeriod = atomicbitops.FromInt64(val) delete(defaults, "cpu.cfs_period_us") } if val, ok := defaults["cpu.cfs_quota_us"]; ok { - c.cfsQuota = val + c.cfsQuota = atomicbitops.FromInt64(val) delete(defaults, "cpu.cfs_quota_us") } if val, ok := defaults["cpu.shares"]; ok { - c.shares = val + c.shares = atomicbitops.FromInt64(val) delete(defaults, "cpu.shares") } @@ -63,9 +64,9 @@ func newCPUController(fs *filesystem, defaults map[string]int64) *cpuController // Clone implements controller.Clone. func (c *cpuController) Clone() controller { new := &cpuController{ - cfsPeriod: c.cfsPeriod, - cfsQuota: c.cfsQuota, - shares: c.shares, + cfsPeriod: atomicbitops.FromInt64(c.cfsPeriod.Load()), + cfsQuota: atomicbitops.FromInt64(c.cfsQuota.Load()), + shares: atomicbitops.FromInt64(c.shares.Load()), } new.controllerCommon.cloneFromParent(c) return new diff --git a/pkg/sentry/fsimpl/cgroupfs/job.go b/pkg/sentry/fsimpl/cgroupfs/job.go index 1cdc3fe64..122723aaf 100644 --- a/pkg/sentry/fsimpl/cgroupfs/job.go +++ b/pkg/sentry/fsimpl/cgroupfs/job.go @@ -15,6 +15,7 @@ package cgroupfs import ( + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -25,7 +26,7 @@ type jobController struct { controllerCommon controllerStateless - id int64 + id atomicbitops.Int64 } var _ controller = (*jobController)(nil) @@ -39,7 +40,7 @@ func newJobController(fs *filesystem) *jobController { // Clone implements controller.Clone. func (c *jobController) Clone() controller { new := &jobController{ - id: c.id, + id: atomicbitops.FromInt64(c.id.Load()), } new.controllerCommon.cloneFromParent(c) return new diff --git a/pkg/sentry/fsimpl/cgroupfs/memory.go b/pkg/sentry/fsimpl/cgroupfs/memory.go index 8bcde8cb8..36b2b6a34 100644 --- a/pkg/sentry/fsimpl/cgroupfs/memory.go +++ b/pkg/sentry/fsimpl/cgroupfs/memory.go @@ -20,6 +20,7 @@ import ( "math" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel" @@ -32,9 +33,9 @@ type memoryController struct { controllerCommon controllerStateless - limitBytes int64 - softLimitBytes int64 - moveChargeAtImmigrate int64 + limitBytes atomicbitops.Int64 + softLimitBytes atomicbitops.Int64 + moveChargeAtImmigrate atomicbitops.Int64 pressureLevel int64 } @@ -46,13 +47,13 @@ func newMemoryController(fs *filesystem, defaults map[string]int64) *memoryContr // which is ~ 2**63 on a 64-bit system. So essentially, inifinity. The // exact value isn't very important. - limitBytes: math.MaxInt64, - softLimitBytes: math.MaxInt64, + limitBytes: atomicbitops.FromInt64(math.MaxInt64), + softLimitBytes: atomicbitops.FromInt64(math.MaxInt64), } - consumeDefault := func(name string, valPtr *int64) { + consumeDefault := func(name string, valPtr *atomicbitops.Int64) { if val, ok := defaults[name]; ok { - *valPtr = val + valPtr.Store(val) delete(defaults, name) } } @@ -68,9 +69,9 @@ func newMemoryController(fs *filesystem, defaults map[string]int64) *memoryContr // Clone implements controller.Clone. func (c *memoryController) Clone() controller { new := &memoryController{ - limitBytes: c.limitBytes, - softLimitBytes: c.softLimitBytes, - moveChargeAtImmigrate: c.moveChargeAtImmigrate, + limitBytes: atomicbitops.FromInt64(c.limitBytes.Load()), + softLimitBytes: atomicbitops.FromInt64(c.softLimitBytes.Load()), + moveChargeAtImmigrate: atomicbitops.FromInt64(c.moveChargeAtImmigrate.Load()), } new.controllerCommon.cloneFromParent(c) return new diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index 5e29a6c4b..e1371e566 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -18,6 +18,7 @@ import ( "sync" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/log" @@ -52,7 +53,7 @@ type connection struct { mu sync.Mutex `state:"nosave"` // attributeVersion is the version of connection's attributes. - attributeVersion uint64 + attributeVersion atomicbitops.Uint64 // We target FUSE 7.23. // The following FUSE_INIT flags are currently unsupported by this implementation: diff --git a/pkg/sentry/fsimpl/fuse/directory.go b/pkg/sentry/fsimpl/fuse/directory.go index 9611edd5a..0cd9234de 100644 --- a/pkg/sentry/fsimpl/fuse/directory.go +++ b/pkg/sentry/fsimpl/fuse/directory.go @@ -15,8 +15,6 @@ package fuse import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -62,7 +60,7 @@ func (dir *directoryFD) IterDirents(ctx context.Context, callback vfs.IterDirent in := linux.FUSEReadIn{ Fh: dir.Fh, - Offset: uint64(atomic.LoadInt64(&dir.off)), + Offset: uint64(dir.off.Load()), Size: linux.FUSE_PAGE_SIZE, Flags: dir.statusFlags(), } @@ -94,7 +92,7 @@ func (dir *directoryFD) IterDirents(ctx context.Context, callback vfs.IterDirent if err := callback.Handle(dirent); err != nil { return err } - atomic.StoreInt64(&dir.off, nextOff) + dir.off.Store(nextOff) } return nil diff --git a/pkg/sentry/fsimpl/fuse/file.go b/pkg/sentry/fsimpl/fuse/file.go index b09c82651..0ef1fcc85 100644 --- a/pkg/sentry/fsimpl/fuse/file.go +++ b/pkg/sentry/fsimpl/fuse/file.go @@ -16,6 +16,7 @@ package fuse import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" @@ -45,7 +46,7 @@ type fileDescription struct { OpenFlag uint32 // off is the file offset. - off int64 + off atomicbitops.Int64 } func (fd *fileDescription) dentry() *kernfs.Dentry { diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index e72f9a480..ae927da20 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -19,9 +19,9 @@ import ( "math" "strconv" "sync" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/log" @@ -334,10 +334,10 @@ type inode struct { locks vfs.FileLocks // size of the file. - size uint64 + size atomicbitops.Uint64 // attributeVersion is the version of inode's attributes. - attributeVersion uint64 + attributeVersion atomicbitops.Uint64 // attributeTime is the remaining vaild time of attributes. attributeTime uint64 @@ -368,7 +368,7 @@ func (fs *filesystem) newInode(ctx context.Context, nodeID uint64, attr linux.FU i := &inode{fs: fs, nodeID: nodeID} creds := auth.Credentials{EffectiveKGID: auth.KGID(attr.UID), EffectiveKUID: auth.KUID(attr.UID)} i.InodeAttrs.Init(ctx, &creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.FileMode(attr.Mode)) - atomic.StoreUint64(&i.size, attr.Size) + i.size.Store(attr.Size) i.OrderedChildren.Init(kernfs.OrderedChildrenOptions{}) i.InitRefs() return i @@ -412,7 +412,7 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentr if !isDir && opts.Mode.IsDir() { return nil, linuxerr.ENOTDIR } - if opts.Flags&linux.O_LARGEFILE == 0 && atomic.LoadUint64(&i.size) > linux.MAX_NON_LFS { + if opts.Flags&linux.O_LARGEFILE == 0 && i.size.Load() > linux.MAX_NON_LFS { return nil, linuxerr.EOVERFLOW } @@ -496,9 +496,8 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentr // by setting the file size to 0. if i.fs.conn.atomicOTrunc && opts.Flags&linux.O_TRUNC != 0 { i.fs.conn.mu.Lock() - i.fs.conn.attributeVersion++ - i.attributeVersion = i.fs.conn.attributeVersion - atomic.StoreUint64(&i.size, 0) + i.attributeVersion.Store(i.fs.conn.attributeVersion.Add(1)) + i.size.Store(0) i.fs.conn.mu.Unlock() i.attributeTime = 0 } @@ -713,7 +712,7 @@ func (i *inode) Readlink(ctx context.Context, mnt *vfs.Mount) (string, error) { func (i *inode) getFUSEAttr() linux.FUSEAttr { return linux.FUSEAttr{ Ino: i.Ino(), - Size: atomic.LoadUint64(&i.size), + Size: i.size.Load(), Mode: uint32(i.Mode()), } } @@ -775,7 +774,7 @@ func statFromFUSEAttr(attr linux.FUSEAttr, mask, devMinor uint32) linux.Statx { // or read from local cache. It updates the corresponding attributes if // necessary. func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOptions, flags uint32, fh uint64) (linux.FUSEAttr, error) { - attributeVersion := atomic.LoadUint64(&i.fs.conn.attributeVersion) + attributeVersion := i.fs.conn.attributeVersion.Load() // TODO(gvisor.dev/issue/3679): send the request only if // - invalid local cache for fields specified in the opts.Mask @@ -814,7 +813,7 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp // Local version is newer, return the local one. // Skip the update. - if attributeVersion != 0 && atomic.LoadUint64(&i.attributeVersion) > attributeVersion { + if attributeVersion != 0 && i.attributeVersion.Load() > attributeVersion { return i.getFUSEAttr(), nil } @@ -826,7 +825,7 @@ func (i *inode) getAttr(ctx context.Context, fs *vfs.Filesystem, opts vfs.StatOp } // Set the size if no error (after SetStat() check). - atomic.StoreUint64(&i.size, out.Attr.Size) + i.size.Store(out.Attr.Size) return out.Attr, nil } diff --git a/pkg/sentry/fsimpl/fuse/read_write.go b/pkg/sentry/fsimpl/fuse/read_write.go index e2b4ffd74..5e4fbb8b4 100644 --- a/pkg/sentry/fsimpl/fuse/read_write.go +++ b/pkg/sentry/fsimpl/fuse/read_write.go @@ -16,7 +16,6 @@ package fuse import ( "io" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -34,7 +33,7 @@ import ( // We do not support direct IO (which read the exact number of bytes) // at this moment. func (fs *filesystem) ReadInPages(ctx context.Context, fd *regularFileFD, off uint64, size uint32) ([][]byte, uint32, error) { - attributeVersion := atomic.LoadUint64(&fs.conn.attributeVersion) + attributeVersion := fs.conn.attributeVersion.Load() t := kernel.TaskFromContext(ctx) if t == nil { @@ -138,10 +137,9 @@ func (fs *filesystem) ReadCallback(ctx context.Context, fd *regularFileFD, off u // Update existing size. newSize := off + uint64(sizeRead) fs.conn.mu.Lock() - if attributeVersion == i.attributeVersion && newSize < atomic.LoadUint64(&i.size) { - fs.conn.attributeVersion++ - i.attributeVersion = i.fs.conn.attributeVersion - atomic.StoreUint64(&i.size, newSize) + if attributeVersion == i.attributeVersion.Load() && newSize < i.size.Load() { + i.attributeVersion.Store(i.fs.conn.attributeVersion.Add(1)) + i.size.Store(newSize) } fs.conn.mu.Unlock() } diff --git a/pkg/sentry/fsimpl/fuse/regular_file.go b/pkg/sentry/fsimpl/fuse/regular_file.go index 38cde8208..8c14a1c64 100644 --- a/pkg/sentry/fsimpl/fuse/regular_file.go +++ b/pkg/sentry/fsimpl/fuse/regular_file.go @@ -18,7 +18,6 @@ import ( "io" "math" "sync" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -64,18 +63,18 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs inode := fd.inode() // Reading beyond EOF, update file size if outdated. - if uint64(offset+size) > atomic.LoadUint64(&inode.size) { + if uint64(offset+size) > inode.size.Load() { if err := inode.reviseAttr(ctx, linux.FUSE_GETATTR_FH, fd.Fh); err != nil { return 0, err } // If the offset after update is still too large, return error. - if uint64(offset) >= atomic.LoadUint64(&inode.size) { + if uint64(offset) >= inode.size.Load() { return 0, io.EOF } } // Truncate the read with updated file size. - fileSize := atomic.LoadUint64(&inode.size) + fileSize := inode.size.Load() if uint64(offset+size) > fileSize { size = int64(fileSize) - offset } @@ -163,7 +162,7 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // be true before we switch out from kernfs. if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 { // Locking inode.metadataMu is sufficient for reading size - offset = int64(inode.size) + offset = int64(inode.size.Load()) } srclen := src.NumBytes() @@ -221,9 +220,9 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off written = int64(n) finalOff = offset + written - if finalOff > int64(inode.size) { - atomic.StoreUint64(&inode.size, uint64(finalOff)) - atomic.AddUint64(&inode.fs.conn.attributeVersion, 1) + if finalOff > int64(inode.size.Load()) { + inode.size.Store(uint64(finalOff)) + inode.fs.conn.attributeVersion.Add(1) } return diff --git a/pkg/sentry/fsimpl/gofer/BUILD b/pkg/sentry/fsimpl/gofer/BUILD index 509dd0e1a..feccaf2b2 100644 --- a/pkg/sentry/fsimpl/gofer/BUILD +++ b/pkg/sentry/fsimpl/gofer/BUILD @@ -48,6 +48,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fd", diff --git a/pkg/sentry/fsimpl/gofer/directory.go b/pkg/sentry/fsimpl/gofer/directory.go index 7224ad09b..6dc2a7f5e 100644 --- a/pkg/sentry/fsimpl/gofer/directory.go +++ b/pkg/sentry/fsimpl/gofer/directory.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -112,17 +113,17 @@ type createSyntheticOpts struct { func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) { now := d.fs.clock.Now().Nanoseconds() child := &dentry{ - refs: 1, // held by d + refs: atomicbitops.FromInt64(1), // held by d fs: d.fs, ino: d.fs.nextIno(), mode: uint32(opts.mode), uid: uint32(opts.kuid), gid: uint32(opts.kgid), blockSize: hostarch.PageSize, // arbitrary - atime: now, - mtime: now, - ctime: now, - btime: now, + atime: atomicbitops.FromInt64(now), + mtime: atomicbitops.FromInt64(now), + ctime: atomicbitops.FromInt64(now), + btime: atomicbitops.FromInt64(now), readFD: -1, writeFD: -1, mmapFD: -1, diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 694621237..4dd712ef0 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -46,6 +46,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -176,7 +177,7 @@ type filesystem struct { // lastIno is the last inode number assigned to a file. lastIno is accessed // using atomic memory operations. - lastIno uint64 + lastIno atomicbitops.Uint64 // savedDentryRW records open read/write handles during save/restore. savedDentryRW map[*dentry]savedDentryRW @@ -519,7 +520,7 @@ func (fs *filesystem) initClientAndRoot(ctx context.Context) error { // caller, and the other is held by fs to prevent the root from being "cached" // and subsequently evicted. if err == nil { - fs.root.refs = 2 + fs.root.refs = atomicbitops.FromInt64(2) } return err } @@ -668,7 +669,7 @@ func (fs *filesystem) Release(ctx context.Context) { d.dataMu.Lock() if h := d.writeHandleLocked(); h.isOpen() { // Write dirty cached data to the remote file. - if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size, mf, h.writeFromBlocksAt); err != nil { + if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { log.Warningf("gofer.filesystem.Release: failed to flush dentry: %v", err) } // TODO(jamieliu): Do we need to flushf/fsync d? @@ -776,7 +777,7 @@ type dentry struct { // reaches 0, the dentry may be added to the cache or destroyed. If refs == // -1, the dentry has already been destroyed. refs is accessed using atomic // memory operations. - refs int64 + refs atomicbitops.Int64 // fs is the owning filesystem. fs is immutable. fs *filesystem @@ -865,10 +866,10 @@ type dentry struct { gid uint32 // auth.KGID, but ... blockSize uint32 // 0 if unknown // Timestamps, all nsecs from the Unix epoch. - atime int64 - mtime int64 - ctime int64 - btime int64 + atime atomicbitops.Int64 + mtime atomicbitops.Int64 + ctime atomicbitops.Int64 + btime atomicbitops.Int64 // File size, which differs from other metadata in two ways: // // - We make a best-effort attempt to keep it up to date even if @@ -876,7 +877,7 @@ type dentry struct { // // - size is protected by both metadataMu and dataMu (i.e. both must be // locked to mutate it; locking either is sufficient to access it). - size uint64 + size atomicbitops.Uint64 // If this dentry does not represent a synthetic file, deleted is 0, and // atimeDirty/mtimeDirty are non-zero, atime/mtime may have diverged from the // remote file's timestamps, which should be updated when this dentry is @@ -1025,22 +1026,22 @@ func (fs *filesystem) newDentry(ctx context.Context, file p9file, qid p9.QID, ma d.gid = dentryGIDFromP9GID(attr.GID) } if mask.Size { - d.size = attr.Size + d.size = atomicbitops.FromUint64(attr.Size) } if attr.BlockSize != 0 { d.blockSize = uint32(attr.BlockSize) } if mask.ATime { - d.atime = dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds) + d.atime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds)) } if mask.MTime { - d.mtime = dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds) + d.mtime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds)) } if mask.CTime { - d.ctime = dentryTimestampFromP9(attr.CTimeSeconds, attr.CTimeNanoSeconds) + d.ctime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.CTimeSeconds, attr.CTimeNanoSeconds)) } if mask.BTime { - d.btime = dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds) + d.btime = atomicbitops.FromInt64(dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds)) } if mask.NLink { d.nlink = uint32(attr.NLink) @@ -1086,22 +1087,22 @@ func (fs *filesystem) newDentryLisa(ctx context.Context, ino *lisafs.Inode) (*de d.gid = dentryGIDFromLisaGID(lisafs.GID(ino.Stat.GID)) } if ino.Stat.Mask&linux.STATX_SIZE != 0 { - d.size = ino.Stat.Size + d.size = atomicbitops.FromUint64(ino.Stat.Size) } if ino.Stat.Blksize != 0 { d.blockSize = ino.Stat.Blksize } if ino.Stat.Mask&linux.STATX_ATIME != 0 { - d.atime = dentryTimestampFromLisa(ino.Stat.Atime) + d.atime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Atime)) } if ino.Stat.Mask&linux.STATX_MTIME != 0 { - d.mtime = dentryTimestampFromLisa(ino.Stat.Mtime) + d.mtime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Mtime)) } if ino.Stat.Mask&linux.STATX_CTIME != 0 { - d.ctime = dentryTimestampFromLisa(ino.Stat.Ctime) + d.ctime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Ctime)) } if ino.Stat.Mask&linux.STATX_BTIME != 0 { - d.btime = dentryTimestampFromLisa(ino.Stat.Btime) + d.btime = atomicbitops.FromInt64(dentryTimestampFromLisa(ino.Stat.Btime)) } if ino.Stat.Mask&linux.STATX_NLINK != 0 { d.nlink = ino.Stat.Nlink @@ -1138,7 +1139,7 @@ func (fs *filesystem) inoFromQIDPath(qidPath uint64) uint64 { } func (fs *filesystem) nextIno() uint64 { - return atomic.AddUint64(&fs.lastIno, 1) + return fs.lastIno.Add(1) } func (d *dentry) isSynthetic() bool { @@ -1173,16 +1174,16 @@ func (d *dentry) updateFromP9AttrsLocked(mask p9.AttrMask, attr *p9.Attr) { // Don't override newer client-defined timestamps with old server-defined // ones. if mask.ATime && atomic.LoadUint32(&d.atimeDirty) == 0 { - atomic.StoreInt64(&d.atime, dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds)) + d.atime.Store(dentryTimestampFromP9(attr.ATimeSeconds, attr.ATimeNanoSeconds)) } if mask.MTime && atomic.LoadUint32(&d.mtimeDirty) == 0 { - atomic.StoreInt64(&d.mtime, dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds)) + d.mtime.Store(dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds)) } if mask.CTime { - atomic.StoreInt64(&d.ctime, dentryTimestampFromP9(attr.CTimeSeconds, attr.CTimeNanoSeconds)) + d.ctime.Store(dentryTimestampFromP9(attr.CTimeSeconds, attr.CTimeNanoSeconds)) } if mask.BTime { - atomic.StoreInt64(&d.btime, dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds)) + d.btime.Store(dentryTimestampFromP9(attr.BTimeSeconds, attr.BTimeNanoSeconds)) } if mask.NLink { atomic.StoreUint32(&d.nlink, uint32(attr.NLink)) @@ -1217,16 +1218,16 @@ func (d *dentry) updateFromLisaStatLocked(stat *linux.Statx) { // Don't override newer client-defined timestamps with old server-defined // ones. if stat.Mask&linux.STATX_ATIME != 0 && atomic.LoadUint32(&d.atimeDirty) == 0 { - atomic.StoreInt64(&d.atime, dentryTimestampFromLisa(stat.Atime)) + d.atime.Store(dentryTimestampFromLisa(stat.Atime)) } if stat.Mask&linux.STATX_MTIME != 0 && atomic.LoadUint32(&d.mtimeDirty) == 0 { - atomic.StoreInt64(&d.mtime, dentryTimestampFromLisa(stat.Mtime)) + d.mtime.Store(dentryTimestampFromLisa(stat.Mtime)) } if stat.Mask&linux.STATX_CTIME != 0 { - atomic.StoreInt64(&d.ctime, dentryTimestampFromLisa(stat.Ctime)) + d.ctime.Store(dentryTimestampFromLisa(stat.Ctime)) } if stat.Mask&linux.STATX_BTIME != 0 { - atomic.StoreInt64(&d.btime, dentryTimestampFromLisa(stat.Btime)) + d.btime.Store(dentryTimestampFromLisa(stat.Btime)) } if stat.Mask&linux.STATX_NLINK != 0 { atomic.StoreUint32(&d.nlink, stat.Nlink) @@ -1372,14 +1373,14 @@ func (d *dentry) statTo(stat *linux.Statx) { stat.GID = atomic.LoadUint32(&d.gid) stat.Mode = uint16(atomic.LoadUint32(&d.mode)) stat.Ino = uint64(d.ino) - stat.Size = atomic.LoadUint64(&d.size) + stat.Size = d.size.Load() // This is consistent with regularFileFD.Seek(), which treats regular files // as having no holes. stat.Blocks = (stat.Size + 511) / 512 - stat.Atime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&d.atime)) - stat.Btime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&d.btime)) - stat.Ctime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&d.ctime)) - stat.Mtime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&d.mtime)) + stat.Atime = linux.NsecToStatxTimestamp(d.atime.Load()) + stat.Btime = linux.NsecToStatxTimestamp(d.btime.Load()) + stat.Ctime = linux.NsecToStatxTimestamp(d.ctime.Load()) + stat.Mtime = linux.NsecToStatxTimestamp(d.mtime.Load()) stat.DevMajor = linux.UNNAMED_MAJOR stat.DevMinor = d.fs.devMinor } @@ -1540,14 +1541,14 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs // !d.cachedMetadataAuthoritative() then we returned after calling // d.file.setAttr(). For the same reason, now must have been initialized. if stat.Mask&linux.STATX_ATIME != 0 && failureMask&linux.STATX_ATIME == 0 { - atomic.StoreInt64(&d.atime, stat.Atime.ToNsec()) + d.atime.Store(stat.Atime.ToNsec()) atomic.StoreUint32(&d.atimeDirty, 0) } if stat.Mask&linux.STATX_MTIME != 0 && failureMask&linux.STATX_MTIME == 0 { - atomic.StoreInt64(&d.mtime, stat.Mtime.ToNsec()) + d.mtime.Store(stat.Mtime.ToNsec()) atomic.StoreUint32(&d.mtimeDirty, 0) } - atomic.StoreInt64(&d.ctime, now) + d.ctime.Store(now) if failureMask != 0 { // Setting some attribute failed on the remote filesystem. return failureErr @@ -1563,7 +1564,7 @@ func (d *dentry) doAllocate(ctx context.Context, offset, length uint64, allocate // Allocating a smaller size is a noop. size := offset + length - if d.cachedMetadataAuthoritative() && size <= d.size { + if d.cachedMetadataAuthoritative() && size <= d.size.RacyLoad() { return nil } @@ -1589,8 +1590,8 @@ func (d *dentry) updateSizeLocked(newSize uint64) { // Postconditions: d.dataMu is unlocked. // +checklocksrelease:d.dataMu func (d *dentry) updateSizeAndUnlockDataMuLocked(newSize uint64) { - oldSize := d.size - atomic.StoreUint64(&d.size, newSize) + oldSize := d.size.RacyLoad() + d.size.Store(newSize) // d.dataMu must be unlocked to lock d.mapsMu and invalidate mappings // below. This allows concurrent calls to Read/Translate/etc. These // functions synchronize with truncation by refusing to use cache @@ -1688,7 +1689,7 @@ func dentryGIDFromLisaGID(gid lisafs.GID) uint32 { func (d *dentry) IncRef() { // d.refs may be 0 if d.fs.renameMu is locked, which serializes against // d.checkCachingLocked(). - r := atomic.AddInt64(&d.refs, 1) + r := d.refs.Add(1) if d.LogRefs() { refsvfs2.LogIncRef(d, r) } @@ -1697,11 +1698,11 @@ func (d *dentry) IncRef() { // TryIncRef implements vfs.DentryImpl.TryIncRef. func (d *dentry) TryIncRef() bool { for { - r := atomic.LoadInt64(&d.refs) + r := d.refs.Load() if r <= 0 { return false } - if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { + if d.refs.CompareAndSwap(r, r+1) { if d.LogRefs() { refsvfs2.LogTryIncRef(d, r+1) } @@ -1721,7 +1722,7 @@ func (d *dentry) DecRef(ctx context.Context) { // d.checkCachingLocked, even if d's reference count reaches 0; callers are // responsible for ensuring that d.checkCachingLocked will be called later. func (d *dentry) decRefNoCaching() int64 { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -1738,7 +1739,7 @@ func (d *dentry) RefType() string { // LeakMessage implements refsvfs2.CheckedObject.LeakMessage. func (d *dentry) LeakMessage() string { - return fmt.Sprintf("[gofer.dentry %p] reference count of %d instead of -1", d, atomic.LoadInt64(&d.refs)) + return fmt.Sprintf("[gofer.dentry %p] reference count of %d instead of -1", d, d.refs.Load()) } // LogRefs implements refsvfs2.CheckedObject.LogRefs. @@ -1794,7 +1795,7 @@ func (d *dentry) OnZeroWatches(ctx context.Context) { // renameMuWriteLocked is true; it may be temporarily unlocked. func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked bool) { d.cachingMu.Lock() - refs := atomic.LoadInt64(&d.refs) + refs := d.refs.Load() if refs == -1 { // Dentry has already been destroyed. d.cachingMu.Unlock() @@ -1821,7 +1822,7 @@ func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked boo defer d.fs.renameMu.Unlock() // Now that renameMu is locked for writing, no more refs can be taken on // d because path resolution requires renameMu for reading at least. - if atomic.LoadInt64(&d.refs) != 0 { + if d.refs.Load() != 0 { // Destroy d only if its ref is still 0. If not, either someone took a // ref on it or it got destroyed before fs.renameMu could be acquired. return @@ -1928,7 +1929,7 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { victim.removeFromCacheLocked() // victim.refs or victim.watches.Size() may have become non-zero from an // earlier path resolution since it was inserted into fs.cachedDentries. - if atomic.LoadInt64(&victim.refs) != 0 || victim.watches.Size() != 0 { + if victim.refs.Load() != 0 || victim.watches.Size() != 0 { victim.cachingMu.Unlock() return } @@ -1962,10 +1963,10 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { // from its former parent dentry. // +checklocks:d.fs.renameMu func (d *dentry) destroyLocked(ctx context.Context) { - switch atomic.LoadInt64(&d.refs) { + switch d.refs.Load() { case 0: // Mark the dentry destroyed. - atomic.StoreInt64(&d.refs, -1) + d.refs.Store(-1) case -1: panic("dentry.destroyLocked() called on already destroyed dentry") default: @@ -1981,7 +1982,7 @@ func (d *dentry) destroyLocked(ctx context.Context) { d.dataMu.Lock() if h := d.writeHandleLocked(); h.isOpen() { // Write dirty pages back to the remote filesystem. - if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size, mf, h.writeFromBlocksAt); err != nil { + if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { log.Warningf("gofer.dentry.destroyLocked: failed to write dirty data back: %v", err) } } @@ -2425,7 +2426,7 @@ func (d *dentry) syncCachedFile(ctx context.Context, forFilesystemSync bool) err if h.isOpen() { // Write back dirty pages to the remote file. d.dataMu.Lock() - err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size, d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) + err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) d.dataMu.Unlock() if err != nil { return err diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index d5cc73f33..0384683f9 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/pkg/sentry/fsimpl/gofer/gofer_test.go @@ -15,7 +15,6 @@ package gofer import ( - "sync/atomic" "testing" "gvisor.dev/gvisor/pkg/p9" @@ -57,11 +56,11 @@ func TestDestroyIdempotent(t *testing.T) { fs.renameMu.Lock() defer fs.renameMu.Unlock() child.checkCachingLocked(ctx, true /* renameMuWriteLocked */) - if got := atomic.LoadInt64(&child.refs); got != -1 { + if got := child.refs.Load(); got != -1 { t.Fatalf("child.refs=%d, want: -1", got) } // Parent will also be destroyed when child reference is removed. - if got := atomic.LoadInt64(&parent.refs); got != -1 { + if got := parent.refs.Load(); got != -1 { t.Fatalf("parent.refs=%d, want: -1", got) } child.checkCachingLocked(ctx, true /* renameMuWriteLocked */) diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go index 69a44d686..8ba5f1131 100644 --- a/pkg/sentry/fsimpl/gofer/regular_file.go +++ b/pkg/sentry/fsimpl/gofer/regular_file.go @@ -150,7 +150,7 @@ func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs // Check for reading at EOF before calling into MM (but not under // InteropModeShared, which makes d.size unreliable). - if d.cachedMetadataAuthoritative() && uint64(offset) >= atomic.LoadUint64(&d.size) { + if d.cachedMetadataAuthoritative() && uint64(offset) >= d.size.Load() { return 0, io.EOF } @@ -235,7 +235,7 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // Set offset to file size if the fd was opened with O_APPEND. if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 { // Holding d.metadataMu is sufficient for reading d.size. - offset = int64(d.size) + offset = int64(d.size.RacyLoad()) } limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes()) if err != nil { @@ -419,12 +419,12 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) } // Compute the range to read (limited by file size and overflow-checked). - if rw.off >= rw.d.size { + end := rw.d.size.Load() + if rw.off >= end { dataMuUnlock() rw.d.handleMu.RUnlock() return 0, io.EOF } - end := rw.d.size if rend := rw.off + dsts.NumBytes(); rend > rw.off && rend < end { end = rend } @@ -468,7 +468,7 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) End: gapEnd, } optMR := gap.Range() - err := rw.d.cache.Fill(rw.ctx, reqMR, maxFillRange(reqMR, optMR), rw.d.size, mf, usage.PageCache, h.readToBlocksAt) + err := rw.d.cache.Fill(rw.ctx, reqMR, maxFillRange(reqMR, optMR), rw.d.size.Load(), mf, usage.PageCache, h.readToBlocksAt) mf.MarkEvictable(rw.d, pgalloc.EvictableRange{optMR.Start, optMR.End}) seg, gap = rw.d.cache.Find(rw.off) if !seg.Ok() { @@ -523,8 +523,8 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro n, err := h.writeFromBlocksAt(rw.ctx, srcs, rw.off) rw.off += n rw.d.dataMu.Lock() - if rw.off > rw.d.size { - atomic.StoreUint64(&rw.d.size, rw.off) + if rw.off > rw.d.size.Load() { + rw.d.size.Store(rw.off) // The remote file's size will implicitly be extended to the correct // value when we write back to it. } @@ -597,8 +597,8 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro } } exitLoop: - if rw.off > rw.d.size { - atomic.StoreUint64(&rw.d.size, rw.off) + if rw.off > rw.d.size.Load() { + rw.d.size.Store(rw.off) // The remote file's size will implicitly be extended to the correct // value when we write back to it. } @@ -608,7 +608,7 @@ exitLoop: if err := fsutil.SyncDirty(rw.ctx, memmap.MappableRange{ Start: start, End: rw.off, - }, &rw.d.cache, &rw.d.dirty, rw.d.size, mf, h.writeFromBlocksAt); err != nil { + }, &rw.d.cache, &rw.d.dirty, rw.d.size.Load(), mf, h.writeFromBlocksAt); err != nil { // We have no idea how many bytes were actually flushed. rw.off = start done = 0 @@ -630,17 +630,18 @@ func (d *dentry) writeback(ctx context.Context, offset, size int64) error { d.dataMu.Lock() defer d.dataMu.Unlock() // Compute the range of valid bytes (overflow-checked). - if uint64(offset) >= d.size { + dentrySize := d.size.Load() + if uint64(offset) >= dentrySize { return nil } - end := int64(d.size) + end := int64(dentrySize) if rend := offset + size; rend > offset && rend < end { end = rend } return fsutil.SyncDirty(ctx, memmap.MappableRange{ Start: uint64(offset), End: uint64(end), - }, &d.cache, &d.dirty, d.size, d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) + }, &d.cache, &d.dirty, dentrySize, d.fs.mfp.MemoryFile(), h.writeFromBlocksAt) } // Seek implements vfs.FileDescriptionImpl.Seek. @@ -669,7 +670,7 @@ func regularFileSeekLocked(ctx context.Context, d *dentry, fdOffset, offset int6 return 0, err } } - size := int64(atomic.LoadUint64(&d.size)) + size := int64(d.size.Load()) // For SEEK_DATA and SEEK_HOLE, treat the file as a single contiguous // block of data. switch whence { @@ -809,7 +810,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab // Constrain translations to d.size (rounded up) to prevent translation to // pages that may be concurrently truncated. - pgend, _ := hostarch.PageRoundUp(d.size) + pgend, _ := hostarch.PageRoundUp(d.size.Load()) var beyondEOF bool if required.End > pgend { if required.Start >= pgend { @@ -826,7 +827,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab mf := d.fs.mfp.MemoryFile() h := d.readHandleLocked() - cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size, mf, usage.PageCache, h.readToBlocksAt) + cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, h.readToBlocksAt) var ts []memmap.Translation var translatedEnd uint64 @@ -900,7 +901,7 @@ func (d *dentry) InvalidateUnsavable(ctx context.Context) error { h := d.writeHandleLocked() d.dataMu.Lock() defer d.dataMu.Unlock() - if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size, mf, h.writeFromBlocksAt); err != nil { + if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { return err } @@ -931,7 +932,7 @@ func (d *dentry) Evict(ctx context.Context, er pgalloc.EvictableRange) { if mgapMR.Length() == 0 { continue } - if err := fsutil.SyncDirty(ctx, mgapMR, &d.cache, &d.dirty, d.size, mf, h.writeFromBlocksAt); err != nil { + if err := fsutil.SyncDirty(ctx, mgapMR, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil { log.Warningf("Failed to writeback cached data %v: %v", mgapMR, err) } d.cache.Drop(mgapMR, mf) diff --git a/pkg/sentry/fsimpl/gofer/save_restore.go b/pkg/sentry/fsimpl/gofer/save_restore.go index 18f8a1ea5..482c48b9e 100644 --- a/pkg/sentry/fsimpl/gofer/save_restore.go +++ b/pkg/sentry/fsimpl/gofer/save_restore.go @@ -152,7 +152,7 @@ func (d *dentry) afterLoad() { d.readFD = -1 d.writeFD = -1 d.mmapFD = -1 - if atomic.LoadInt64(&d.refs) != -1 { + if d.refs.Load() != -1 { refsvfs2.Register(d) } } @@ -279,16 +279,16 @@ func (d *dentry) restoreFile(ctx context.Context, file p9file, qid p9.QID, attrM if !attrMask.Size { return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: file size not available", genericDebugPathname(d))} } - if d.size != attr.Size { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size, attr.Size)} + if d.size.Load() != attr.Size { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size.Load(), attr.Size)} } } if opts.ValidateFileModificationTimestamps { if !attrMask.MTime { return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime not available", genericDebugPathname(d))} } - if want := dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds); d.mtime != want { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime), linux.NsecToStatxTimestamp(want))} + if want := dentryTimestampFromP9(attr.MTimeSeconds, attr.MTimeNanoSeconds); d.mtime.Load() != want { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime.Load()), linux.NsecToStatxTimestamp(want))} } } } @@ -328,16 +328,16 @@ func (d *dentry) restoreFileLisa(ctx context.Context, inode *lisafs.Inode, opts if inode.Stat.Mask&linux.STATX_SIZE == 0 { return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: file size not available", genericDebugPathname(d))} } - if d.size != inode.Stat.Size { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size, inode.Stat.Size)} + if d.size.RacyLoad() != inode.Stat.Size { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size.Load(), inode.Stat.Size)} } } if opts.ValidateFileModificationTimestamps { if inode.Stat.Mask&linux.STATX_MTIME != 0 { return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime not available", genericDebugPathname(d))} } - if want := dentryTimestampFromLisa(inode.Stat.Mtime); d.mtime != want { - return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime), linux.NsecToStatxTimestamp(want))} + if want := dentryTimestampFromLisa(inode.Stat.Mtime); d.mtime.RacyLoad() != want { + return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime.RacyLoad()), linux.NsecToStatxTimestamp(want))} } } } diff --git a/pkg/sentry/fsimpl/gofer/special_file.go b/pkg/sentry/fsimpl/gofer/special_file.go index dfcd5acf9..a9ad0a239 100644 --- a/pkg/sentry/fsimpl/gofer/special_file.go +++ b/pkg/sentry/fsimpl/gofer/special_file.go @@ -322,7 +322,7 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // Set offset to file size if the regular file was opened with O_APPEND. if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 { // Holding d.metadataMu is sufficient for reading d.size. - offset = int64(d.size) + offset = int64(d.size.RacyLoad()) } limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes()) if err != nil { @@ -352,10 +352,10 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // Update file size for regular files. if fd.isRegularFile { // d.metadataMu is already locked at this point. - if uint64(offset) > d.size { + if uint64(offset) > d.size.RacyLoad() { d.dataMu.Lock() defer d.dataMu.Unlock() - atomic.StoreUint64(&d.size, uint64(offset)) + d.size.Store(uint64(offset)) } } return int64(n), offset, err diff --git a/pkg/sentry/fsimpl/gofer/time.go b/pkg/sentry/fsimpl/gofer/time.go index 07940b225..2381ea30c 100644 --- a/pkg/sentry/fsimpl/gofer/time.go +++ b/pkg/sentry/fsimpl/gofer/time.go @@ -39,7 +39,7 @@ func (d *dentry) touchAtime(mnt *vfs.Mount) { } now := d.fs.clock.Now().Nanoseconds() d.metadataMu.Lock() - atomic.StoreInt64(&d.atime, now) + d.atime.Store(now) atomic.StoreUint32(&d.atimeDirty, 1) d.metadataMu.Unlock() mnt.EndWrite() @@ -54,7 +54,7 @@ func (d *dentry) touchAtimeLocked(mnt *vfs.Mount) { return } now := d.fs.clock.Now().Nanoseconds() - atomic.StoreInt64(&d.atime, now) + d.atime.Store(now) atomic.StoreUint32(&d.atimeDirty, 1) mnt.EndWrite() } @@ -65,7 +65,7 @@ func (d *dentry) touchAtimeLocked(mnt *vfs.Mount) { func (d *dentry) touchCtime() { now := d.fs.clock.Now().Nanoseconds() d.metadataMu.Lock() - atomic.StoreInt64(&d.ctime, now) + d.ctime.Store(now) d.metadataMu.Unlock() } @@ -75,8 +75,8 @@ func (d *dentry) touchCtime() { func (d *dentry) touchCMtime() { now := d.fs.clock.Now().Nanoseconds() d.metadataMu.Lock() - atomic.StoreInt64(&d.mtime, now) - atomic.StoreInt64(&d.ctime, now) + d.mtime.Store(now) + d.ctime.Store(now) atomic.StoreUint32(&d.mtimeDirty, 1) d.metadataMu.Unlock() } @@ -86,7 +86,7 @@ func (d *dentry) touchCMtime() { // * The caller has locked d.metadataMu. func (d *dentry) touchCMtimeLocked() { now := d.fs.clock.Now().Nanoseconds() - atomic.StoreInt64(&d.mtime, now) - atomic.StoreInt64(&d.ctime, now) + d.mtime.Store(now) + d.ctime.Store(now) atomic.StoreUint32(&d.mtimeDirty, 1) } diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index d314e128e..424d3f33d 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -176,7 +177,7 @@ func (InodeNotSymlink) Getlink(context.Context, *vfs.Mount) (vfs.VirtualDentry, type InodeAttrs struct { devMajor uint32 devMinor uint32 - ino uint64 + ino atomicbitops.Uint64 mode uint32 uid uint32 gid uint32 @@ -184,9 +185,9 @@ type InodeAttrs struct { blockSize uint32 // Timestamps, all nsecs from the Unix epoch. - atime int64 - mtime int64 - ctime int64 + atime atomicbitops.Int64 + mtime atomicbitops.Int64 + ctime atomicbitops.Int64 } // Init initializes this InodeAttrs. @@ -201,16 +202,16 @@ func (a *InodeAttrs) Init(ctx context.Context, creds *auth.Credentials, devMajor } a.devMajor = devMajor a.devMinor = devMinor - atomic.StoreUint64(&a.ino, ino) + a.ino.Store(ino) atomic.StoreUint32(&a.mode, uint32(mode)) atomic.StoreUint32(&a.uid, uint32(creds.EffectiveKUID)) atomic.StoreUint32(&a.gid, uint32(creds.EffectiveKGID)) atomic.StoreUint32(&a.nlink, nlink) atomic.StoreUint32(&a.blockSize, hostarch.PageSize) now := ktime.NowFromContext(ctx).Nanoseconds() - atomic.StoreInt64(&a.atime, now) - atomic.StoreInt64(&a.mtime, now) - atomic.StoreInt64(&a.ctime, now) + a.atime.Store(now) + a.mtime.Store(now) + a.ctime.Store(now) } // DevMajor returns the device major number. @@ -225,7 +226,7 @@ func (a *InodeAttrs) DevMinor() uint32 { // Ino returns the inode id. func (a *InodeAttrs) Ino() uint64 { - return atomic.LoadUint64(&a.ino) + return a.ino.Load() } // Mode implements Inode.Mode. @@ -246,7 +247,7 @@ func (a *InodeAttrs) TouchAtime(ctx context.Context, mnt *vfs.Mount) { if err := mnt.CheckBeginWrite(); err != nil { return } - atomic.StoreInt64(&a.atime, ktime.NowFromContext(ctx).Nanoseconds()) + a.atime.Store(ktime.NowFromContext(ctx).Nanoseconds()) mnt.EndWrite() } @@ -255,8 +256,8 @@ func (a *InodeAttrs) TouchAtime(ctx context.Context, mnt *vfs.Mount) { // value. func (a *InodeAttrs) TouchCMtime(ctx context.Context) { now := ktime.NowFromContext(ctx).Nanoseconds() - atomic.StoreInt64(&a.mtime, now) - atomic.StoreInt64(&a.ctime, now) + a.mtime.Store(now) + a.ctime.Store(now) } // Stat partially implements Inode.Stat. Note that this function doesn't provide @@ -267,15 +268,15 @@ func (a *InodeAttrs) Stat(context.Context, *vfs.Filesystem, vfs.StatOptions) (li stat.Mask = linux.STATX_TYPE | linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID | linux.STATX_INO | linux.STATX_NLINK | linux.STATX_ATIME | linux.STATX_MTIME | linux.STATX_CTIME stat.DevMajor = a.devMajor stat.DevMinor = a.devMinor - stat.Ino = atomic.LoadUint64(&a.ino) + stat.Ino = a.ino.Load() stat.Mode = uint16(a.Mode()) stat.UID = atomic.LoadUint32(&a.uid) stat.GID = atomic.LoadUint32(&a.gid) stat.Nlink = atomic.LoadUint32(&a.nlink) stat.Blksize = atomic.LoadUint32(&a.blockSize) - stat.Atime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&a.atime)) - stat.Mtime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&a.mtime)) - stat.Ctime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&a.ctime)) + stat.Atime = linux.NsecToStatxTimestamp(a.atime.Load()) + stat.Mtime = linux.NsecToStatxTimestamp(a.mtime.Load()) + stat.Ctime = linux.NsecToStatxTimestamp(a.ctime.Load()) return stat, nil } @@ -341,13 +342,13 @@ func (a *InodeAttrs) SetStat(ctx context.Context, fs *vfs.Filesystem, creds *aut if stat.Atime.Nsec == linux.UTIME_NOW { stat.Atime = linux.NsecToStatxTimestamp(now) } - atomic.StoreInt64(&a.atime, stat.Atime.ToNsec()) + a.atime.Store(stat.Atime.ToNsec()) } if stat.Mask&linux.STATX_MTIME != 0 { if stat.Mtime.Nsec == linux.UTIME_NOW { stat.Mtime = linux.NsecToStatxTimestamp(now) } - atomic.StoreInt64(&a.mtime, stat.Mtime.ToNsec()) + a.mtime.Store(stat.Mtime.ToNsec()) } return nil diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index 9a349b188..d4afec291 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -60,6 +60,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -109,7 +110,7 @@ type Filesystem struct { // nextInoMinusOne is used to to allocate inode numbers on this // filesystem. Must be accessed by atomic operations. - nextInoMinusOne uint64 + nextInoMinusOne atomicbitops.Uint64 // cachedDentries contains all dentries with 0 references. (Due to race // conditions, it may also contain dentries with non-zero references.) @@ -184,7 +185,7 @@ func (fs *Filesystem) VFSFilesystem() *vfs.Filesystem { // NextIno allocates a new inode number on this filesystem. func (fs *Filesystem) NextIno() uint64 { - return atomic.AddUint64(&fs.nextInoMinusOne, 1) + return fs.nextInoMinusOne.Add(1) } // These consts are used in the Dentry.flags field. @@ -214,7 +215,7 @@ type Dentry struct { // added to the cache or destroyed. If refs == -1, the dentry has already // been destroyed. refs are allowed to go to 0 and increase again. refs is // accessed using atomic memory operations. - refs int64 + refs atomicbitops.Int64 // fs is the owning filesystem. fs is immutable. fs *Filesystem @@ -247,7 +248,7 @@ type Dentry struct { func (d *Dentry) IncRef() { // d.refs may be 0 if d.fs.mu is locked, which serializes against // d.cacheLocked(). - r := atomic.AddInt64(&d.refs, 1) + r := d.refs.Add(1) if d.LogRefs() { refsvfs2.LogIncRef(d, r) } @@ -256,11 +257,11 @@ func (d *Dentry) IncRef() { // TryIncRef implements vfs.DentryImpl.TryIncRef. func (d *Dentry) TryIncRef() bool { for { - r := atomic.LoadInt64(&d.refs) + r := d.refs.Load() if r <= 0 { return false } - if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { + if d.refs.CompareAndSwap(r, r+1) { if d.LogRefs() { refsvfs2.LogTryIncRef(d, r+1) } @@ -271,7 +272,7 @@ func (d *Dentry) TryIncRef() bool { // DecRef implements vfs.DentryImpl.DecRef. func (d *Dentry) DecRef(ctx context.Context) { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -285,7 +286,7 @@ func (d *Dentry) DecRef(ctx context.Context) { } func (d *Dentry) decRefLocked(ctx context.Context) { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -309,7 +310,7 @@ func (d *Dentry) cacheLocked(ctx context.Context) { // to obtain a reference on a dentry with zero references is via path // resolution, which requires d.fs.mu, so if d.refs is zero then it will // remain zero while we hold d.fs.mu for writing.) - refs := atomic.LoadInt64(&d.refs) + refs := d.refs.Load() if refs == -1 { // Dentry has already been destroyed. return @@ -370,7 +371,7 @@ func (fs *Filesystem) evictCachedDentryLocked(ctx context.Context) { victim.cached = false // victim.refs may have become non-zero from an earlier path resolution // after it was inserted into fs.cachedDentries. - if atomic.LoadInt64(&victim.refs) == 0 { + if victim.refs.Load() == 0 { if !victim.vfsd.IsDead() { victim.parent.dirMu.Lock() // Note that victim can't be a mount point (in any mount @@ -394,11 +395,11 @@ func (fs *Filesystem) evictCachedDentryLocked(ctx context.Context) { // by path traversal. // * d.vfsd.IsDead() is true. func (d *Dentry) destroyLocked(ctx context.Context) { - refs := atomic.LoadInt64(&d.refs) + refs := d.refs.Load() switch refs { case 0: // Mark the dentry destroyed. - atomic.StoreInt64(&d.refs, -1) + d.refs.Store(-1) case -1: panic("dentry.destroyLocked() called on already destroyed dentry") default: @@ -422,7 +423,7 @@ func (d *Dentry) RefType() string { // LeakMessage implements refsvfs2.CheckedObject.LeakMessage. func (d *Dentry) LeakMessage() string { - return fmt.Sprintf("[kernfs.Dentry %p] reference count of %d instead of -1", d, atomic.LoadInt64(&d.refs)) + return fmt.Sprintf("[kernfs.Dentry %p] reference count of %d instead of -1", d, d.refs.Load()) } // LogRefs implements refsvfs2.CheckedObject.LogRefs. @@ -455,7 +456,7 @@ func (d *Dentry) Init(fs *Filesystem, inode Inode) { d.vfsd.Init(d) d.fs = fs d.inode = inode - atomic.StoreInt64(&d.refs, 1) + d.refs.Store(1) ftype := inode.Mode().FileType() if ftype == linux.ModeDirectory { d.flags |= dflagsIsDir diff --git a/pkg/sentry/fsimpl/kernfs/save_restore.go b/pkg/sentry/fsimpl/kernfs/save_restore.go index f78509eb7..e22592c77 100644 --- a/pkg/sentry/fsimpl/kernfs/save_restore.go +++ b/pkg/sentry/fsimpl/kernfs/save_restore.go @@ -15,14 +15,12 @@ package kernfs import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/refsvfs2" ) // afterLoad is invoked by stateify. func (d *Dentry) afterLoad() { - if atomic.LoadInt64(&d.refs) >= 0 { + if d.refs.Load() >= 0 { refsvfs2.Register(d) } } diff --git a/pkg/sentry/fsimpl/overlay/BUILD b/pkg/sentry/fsimpl/overlay/BUILD index d16dfef9b..d1912dcd4 100644 --- a/pkg/sentry/fsimpl/overlay/BUILD +++ b/pkg/sentry/fsimpl/overlay/BUILD @@ -28,6 +28,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go index 520487066..ec50f52f5 100644 --- a/pkg/sentry/fsimpl/overlay/copy_up.go +++ b/pkg/sentry/fsimpl/overlay/copy_up.go @@ -280,7 +280,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy } atomic.StoreUint32(&d.devMajor, upperStat.DevMajor) atomic.StoreUint32(&d.devMinor, upperStat.DevMinor) - atomic.StoreUint64(&d.ino, upperStat.Ino) + d.ino.Store(upperStat.Ino) } if mmapOpts != nil && mmapOpts.Mappable != nil { diff --git a/pkg/sentry/fsimpl/overlay/directory.go b/pkg/sentry/fsimpl/overlay/directory.go index ad3cdbb56..d47eab1ab 100644 --- a/pkg/sentry/fsimpl/overlay/directory.go +++ b/pkg/sentry/fsimpl/overlay/directory.go @@ -163,13 +163,13 @@ func (d *dentry) getDirentsLocked(ctx context.Context) ([]vfs.Dirent, error) { { Name: ".", Type: linux.DT_DIR, - Ino: d.ino, + Ino: d.ino.Load(), NextOff: 1, }, { Name: "..", Type: uint8(atomic.LoadUint32(&parent.mode) >> 12), - Ino: parent.ino, + Ino: parent.ino.Load(), NextOff: 2, }, } diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 8f7bced09..e27bc3020 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -20,6 +20,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -97,7 +98,7 @@ func (fs *filesystem) renameMuRUnlockAndCheckDrop(ctx context.Context, dsp **[]* // re-locking renameMu) if we actually have any dentries with zero refs. checkAny := false for i := range ds { - if atomic.LoadInt64(&ds[i].refs) == 0 { + if ds[i].refs.Load() == 0 { checkAny = true break } @@ -286,7 +287,7 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str child.gid = stat.GID child.devMajor = stat.DevMajor child.devMinor = stat.DevMinor - child.ino = stat.Ino + child.ino = atomicbitops.FromUint64(stat.Ino) } // For non-directory files, only the topmost layer that contains a file diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 327c37477..74079a18e 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -39,6 +39,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -250,7 +251,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // Construct the root dentry. root := fs.newDentry() - root.refs = 1 + root.refs = atomicbitops.FromInt64(1) if fs.opts.UpperRoot.Ok() { fs.opts.UpperRoot.IncRef() root.copiedUp = 1 @@ -297,7 +298,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt return nil, nil, err } root.devMinor = rootDevMinor - root.ino = rootStat.Ino + root.ino.Store(rootStat.Ino) return &fs.vfsfs, &root.vfsd, nil } @@ -377,7 +378,7 @@ func (fs *filesystem) getPrivateDevMinor(layerMajor, layerMinor uint32) (uint32, type dentry struct { vfsd vfs.Dentry - refs int64 + refs atomicbitops.Int64 // fs is the owning filesystem. fs is immutable. fs *filesystem @@ -429,7 +430,7 @@ type dentry struct { // using atomic memory operations. devMajor uint32 devMinor uint32 - ino uint64 + ino atomicbitops.Uint64 // If this dentry represents a regular file, then: // @@ -488,7 +489,7 @@ func (fs *filesystem) newDentry() *dentry { func (d *dentry) IncRef() { // d.refs may be 0 if d.fs.renameMu is locked, which serializes against // d.checkDropLocked(). - r := atomic.AddInt64(&d.refs, 1) + r := d.refs.Add(1) if d.LogRefs() { refsvfs2.LogIncRef(d, r) } @@ -497,11 +498,11 @@ func (d *dentry) IncRef() { // TryIncRef implements vfs.DentryImpl.TryIncRef. func (d *dentry) TryIncRef() bool { for { - r := atomic.LoadInt64(&d.refs) + r := d.refs.Load() if r <= 0 { return false } - if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { + if d.refs.CompareAndSwap(r, r+1) { if d.LogRefs() { refsvfs2.LogTryIncRef(d, r+1) } @@ -512,7 +513,7 @@ func (d *dentry) TryIncRef() bool { // DecRef implements vfs.DentryImpl.DecRef. func (d *dentry) DecRef(ctx context.Context) { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -526,7 +527,7 @@ func (d *dentry) DecRef(ctx context.Context) { } func (d *dentry) decRefLocked(ctx context.Context) { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -547,7 +548,7 @@ func (d *dentry) checkDropLocked(ctx context.Context) { // resolution, which requires renameMu, so if d.refs is zero then it will // remain zero while we hold renameMu for writing.) Dentries with a // negative reference count have already been destroyed. - if atomic.LoadInt64(&d.refs) != 0 { + if d.refs.Load() != 0 { return } @@ -569,10 +570,10 @@ func (d *dentry) checkDropLocked(ctx context.Context) { // * d.fs.renameMu must be locked for writing. // * d.refs == 0. func (d *dentry) destroyLocked(ctx context.Context) { - switch atomic.LoadInt64(&d.refs) { + switch d.refs.Load() { case 0: // Mark the dentry destroyed. - atomic.StoreInt64(&d.refs, -1) + d.refs.Store(-1) case -1: panic("overlay.dentry.destroyLocked() called on already destroyed dentry") default: @@ -608,7 +609,7 @@ func (d *dentry) RefType() string { // LeakMessage implements refsvfs2.CheckedObject.LeakMessage. func (d *dentry) LeakMessage() string { - return fmt.Sprintf("[overlay.dentry %p] reference count of %d instead of -1", d, atomic.LoadInt64(&d.refs)) + return fmt.Sprintf("[overlay.dentry %p] reference count of %d instead of -1", d, d.refs.Load()) } // LogRefs implements refsvfs2.CheckedObject.LogRefs. @@ -645,7 +646,7 @@ func (d *dentry) Watches() *vfs.Watches { // OnZeroWatches implements vfs.DentryImpl.OnZeroWatches. func (d *dentry) OnZeroWatches(ctx context.Context) { - if atomic.LoadInt64(&d.refs) == 0 { + if d.refs.Load() == 0 { d.fs.renameMu.Lock() d.checkDropLocked(ctx) d.fs.renameMu.Unlock() @@ -718,7 +719,7 @@ func (d *dentry) statInternalTo(ctx context.Context, opts *vfs.StatOptions, stat stat.UID = atomic.LoadUint32(&d.uid) stat.GID = atomic.LoadUint32(&d.gid) stat.Mode = uint16(atomic.LoadUint32(&d.mode)) - stat.Ino = atomic.LoadUint64(&d.ino) + stat.Ino = d.ino.Load() stat.DevMajor = atomic.LoadUint32(&d.devMajor) stat.DevMinor = atomic.LoadUint32(&d.devMinor) } diff --git a/pkg/sentry/fsimpl/overlay/save_restore.go b/pkg/sentry/fsimpl/overlay/save_restore.go index 54809f16c..61f408704 100644 --- a/pkg/sentry/fsimpl/overlay/save_restore.go +++ b/pkg/sentry/fsimpl/overlay/save_restore.go @@ -15,13 +15,11 @@ package overlay import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/refsvfs2" ) func (d *dentry) afterLoad() { - if atomic.LoadInt64(&d.refs) != -1 { + if d.refs.Load() != -1 { refsvfs2.Register(d) } } diff --git a/pkg/sentry/fsimpl/proc/task_files.go b/pkg/sentry/fsimpl/proc/task_files.go index e7cfef94b..368f96683 100644 --- a/pkg/sentry/fsimpl/proc/task_files.go +++ b/pkg/sentry/fsimpl/proc/task_files.go @@ -833,13 +833,13 @@ func (i *ioData) Generate(ctx context.Context, buf *bytes.Buffer) error { io := usage.IO{} io.Accumulate(i.IOUsage()) - fmt.Fprintf(buf, "char: %d\n", io.CharsRead) - fmt.Fprintf(buf, "wchar: %d\n", io.CharsWritten) - fmt.Fprintf(buf, "syscr: %d\n", io.ReadSyscalls) - fmt.Fprintf(buf, "syscw: %d\n", io.WriteSyscalls) - fmt.Fprintf(buf, "read_bytes: %d\n", io.BytesRead) - fmt.Fprintf(buf, "write_bytes: %d\n", io.BytesWritten) - fmt.Fprintf(buf, "cancelled_write_bytes: %d\n", io.BytesWriteCancelled) + fmt.Fprintf(buf, "char: %d\n", io.CharsRead.RacyLoad()) + fmt.Fprintf(buf, "wchar: %d\n", io.CharsWritten.RacyLoad()) + fmt.Fprintf(buf, "syscr: %d\n", io.ReadSyscalls.RacyLoad()) + fmt.Fprintf(buf, "syscw: %d\n", io.WriteSyscalls.RacyLoad()) + fmt.Fprintf(buf, "read_bytes: %d\n", io.BytesRead.RacyLoad()) + fmt.Fprintf(buf, "write_bytes: %d\n", io.BytesWritten.RacyLoad()) + fmt.Fprintf(buf, "cancelled_write_bytes: %d\n", io.BytesWriteCancelled.RacyLoad()) return nil } diff --git a/pkg/sentry/fsimpl/timerfd/BUILD b/pkg/sentry/fsimpl/timerfd/BUILD index 2b83d7d9a..8bd95074a 100644 --- a/pkg/sentry/fsimpl/timerfd/BUILD +++ b/pkg/sentry/fsimpl/timerfd/BUILD @@ -7,6 +7,7 @@ go_library( srcs = ["timerfd.go"], visibility = ["//pkg/sentry:internal"], deps = [ + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/fsimpl/timerfd/timerfd.go b/pkg/sentry/fsimpl/timerfd/timerfd.go index b740aa396..afea7e637 100644 --- a/pkg/sentry/fsimpl/timerfd/timerfd.go +++ b/pkg/sentry/fsimpl/timerfd/timerfd.go @@ -16,8 +16,7 @@ package timerfd import ( - "sync/atomic" - + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -43,7 +42,7 @@ type TimerFileDescription struct { // val is the number of timer expirations since the last successful // call to PRead, or SetTime. val must be accessed using atomic memory // operations. - val uint64 + val atomicbitops.Uint64 } var _ vfs.FileDescriptionImpl = (*TimerFileDescription)(nil) @@ -71,7 +70,7 @@ func (tfd *TimerFileDescription) Read(ctx context.Context, dst usermem.IOSequenc if dst.NumBytes() < sizeofUint64 { return 0, linuxerr.EINVAL } - if val := atomic.SwapUint64(&tfd.val, 0); val != 0 { + if val := tfd.val.Swap(0); val != 0 { var buf [sizeofUint64]byte hostarch.ByteOrder.PutUint64(buf[:], val) if _, err := dst.CopyOut(ctx, buf[:]); err != nil { @@ -99,13 +98,13 @@ func (tfd *TimerFileDescription) GetTime() (ktime.Time, ktime.Setting) { // of expirations to 0, and returns the previous setting and the time at which // it was observed. func (tfd *TimerFileDescription) SetTime(s ktime.Setting) (ktime.Time, ktime.Setting) { - return tfd.timer.SwapAnd(s, func() { atomic.StoreUint64(&tfd.val, 0) }) + return tfd.timer.SwapAnd(s, func() { tfd.val.Store(0) }) } // Readiness implements waiter.Waitable.Readiness. func (tfd *TimerFileDescription) Readiness(mask waiter.EventMask) waiter.EventMask { var ready waiter.EventMask - if atomic.LoadUint64(&tfd.val) != 0 { + if tfd.val.Load() != 0 { ready |= waiter.ReadableEvents } return ready @@ -144,7 +143,7 @@ func (tfd *TimerFileDescription) Release(context.Context) { // NotifyTimer implements ktime.TimerListener.NotifyTimer. func (tfd *TimerFileDescription) NotifyTimer(exp uint64, setting ktime.Setting) (ktime.Setting, bool) { - atomic.AddUint64(&tfd.val, exp) + tfd.val.Add(exp) tfd.events.Notify(waiter.ReadableEvents) return ktime.Setting{}, false } diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index 7fab2e5a4..56f9c1e4a 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -117,6 +117,7 @@ go_test( library = ":tmpfs", deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/fsimpl/tmpfs/directory.go b/pkg/sentry/fsimpl/tmpfs/directory.go index c25494c0b..2848d1019 100644 --- a/pkg/sentry/fsimpl/tmpfs/directory.go +++ b/pkg/sentry/fsimpl/tmpfs/directory.go @@ -18,6 +18,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -39,7 +40,7 @@ type directory struct { // numChildren is len(childMap), but accessed using atomic memory // operations to avoid locking in inode.statTo(). - numChildren int64 + numChildren atomicbitops.Int64 // childList is a list containing (1) child dentries and (2) fake dentries // (with inode == nil) that represent the iteration position of @@ -68,7 +69,7 @@ func (dir *directory) insertChildLocked(child *dentry, name string) { dir.childMap = make(map[string]*dentry) } dir.childMap[name] = child - atomic.AddInt64(&dir.numChildren, 1) + dir.numChildren.Add(1) dir.iterMu.Lock() dir.childList.PushBack(child) dir.iterMu.Unlock() @@ -77,7 +78,7 @@ func (dir *directory) insertChildLocked(child *dentry, name string) { // Preconditions: filesystem.mu must be locked for writing. func (dir *directory) removeChildLocked(child *dentry) { delete(dir.childMap, child.name) - atomic.AddInt64(&dir.numChildren, -1) + dir.numChildren.Add(-1) dir.iterMu.Lock() dir.childList.Remove(child) dir.iterMu.Unlock() diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 3a30e043f..ea97e8cac 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -21,6 +21,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -89,7 +90,7 @@ type regularFile struct { // either mutex, while writing requires holding both AND using atomics. // Readers that do not require consistency (like Stat) may read the // value atomically without holding either lock. - size uint64 + size atomicbitops.Uint64 } func (fs *filesystem) newRegularFile(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, parentDir *directory) *inode { @@ -144,7 +145,7 @@ func NewZeroFile(ctx context.Context, creds *auth.Credentials, mount *vfs.Mount, } rf := fd.inode().impl.(*regularFile) rf.memoryUsageKind = usage.Anonymous - rf.size = size + rf.size.Store(size) return &fd.vfsfd, err } @@ -173,7 +174,7 @@ func (rf *regularFile) truncate(newSize uint64) (bool, error) { // Preconditions: rf.inode.mu must be held. func (rf *regularFile) truncateLocked(newSize uint64) (bool, error) { - oldSize := rf.size + oldSize := rf.size.RacyLoad() if newSize == oldSize { // Nothing to do. return false, nil @@ -188,7 +189,7 @@ func (rf *regularFile) truncateLocked(newSize uint64) (bool, error) { return false, linuxerr.EPERM } // We only need to update the file size. - atomic.StoreUint64(&rf.size, newSize) + rf.size.Store(newSize) rf.dataMu.Unlock() return true, nil } @@ -200,7 +201,7 @@ func (rf *regularFile) truncateLocked(newSize uint64) (bool, error) { } // Update the file size. - atomic.StoreUint64(&rf.size, newSize) + rf.size.Store(newSize) rf.dataMu.Unlock() // Invalidate past translations of truncated pages. @@ -282,7 +283,7 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap. // Constrain translations to f.attr.Size (rounded up) to prevent // translation to pages that may be concurrently truncated. - pgend := fs.OffsetPageEnd(int64(rf.size)) + pgend := fs.OffsetPageEnd(int64(rf.size.RacyLoad())) var beyondEOF bool if required.End > pgend { if required.Start >= pgend { @@ -295,7 +296,7 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap. optional.End = pgend } - cerr := rf.data.Fill(ctx, required, optional, rf.size, rf.memFile, rf.memoryUsageKind, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { + cerr := rf.data.Fill(ctx, required, optional, rf.size.RacyLoad(), rf.memFile, rf.memoryUsageKind, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { // Newly-allocated pages are zeroed, so we don't need to do anything. return dsts.NumBytes(), nil }) @@ -350,7 +351,7 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint f.inode.mu.Lock() defer f.inode.mu.Unlock() - oldSize := f.size + oldSize := f.size.RacyLoad() size := offset + length if oldSize >= size { return nil @@ -428,7 +429,7 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // If the file is opened with O_APPEND, update offset to file size. if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 { // Locking f.inode.mu is sufficient for reading f.size. - offset = int64(f.size) + offset = int64(f.size.RacyLoad()) } if end := offset + srclen; end < offset { // Overflow. @@ -474,7 +475,7 @@ func (fd *regularFileFD) Seek(ctx context.Context, offset int64, whence int32) ( case linux.SEEK_CUR: offset += fd.off case linux.SEEK_END: - offset += int64(atomic.LoadUint64(&fd.inode().impl.(*regularFile).size)) + offset += int64(fd.inode().impl.(*regularFile).size.Load()) default: return 0, linuxerr.EINVAL } @@ -523,7 +524,7 @@ func putRegularFileReadWriter(rw *regularFileReadWriter) { func (rw *regularFileReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) { rw.file.dataMu.RLock() defer rw.file.dataMu.RUnlock() - size := rw.file.size + size := rw.file.size.RacyLoad() // Compute the range to read (limited by file size and overflow-checked). if rw.off >= size { @@ -595,7 +596,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, switch { case rw.file.seals&linux.F_SEAL_WRITE != 0: // Write sealed return 0, linuxerr.EPERM - case end > rw.file.size && rw.file.seals&linux.F_SEAL_GROW != 0: // Grow sealed + case end > rw.file.size.RacyLoad() && rw.file.seals&linux.F_SEAL_GROW != 0: // Grow sealed // When growth is sealed, Linux effectively allows writes which would // normally grow the file to partially succeed up to the current EOF, // rounded down to the page boundary before the EOF. @@ -610,7 +611,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, // // See Linux, mm/filemap.c:generic_perform_write() and // mm/shmem.c:shmem_write_begin(). - if pgstart := uint64(hostarch.Addr(rw.file.size).RoundDown()); end > pgstart { + if pgstart := uint64(hostarch.Addr(rw.file.size.RacyLoad()).RoundDown()); end > pgstart { end = pgstart } if end <= rw.off { @@ -673,8 +674,8 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, exitLoop: // If the write ends beyond the file's previous size, it causes the // file to grow. - if rw.off > rw.file.size { - atomic.StoreUint64(&rw.file.size, rw.off) + if rw.off > rw.file.size.RacyLoad() { + rw.file.size.Store(rw.off) } return done, retErr diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 80813f8bd..d64046f45 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -35,6 +35,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -82,7 +83,7 @@ type filesystem struct { // mu serializes changes to the Dentry tree. mu sync.RWMutex `state:"nosave"` - nextInoMinusOne uint64 // accessed using atomic memory operations + nextInoMinusOne atomicbitops.Uint64 // accessed using atomic memory operations root *dentry @@ -399,9 +400,9 @@ type inode struct { ino uint64 // immutable // Linux's tmpfs has no concept of btime. - atime int64 // nanoseconds - ctime int64 // nanoseconds - mtime int64 // nanoseconds + atime atomicbitops.Int64 // nanoseconds + ctime atomicbitops.Int64 // nanoseconds + mtime atomicbitops.Int64 // nanoseconds locks vfs.FileLocks @@ -430,12 +431,12 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth i.mode = uint32(mode) i.uid = uint32(kuid) i.gid = uint32(kgid) - i.ino = atomic.AddUint64(&fs.nextInoMinusOne, 1) + i.ino = fs.nextInoMinusOne.Add(1) // Tmpfs creation sets atime, ctime, and mtime to current time. now := fs.clock.Now().Nanoseconds() - i.atime = now - i.ctime = now - i.mtime = now + i.atime = atomicbitops.FromInt64(now) + i.ctime = atomicbitops.FromInt64(now) + i.mtime = atomicbitops.FromInt64(now) // i.nlink initialized by caller i.impl = impl i.refs.InitRefs() @@ -514,21 +515,21 @@ func (i *inode) statTo(stat *linux.Statx) { stat.GID = atomic.LoadUint32(&i.gid) stat.Mode = uint16(atomic.LoadUint32(&i.mode)) stat.Ino = i.ino - stat.Atime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&i.atime)) - stat.Ctime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&i.ctime)) - stat.Mtime = linux.NsecToStatxTimestamp(atomic.LoadInt64(&i.mtime)) + stat.Atime = linux.NsecToStatxTimestamp(i.atime.Load()) + stat.Ctime = linux.NsecToStatxTimestamp(i.ctime.Load()) + stat.Mtime = linux.NsecToStatxTimestamp(i.mtime.Load()) stat.DevMajor = linux.UNNAMED_MAJOR stat.DevMinor = i.fs.devMinor switch impl := i.impl.(type) { case *regularFile: stat.Mask |= linux.STATX_SIZE | linux.STATX_BLOCKS - stat.Size = uint64(atomic.LoadUint64(&impl.size)) + stat.Size = uint64(impl.size.Load()) // TODO(jamieliu): This should be impl.data.Span() / 512, but this is // too expensive to compute here. Cache it in regularFile. stat.Blocks = allocatedBlocksForSize(stat.Size) case *directory: // "20" is mm/shmem.c:BOGO_DIRENT_SIZE. - stat.Size = 20 * (2 + uint64(atomic.LoadInt64(&impl.numChildren))) + stat.Size = 20 * (2 + uint64(impl.numChildren.Load())) // stat.Blocks is 0. case *symlink: stat.Size = uint64(len(impl.target)) @@ -611,17 +612,17 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. now := i.fs.clock.Now().Nanoseconds() if mask&linux.STATX_ATIME != 0 { if stat.Atime.Nsec == linux.UTIME_NOW { - atomic.StoreInt64(&i.atime, now) + i.atime.Store(now) } else { - atomic.StoreInt64(&i.atime, stat.Atime.ToNsecCapped()) + i.atime.Store(stat.Atime.ToNsecCapped()) } needsCtimeBump = true } if mask&linux.STATX_MTIME != 0 { if stat.Mtime.Nsec == linux.UTIME_NOW { - atomic.StoreInt64(&i.mtime, now) + i.mtime.Store(now) } else { - atomic.StoreInt64(&i.mtime, stat.Mtime.ToNsecCapped()) + i.mtime.Store(stat.Mtime.ToNsecCapped()) } needsCtimeBump = true // Ignore the mtime bump, since we just set it ourselves. @@ -629,9 +630,9 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. } if mask&linux.STATX_CTIME != 0 { if stat.Ctime.Nsec == linux.UTIME_NOW { - atomic.StoreInt64(&i.ctime, now) + i.ctime.Store(now) } else { - atomic.StoreInt64(&i.ctime, stat.Ctime.ToNsecCapped()) + i.ctime.Store(stat.Ctime.ToNsecCapped()) } // Ignore the ctime bump, since we just set it ourselves. needsCtimeBump = false @@ -651,10 +652,10 @@ func (i *inode) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs. } if needsMtimeBump { - atomic.StoreInt64(&i.mtime, now) + i.mtime.Store(now) } if needsCtimeBump { - atomic.StoreInt64(&i.ctime, now) + i.ctime.Store(now) } return nil @@ -709,7 +710,7 @@ func (i *inode) touchAtime(mnt *vfs.Mount) { } now := i.fs.clock.Now().Nanoseconds() i.mu.Lock() - atomic.StoreInt64(&i.atime, now) + i.atime.Store(now) i.mu.Unlock() mnt.EndWrite() } @@ -718,7 +719,7 @@ func (i *inode) touchAtime(mnt *vfs.Mount) { func (i *inode) touchCtime() { now := i.fs.clock.Now().Nanoseconds() i.mu.Lock() - atomic.StoreInt64(&i.ctime, now) + i.ctime.Store(now) i.mu.Unlock() } @@ -726,8 +727,8 @@ func (i *inode) touchCtime() { func (i *inode) touchCMtime() { now := i.fs.clock.Now().Nanoseconds() i.mu.Lock() - atomic.StoreInt64(&i.mtime, now) - atomic.StoreInt64(&i.ctime, now) + i.mtime.Store(now) + i.ctime.Store(now) i.mu.Unlock() } @@ -736,8 +737,8 @@ func (i *inode) touchCMtime() { // * inode.mu must be locked. func (i *inode) touchCMtimeLocked() { now := i.fs.clock.Now().Nanoseconds() - atomic.StoreInt64(&i.mtime, now) - atomic.StoreInt64(&i.ctime, now) + i.mtime.Store(now) + i.ctime.Store(now) } func checkXattrName(name string) error { diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go b/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go index fc5323abc..8da9a6cb5 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go @@ -16,9 +16,9 @@ package tmpfs import ( "fmt" - "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fspath" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -26,7 +26,7 @@ import ( ) // nextFileID is used to generate unique file names. -var nextFileID int64 +var nextFileID atomicbitops.Int64 // newTmpfsRoot creates a new tmpfs mount, and returns the root. If the error // is not nil, then cleanup should be called when the root is no longer needed. @@ -63,7 +63,7 @@ func newFileFD(ctx context.Context, mode linux.FileMode) (*vfs.FileDescription, return nil, nil, err } - filename := fmt.Sprintf("tmpfs-test-file-%d", atomic.AddInt64(&nextFileID, 1)) + filename := fmt.Sprintf("tmpfs-test-file-%d", nextFileID.Add(1)) // Create the file that will be write/read. fd, err := vfsObj.OpenAt(ctx, creds, &vfs.PathOperation{ @@ -90,7 +90,7 @@ func newDirFD(ctx context.Context, mode linux.FileMode) (*vfs.FileDescription, f return nil, nil, err } - dirname := fmt.Sprintf("tmpfs-test-dir-%d", atomic.AddInt64(&nextFileID, 1)) + dirname := fmt.Sprintf("tmpfs-test-dir-%d", nextFileID.Add(1)) // Create the dir. if err := vfsObj.MkdirAt(ctx, creds, &vfs.PathOperation{ @@ -128,7 +128,7 @@ func newPipeFD(ctx context.Context, mode linux.FileMode) (*vfs.FileDescription, return nil, nil, err } - name := fmt.Sprintf("tmpfs-test-%d", atomic.AddInt64(&nextFileID, 1)) + name := fmt.Sprintf("tmpfs-test-%d", nextFileID.Add(1)) if err := vfsObj.MknodAt(ctx, creds, &vfs.PathOperation{ Root: root, diff --git a/pkg/sentry/fsimpl/verity/BUILD b/pkg/sentry/fsimpl/verity/BUILD index c12abdf33..8a6f2261a 100644 --- a/pkg/sentry/fsimpl/verity/BUILD +++ b/pkg/sentry/fsimpl/verity/BUILD @@ -26,6 +26,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/fsimpl/verity/save_restore.go b/pkg/sentry/fsimpl/verity/save_restore.go index 46b064342..95fd34792 100644 --- a/pkg/sentry/fsimpl/verity/save_restore.go +++ b/pkg/sentry/fsimpl/verity/save_restore.go @@ -15,13 +15,11 @@ package verity import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/refsvfs2" ) func (d *dentry) afterLoad() { - if atomic.LoadInt64(&d.refs) != -1 { + if d.refs.Load() != -1 { refsvfs2.Register(d) } } diff --git a/pkg/sentry/fsimpl/verity/verity.go b/pkg/sentry/fsimpl/verity/verity.go index 9d41371c5..297caf566 100644 --- a/pkg/sentry/fsimpl/verity/verity.go +++ b/pkg/sentry/fsimpl/verity/verity.go @@ -47,6 +47,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -379,7 +380,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt // Set the root's reference count to 2. One reference is returned to // the caller, and the other is held by fs to prevent the root from // being "cached" and subsequently evicted. - d.refs = 2 + d.refs = atomicbitops.FromInt64(2) lowerVD := vfs.MakeVirtualDentry(lowerMount, lowerMount.Root()) lowerVD.IncRef() d.lowerVD = lowerVD @@ -582,7 +583,7 @@ type dentry struct { // added to the cache or destroyed. If refs == -1, the dentry has // already been destroyed. refs is accessed using atomic memory // operations. - refs int64 + refs atomicbitops.Int64 // fs is the owning filesystem. fs is immutable. fs *filesystem @@ -664,7 +665,7 @@ func (fs *filesystem) newDentry() *dentry { // IncRef implements vfs.DentryImpl.IncRef. func (d *dentry) IncRef() { - r := atomic.AddInt64(&d.refs, 1) + r := d.refs.Add(1) if d.LogRefs() { refsvfs2.LogIncRef(d, r) } @@ -673,11 +674,11 @@ func (d *dentry) IncRef() { // TryIncRef implements vfs.DentryImpl.TryIncRef. func (d *dentry) TryIncRef() bool { for { - r := atomic.LoadInt64(&d.refs) + r := d.refs.Load() if r <= 0 { return false } - if atomic.CompareAndSwapInt64(&d.refs, r, r+1) { + if d.refs.CompareAndSwap(r, r+1) { if d.LogRefs() { refsvfs2.LogTryIncRef(d, r+1) } @@ -697,7 +698,7 @@ func (d *dentry) DecRef(ctx context.Context) { // d.checkCachingLocked, even if d's reference count reaches 0; callers are // responsible for ensuring that d.checkCachingLocked will be called later. func (d *dentry) decRefNoCaching() int64 { - r := atomic.AddInt64(&d.refs, -1) + r := d.refs.Add(-1) if d.LogRefs() { refsvfs2.LogDecRef(d, r) } @@ -713,10 +714,10 @@ func (d *dentry) decRefNoCaching() int64 { // * d.fs.renameMu must be locked for writing. // * d.refs == 0. func (d *dentry) destroyLocked(ctx context.Context) { - switch atomic.LoadInt64(&d.refs) { + switch d.refs.Load() { case 0: // Mark the dentry destroyed. - atomic.StoreInt64(&d.refs, -1) + d.refs.Store(-1) case -1: panic("verity.dentry.destroyLocked() called on already destroyed dentry") default: @@ -752,7 +753,7 @@ func (d *dentry) RefType() string { // LeakMessage implements refsvfs2.CheckedObject.LeakMessage. func (d *dentry) LeakMessage() string { - return fmt.Sprintf("[verity.dentry %p] reference count of %d instead of -1", d, atomic.LoadInt64(&d.refs)) + return fmt.Sprintf("[verity.dentry %p] reference count of %d instead of -1", d, d.refs.Load()) } // LogRefs implements refsvfs2.CheckedObject.LogRefs. @@ -797,7 +798,7 @@ func (d *dentry) OnZeroWatches(context.Context) { // renameMuWriteLocked is true; it may be temporarily unlocked. func (d *dentry) checkCachingLocked(ctx context.Context, renameMuWriteLocked bool) { d.cachingMu.Lock() - refs := atomic.LoadInt64(&d.refs) + refs := d.refs.Load() if refs == -1 { // Dentry has already been destroyed. d.cachingMu.Unlock() @@ -897,7 +898,7 @@ func (fs *filesystem) evictCachedDentryLocked(ctx context.Context) { victim.removeFromCacheLocked() // victim.refs may have become non-zero from an earlier path resolution // since it was inserted into fs.cachedDentries. - if atomic.LoadInt64(&victim.refs) != 0 { + if victim.refs.Load() != 0 { victim.cachingMu.Unlock() return } diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index babce93fc..2dcc5be95 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -39,6 +39,7 @@ import ( "time" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/cpuid" @@ -101,19 +102,18 @@ var FUSEEnabled = false type userCounters struct { uid auth.KUID - // +checkatomic - rlimitNProc uint64 + rlimitNProc atomicbitops.Uint64 } // incRLimitNProc increments the rlimitNProc counter. func (uc *userCounters) incRLimitNProc(ctx context.Context) error { lim := limits.FromContext(ctx).Get(limits.ProcessCount) creds := auth.CredentialsFromContext(ctx) - nproc := atomic.AddUint64(&uc.rlimitNProc, 1) + nproc := uc.rlimitNProc.Add(1) if nproc > lim.Cur && !creds.HasCapability(linux.CAP_SYS_ADMIN) && !creds.HasCapability(linux.CAP_SYS_RESOURCE) { - atomic.AddUint64(&uc.rlimitNProc, ^uint64(0)) + uc.rlimitNProc.Add(^uint64(0)) return linuxerr.EAGAIN } return nil @@ -121,7 +121,7 @@ func (uc *userCounters) incRLimitNProc(ctx context.Context) error { // decRLimitNProc decrements the rlimitNProc counter. func (uc *userCounters) decRLimitNProc() { - atomic.AddUint64(&uc.rlimitNProc, ^uint64(0)) + uc.rlimitNProc.Add(^uint64(0)) } // Kernel represents an emulated Linux kernel. It must be initialized by calling @@ -198,7 +198,7 @@ type Kernel struct { // // runningTasks must be accessed atomically. Increments from 0 to 1 are // further protected by runningTasksMu (see incRunningTasks). - runningTasks int64 + runningTasks atomicbitops.Int64 // cpuClock is incremented every linux.ClockTick. cpuClock is used to // measure task CPU usage, since sampling monotonicClock twice on every @@ -210,7 +210,7 @@ type Kernel struct { // doesn't provide this information. // // cpuClock is mutable, and is accessed using atomic memory operations. - cpuClock uint64 + cpuClock atomicbitops.Uint64 // cpuClockTicker increments cpuClock. cpuClockTicker *ktime.Timer `state:"nosave"` @@ -234,7 +234,7 @@ type Kernel struct { // uniqueID is used to generate unique identifiers. // // uniqueID is mutable, and is accessed using atomic memory operations. - uniqueID uint64 + uniqueID atomicbitops.Uint64 // nextInotifyCookie is a monotonically increasing counter used for // generating unique inotify event cookies. @@ -300,7 +300,7 @@ type Kernel struct { pipeMount *vfs.Mount // shmMount is the Mount used for anonymous files created by the - // memfd_create() syscalls. It is analagous to Linux's shm_mnt. + // memfd_create() syscalls. It is analogous to Linux's shm_mnt. shmMount *vfs.Mount // socketMount is the Mount used for sockets created by the socket() and @@ -757,7 +757,7 @@ func (k *Kernel) LoadFrom(ctx context.Context, r wire.Reader, timeReady chan str // UniqueID returns a unique identifier. func (k *Kernel) UniqueID() uint64 { - id := atomic.AddUint64(&k.uniqueID, 1) + id := k.uniqueID.Add(1) if id == 0 { panic("unique identifier generator wrapped around") } @@ -1202,10 +1202,10 @@ func (k *Kernel) resumeTimeLocked(ctx context.Context) { func (k *Kernel) incRunningTasks() { for { - tasks := atomic.LoadInt64(&k.runningTasks) + tasks := k.runningTasks.Load() if tasks != 0 { // Standard case. Simply increment. - if !atomic.CompareAndSwapInt64(&k.runningTasks, tasks, tasks+1) { + if !k.runningTasks.CompareAndSwap(tasks, tasks+1) { continue } return @@ -1213,18 +1213,18 @@ func (k *Kernel) incRunningTasks() { // Transition from 0 -> 1. Synchronize with other transitions and timer. k.runningTasksMu.Lock() - tasks = atomic.LoadInt64(&k.runningTasks) + tasks = k.runningTasks.Load() if tasks != 0 { // We're no longer the first task, no need to // re-enable. - atomic.AddInt64(&k.runningTasks, 1) + k.runningTasks.Add(1) k.runningTasksMu.Unlock() return } if !k.cpuClockTickerDisabled { // Timer was never disabled. - atomic.StoreInt64(&k.runningTasks, 1) + k.runningTasks.Store(1) k.runningTasksMu.Unlock() return } @@ -1260,12 +1260,12 @@ func (k *Kernel) incRunningTasks() { // don't matter. setting, exp := k.cpuClockTickerSetting.At(k.timekeeper.monotonicClock.Now()) if exp > 0 { - atomic.AddUint64(&k.cpuClock, exp) + k.cpuClock.Add(exp) } // Now that cpuClock is updated it is safe to allow other tasks to // transition to running. - atomic.StoreInt64(&k.runningTasks, 1) + k.runningTasks.Store(1) // N.B. we must unlock before calling Swap to maintain lock ordering. // @@ -1285,7 +1285,7 @@ func (k *Kernel) incRunningTasks() { } func (k *Kernel) decRunningTasks() { - tasks := atomic.AddInt64(&k.runningTasks, -1) + tasks := k.runningTasks.Add(-1) if tasks < 0 { panic(fmt.Sprintf("Invalid running count %d", tasks)) } @@ -1478,7 +1478,7 @@ func (k *Kernel) MonotonicClock() ktime.Clock { // CPUClockNow returns the current value of k.cpuClock. func (k *Kernel) CPUClockNow() uint64 { - return atomic.LoadUint64(&k.cpuClock) + return k.cpuClock.Load() } // Syslog returns the syslog. diff --git a/pkg/sentry/kernel/pipe/vfs.go b/pkg/sentry/kernel/pipe/vfs.go index 53507ccdc..2389157db 100644 --- a/pkg/sentry/kernel/pipe/vfs.go +++ b/pkg/sentry/kernel/pipe/vfs.go @@ -31,7 +31,7 @@ import ( // This file contains types enabling the pipe package to be used with the vfs // package. -// VFSPipe represents the actual pipe, analagous to an inode. VFSPipes should +// VFSPipe represents the actual pipe, analogous to an inode. VFSPipes should // not be copied. // // +stateify savable diff --git a/pkg/sentry/kernel/sessions.go b/pkg/sentry/kernel/sessions.go index f9f872522..26e3e3020 100644 --- a/pkg/sentry/kernel/sessions.go +++ b/pkg/sentry/kernel/sessions.go @@ -232,7 +232,7 @@ func (pg *ProcessGroup) Session() *Session { } // SendSignal sends a signal to all processes inside the process group. It is -// analagous to kernel/signal.c:kill_pgrp. +// analogous to kernel/signal.c:kill_pgrp. func (pg *ProcessGroup) SendSignal(info *linux.SignalInfo) error { tasks := pg.originator.TaskSet() tasks.mu.RLock() diff --git a/pkg/sentry/kernel/task.go b/pkg/sentry/kernel/task.go index d1988ea44..df682548b 100644 --- a/pkg/sentry/kernel/task.go +++ b/pkg/sentry/kernel/task.go @@ -20,6 +20,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/bpf" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -62,7 +63,7 @@ type Task struct { // but since it's used to detect cases where non-task goroutines // incorrectly access state owned by, or exclusive to, the task goroutine, // goid is always accessed using atomic memory operations. - goid int64 `state:"nosave"` + goid atomicbitops.Int64 `state:"nosave"` // runState is what the task goroutine is executing if it is not stopped. // If runState is nil, the task goroutine should exit or has exited. @@ -111,7 +112,7 @@ type Task struct { // // yieldCount is accessed using atomic memory operations. yieldCount is // owned by the task goroutine. - yieldCount uint64 + yieldCount atomicbitops.Uint64 // pendingSignals is the set of pending signals that may be handled only by // this task. @@ -128,7 +129,7 @@ type Task struct { // signal mutex is locked or if atomic memory operations are used, while // writing signalMask requires both). signalMask is owned by the task // goroutine. - signalMask linux.SignalSet + signalMask atomicbitops.Uint64 // If the task goroutine is currently executing Task.sigtimedwait, // realSignalMask is the previous value of signalMask, which has temporarily diff --git a/pkg/sentry/kernel/task_acct.go b/pkg/sentry/kernel/task_acct.go index dd364ae50..4b5ec7ca1 100644 --- a/pkg/sentry/kernel/task_acct.go +++ b/pkg/sentry/kernel/task_acct.go @@ -124,7 +124,8 @@ func (tg *ThreadGroup) IOUsage() *usage.IO { tg.pidns.owner.mu.RLock() defer tg.pidns.owner.mu.RUnlock() - io := *tg.ioUsage + var io usage.IO + tg.ioUsage.Clone(&io) // Account for active tasks. for t := tg.tasks.Front(); t != nil; t = t.Next() { io.Accumulate(t.IOUsage()) diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index 34d09ed0d..c03769123 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -57,7 +57,7 @@ type taskRunState interface { // make it visible in stack dumps. A goroutine for a given task can be identified // searching for Task.run()'s argument value. func (t *Task) run(threadID uintptr) { - atomic.StoreInt64(&t.goid, goid.Get()) + t.goid.Store(goid.Get()) // Construct t.blockingTimer here. We do this here because we can't // reconstruct t.blockingTimer during restore in Task.afterLoad(), because @@ -103,7 +103,7 @@ func (t *Task) run(threadID uintptr) { // Deferring this store triggers a false positive in the race // detector (https://github.com/golang/go/issues/42599). - atomic.StoreInt64(&t.goid, 0) + t.goid.Store(0) // Keep argument alive because stack trace for dead variables may not be correct. runtime.KeepAlive(threadID) return @@ -347,14 +347,14 @@ func (app *runApp) execute(t *Task) taskRunState { // assertTaskGoroutine panics if the caller is not running on t's task // goroutine. func (t *Task) assertTaskGoroutine() { - if got, want := goid.Get(), atomic.LoadInt64(&t.goid); got != want { + if got, want := goid.Get(), t.goid.Load(); got != want { panic(fmt.Sprintf("running on goroutine %d (task goroutine for kernel.Task %p is %d)", got, t, want)) } } // GoroutineID returns the ID of t's task goroutine. func (t *Task) GoroutineID() int64 { - return atomic.LoadInt64(&t.goid) + return t.goid.Load() } // waitGoroutineStoppedOrExited blocks until t's task goroutine stops or exits. @@ -373,6 +373,6 @@ func (tg *ThreadGroup) WaitExited() { // Yield yields the processor for the calling task. func (t *Task) Yield() { - atomic.AddUint64(&t.yieldCount, 1) + t.yieldCount.Add(1) runtime.Gosched() } diff --git a/pkg/sentry/kernel/task_sched.go b/pkg/sentry/kernel/task_sched.go index 9882f1c12..199483cf4 100644 --- a/pkg/sentry/kernel/task_sched.go +++ b/pkg/sentry/kernel/task_sched.go @@ -186,7 +186,7 @@ func (t *Task) cpuStatsAt(now uint64) usage.CPUStats { return usage.CPUStats{ UserTime: time.Duration(tsched.userTicksAt(now) * uint64(linux.ClockTick)), SysTime: time.Duration(tsched.sysTicksAt(now) * uint64(linux.ClockTick)), - VoluntarySwitches: atomic.LoadUint64(&t.yieldCount), + VoluntarySwitches: t.yieldCount.Load(), } } @@ -360,7 +360,7 @@ func (ticker *kernelCPUClockTicker) NotifyTimer(exp uint64, setting ktime.Settin // presumably task goroutines as well, from executing for a long period of // time. It's also necessary to prevent CPU clocks from seeing large // discontinuous jumps. - now := atomic.AddUint64(&ticker.k.cpuClock, 1) + now := ticker.k.cpuClock.Add(1) // Check thread group CPU timers. tgs := ticker.k.tasks.Root.ThreadGroupsAppend(ticker.tgs) @@ -451,11 +451,11 @@ func (ticker *kernelCPUClockTicker) NotifyTimer(exp uint64, setting ktime.Settin ticker.tgs = tgs[:0] // If nothing is running, we can disable the timer. - tasks := atomic.LoadInt64(&ticker.k.runningTasks) + tasks := ticker.k.runningTasks.Load() if tasks == 0 { ticker.k.runningTasksMu.Lock() defer ticker.k.runningTasksMu.Unlock() - tasks := atomic.LoadInt64(&ticker.k.runningTasks) + tasks := ticker.k.runningTasks.Load() if tasks != 0 { // Raced with a 0 -> 1 transition. return setting, false diff --git a/pkg/sentry/kernel/task_signals.go b/pkg/sentry/kernel/task_signals.go index 2e66e40ea..b2639a0e1 100644 --- a/pkg/sentry/kernel/task_signals.go +++ b/pkg/sentry/kernel/task_signals.go @@ -18,7 +18,6 @@ package kernel import ( "fmt" - "sync/atomic" "time" "gvisor.dev/gvisor/pkg/abi/linux" @@ -265,7 +264,7 @@ func (t *Task) deliverSignalToHandler(info *linux.SignalInfo, act linux.SigActio IO: mm, Bottom: sp, } - mask := t.signalMask + mask := linux.SignalSet(t.signalMask.Load()) if t.haveSavedSignalMask { mask = t.savedSignalMask } @@ -289,7 +288,7 @@ func (t *Task) deliverSignalToHandler(info *linux.SignalInfo, act linux.SigActio t.haveSavedSignalMask = false // Add our signal mask. - newMask := t.signalMask | act.Mask + newMask := linux.SignalSet(t.signalMask.Load()) | act.Mask if act.Flags&linux.SA_NODEFER == 0 { newMask |= linux.SignalSetOf(linux.Signal(info.Signo)) } @@ -345,8 +344,8 @@ func (t *Task) Sigtimedwait(set linux.SignalSet, timeout time.Duration) (*linux. // Unblock signals we're waiting for. Remember the original signal mask so // that Task.sendSignalTimerLocked doesn't discard ignored signals that // we're temporarily unblocking. - t.realSignalMask = t.signalMask - t.setSignalMaskLocked(t.signalMask & mask) + t.realSignalMask = linux.SignalSet(t.signalMask.RacyLoad()) + t.setSignalMaskLocked(t.realSignalMask & mask) // Wait for a timeout or new signal. t.tg.signalHandlers.mu.Unlock() @@ -437,7 +436,7 @@ func (t *Task) sendSignalTimerLocked(info *linux.SignalInfo, group bool, timer * // Linux's kernel/signal.c:__send_signal() => prepare_signal() => // sig_ignored(). ignored := computeAction(sig, t.tg.signalHandlers.actions[sig]) == SignalActionIgnore - if sigset := linux.SignalSetOf(sig); sigset&t.signalMask == 0 && sigset&t.realSignalMask == 0 && ignored && !t.hasTracer() { + if sigset := linux.SignalSetOf(sig); sigset&linux.SignalSet(t.signalMask.RacyLoad()) == 0 && sigset&t.realSignalMask == 0 && ignored && !t.hasTracer() { t.Debugf("Discarding ignored signal %d", sig) if timer != nil { timer.signalRejectedLocked() @@ -524,7 +523,7 @@ func (t *Task) canReceiveSignalLocked(sig linux.Signal) bool { t.signalQueue.Notify(waiter.EventMask(linux.MakeSignalSet(sig))) // - Do not choose tasks that are blocking the signal. - if linux.SignalSetOf(sig)&t.signalMask != 0 { + if linux.SignalSetOf(sig)&linux.SignalSet(t.signalMask.RacyLoad()) != 0 { return false } // - No need to check Task.exitState, as the exit path sets every bit in the @@ -571,21 +570,21 @@ func (t *Task) forceSignal(sig linux.Signal, unconditional bool) { } func (t *Task) forceSignalLocked(sig linux.Signal, unconditional bool) { - blocked := linux.SignalSetOf(sig)&t.signalMask != 0 + blocked := linux.SignalSetOf(sig)&linux.SignalSet(t.signalMask.RacyLoad()) != 0 act := t.tg.signalHandlers.actions[sig] ignored := act.Handler == linux.SIG_IGN if blocked || ignored || unconditional { act.Handler = linux.SIG_DFL t.tg.signalHandlers.actions[sig] = act if blocked { - t.setSignalMaskLocked(t.signalMask &^ linux.SignalSetOf(sig)) + t.setSignalMaskLocked(linux.SignalSet(t.signalMask.RacyLoad()) &^ linux.SignalSetOf(sig)) } } } // SignalMask returns a copy of t's signal mask. func (t *Task) SignalMask() linux.SignalSet { - return linux.SignalSet(atomic.LoadUint64((*uint64)(&t.signalMask))) + return linux.SignalSet(t.signalMask.Load()) } // SetSignalMask sets t's signal mask. @@ -603,8 +602,8 @@ func (t *Task) SetSignalMask(mask linux.SignalSet) { // Preconditions: The signal mutex must be locked. func (t *Task) setSignalMaskLocked(mask linux.SignalSet) { - oldMask := t.signalMask - atomic.StoreUint64((*uint64)(&t.signalMask), uint64(mask)) + oldMask := linux.SignalSet(t.signalMask.RacyLoad()) + t.signalMask.Store(uint64(mask)) // If the new mask blocks any signals that were not blocked by the old // mask, and at least one such signal is pending in tg.pendingSignals, and @@ -1015,7 +1014,7 @@ func (*runInterrupt) execute(t *Task) taskRunState { } // Are there signals pending? - if info := t.dequeueSignalLocked(t.signalMask); info != nil { + if info := t.dequeueSignalLocked(linux.SignalSet(t.signalMask.RacyLoad())); info != nil { t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) if linux.SignalSetOf(linux.Signal(info.Signo))&StopSignals != 0 { @@ -1083,7 +1082,7 @@ func (*runInterruptAfterSignalDeliveryStop) execute(t *Task) taskRunState { t.tg.signalHandlers.mu.Lock() t.tg.pidns.owner.mu.Unlock() // If the signal is masked, re-queue it. - if linux.SignalSetOf(sig)&t.signalMask != 0 { + if linux.SignalSetOf(sig)&linux.SignalSet(t.signalMask.RacyLoad()) != 0 { t.sendSignalLocked(info, false /* group */) t.tg.signalHandlers.mu.Unlock() return (*runInterrupt)(nil) diff --git a/pkg/sentry/kernel/task_start.go b/pkg/sentry/kernel/task_start.go index e88a5284d..fdf10dca6 100644 --- a/pkg/sentry/kernel/task_start.go +++ b/pkg/sentry/kernel/task_start.go @@ -16,6 +16,7 @@ package kernel import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/hostarch" @@ -141,7 +142,7 @@ func (ts *TaskSet) newTask(cfg *TaskConfig) (*Task, error) { }, runState: (*runApp)(nil), interruptChan: make(chan struct{}, 1), - signalMask: cfg.SignalMask, + signalMask: atomicbitops.FromUint64(uint64(cfg.SignalMask)), signalStack: linux.SignalStack{Flags: linux.SS_DISABLE}, image: *image, fsContext: cfg.FSContext, diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go index 64659d2ac..080cbb83a 100644 --- a/pkg/sentry/kernel/thread_group.go +++ b/pkg/sentry/kernel/thread_group.go @@ -516,7 +516,7 @@ func (tg *ThreadGroup) SetForegroundProcessGroup(tty *TTY, pgid ProcessGroupID) // signal is sent to all members of this background process group. // We need also need to check whether it is ignoring or blocking SIGTTOU. ignored := signalAction.Handler == linux.SIG_IGN - blocked := tg.leader.signalMask == linux.SignalSetOf(linux.SIGTTOU) + blocked := linux.SignalSet(tg.leader.signalMask.RacyLoad()) == linux.SignalSetOf(linux.SIGTTOU) if tg.processGroup.id != tg.processGroup.session.foreground.id && !ignored && !blocked { tg.leader.sendSignalLocked(SignalInfoPriv(linux.SIGTTOU), true) } diff --git a/pkg/sentry/kernel/timekeeper.go b/pkg/sentry/kernel/timekeeper.go index a68e495f3..5560b48d2 100644 --- a/pkg/sentry/kernel/timekeeper.go +++ b/pkg/sentry/kernel/timekeeper.go @@ -16,9 +16,9 @@ package kernel import ( "fmt" - "sync/atomic" "time" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/log" ktime "gvisor.dev/gvisor/pkg/sentry/kernel/time" "gvisor.dev/gvisor/pkg/sentry/memmap" @@ -57,7 +57,7 @@ type Timekeeper struct { monotonicOffset int64 `state:"nosave"` // monotonicLowerBound is the lowerBound for monotonic time. - monotonicLowerBound int64 `state:"nosave"` + monotonicLowerBound atomicbitops.Int64 `state:"nosave"` // restored, if non-nil, indicates that this Timekeeper was restored // from a state file. The clocks are not set until restored is closed. @@ -314,12 +314,12 @@ func (t *Timekeeper) GetTime(c sentrytime.ClockID) (int64, error) { // TSC and host TSC, which may not be perfectly in sync. To // work around this issue, ensure that the monotonic time is // always bounded by the last time read. - oldLowerBound := atomic.LoadInt64(&t.monotonicLowerBound) + oldLowerBound := t.monotonicLowerBound.Load() if now < oldLowerBound { now = oldLowerBound break } - if atomic.CompareAndSwapInt64(&t.monotonicLowerBound, oldLowerBound, now) { + if t.monotonicLowerBound.CompareAndSwap(oldLowerBound, now) { break } } diff --git a/pkg/sentry/platform/kvm/address_space.go b/pkg/sentry/platform/kvm/address_space.go index b1216c4c8..79ccbea35 100644 --- a/pkg/sentry/platform/kvm/address_space.go +++ b/pkg/sentry/platform/kvm/address_space.go @@ -15,8 +15,6 @@ package kvm import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/ring0/pagetables" @@ -27,7 +25,7 @@ import ( // dirtySet tracks vCPUs for invalidation. type dirtySet struct { - vCPUMasks []uint64 + vCPUMasks []atomicbitops.Uint64 } // forEach iterates over all CPUs in the dirty set. @@ -35,7 +33,7 @@ type dirtySet struct { //go:nosplit func (ds *dirtySet) forEach(m *machine, fn func(c *vCPU)) { for index := range ds.vCPUMasks { - mask := atomic.SwapUint64(&ds.vCPUMasks[index], 0) + mask := ds.vCPUMasks[index].Swap(0) if mask != 0 { for bit := 0; bit < 64; bit++ { if mask&(1<= n { + if c.guestExits.Load() >= n { t.Errorf("vdso calls trigger vmexit") } return false diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go index d806842ad..059ff561e 100644 --- a/pkg/sentry/platform/kvm/machine.go +++ b/pkg/sentry/platform/kvm/machine.go @@ -116,13 +116,13 @@ type vCPU struct { fd int // tid is the last set tid. - tid uint64 + tid atomicbitops.Uint64 // userExits is the count of user exits. - userExits uint64 + userExits atomicbitops.Uint64 // guestExits is the count of guest to host world switches. - guestExits uint64 + guestExits atomicbitops.Uint64 // faults is a count of world faults (informational only). faults uint32 @@ -505,7 +505,8 @@ func (m *machine) Put(c *vCPU) { // newDirtySet returns a new dirty set. func (m *machine) newDirtySet() *dirtySet { return &dirtySet{ - vCPUMasks: make([]uint64, (m.maxVCPUs+63)/64, (m.maxVCPUs+63)/64), + vCPUMasks: make([]atomicbitops.Uint64, + (m.maxVCPUs+63)/64, (m.maxVCPUs+63)/64), } } @@ -587,8 +588,8 @@ var pid = unix.Getpid() // // This effectively unwinds the state machine. func (c *vCPU) bounce(forceGuestExit bool) { - origGuestExits := atomic.LoadUint64(&c.guestExits) - origUserExits := atomic.LoadUint64(&c.userExits) + origGuestExits := c.guestExits.Load() + origUserExits := c.userExits.Load() for { switch state := atomic.LoadUint32(&c.state); state { case vCPUReady, vCPUWaiter: @@ -623,7 +624,7 @@ func (c *vCPU) bounce(forceGuestExit bool) { // under memory pressure. Since we already // marked ourselves as a waiter, we need to // ensure that a signal is actually delivered. - if err := unix.Tgkill(pid, int(atomic.LoadUint64(&c.tid)), bounceSignal); err == nil { + if err := unix.Tgkill(pid, int(c.tid.Load()), bounceSignal); err == nil { break } else if err.(unix.Errno) == unix.EAGAIN { continue @@ -647,8 +648,8 @@ func (c *vCPU) bounce(forceGuestExit bool) { // Check if we've missed the state transition, but // we can safely return at this point in time. - newGuestExits := atomic.LoadUint64(&c.guestExits) - newUserExits := atomic.LoadUint64(&c.userExits) + newGuestExits := c.guestExits.Load() + newUserExits := c.userExits.Load() if newUserExits != origUserExits && (!forceGuestExit || newGuestExits != origGuestExits) { return } diff --git a/pkg/sentry/platform/kvm/machine_amd64_unsafe.go b/pkg/sentry/platform/kvm/machine_amd64_unsafe.go index fbacea9ad..7da23c8dd 100644 --- a/pkg/sentry/platform/kvm/machine_amd64_unsafe.go +++ b/pkg/sentry/platform/kvm/machine_amd64_unsafe.go @@ -19,7 +19,6 @@ package kvm import ( "fmt" - "sync/atomic" "unsafe" "golang.org/x/sys/unix" @@ -46,7 +45,7 @@ func (c *vCPU) loadSegments(tid uint64) { 0); errno != 0 { throw("getting GS segment") } - atomic.StoreUint64(&c.tid, tid) + c.tid.Store(tid) } // setCPUID sets the CPUID to be used by the guest. diff --git a/pkg/sentry/platform/kvm/machine_arm64_unsafe.go b/pkg/sentry/platform/kvm/machine_arm64_unsafe.go index a06408a19..cd0f28635 100644 --- a/pkg/sentry/platform/kvm/machine_arm64_unsafe.go +++ b/pkg/sentry/platform/kvm/machine_arm64_unsafe.go @@ -20,7 +20,6 @@ package kvm import ( "fmt" "reflect" - "sync/atomic" "unsafe" "golang.org/x/sys/unix" @@ -234,7 +233,7 @@ func (c *vCPU) setSystemTime() error { func (c *vCPU) loadSegments(tid uint64) { // TODO(gvisor.dev/issue/1238): TLS is not supported. // Get TLS from tpidr_el0. - atomic.StoreUint64(&c.tid, tid) + c.tid.Store(tid) } func (c *vCPU) setOneRegister(reg *kvmOneReg) error { diff --git a/pkg/sentry/platform/kvm/machine_unsafe.go b/pkg/sentry/platform/kvm/machine_unsafe.go index 17b04194e..688acc897 100644 --- a/pkg/sentry/platform/kvm/machine_unsafe.go +++ b/pkg/sentry/platform/kvm/machine_unsafe.go @@ -30,6 +30,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" ) //go:linkname entersyscall runtime.entersyscall @@ -175,7 +176,7 @@ func (c *vCPU) setSignalMask() error { // seccompMmapHandlerCnt is a number of currently running seccompMmapHandler // instances. -var seccompMmapHandlerCnt int64 +var seccompMmapHandlerCnt atomicbitops.Int64 // seccompMmapSync waits for all currently runnuing seccompMmapHandler // instances. @@ -188,7 +189,7 @@ var seccompMmapHandlerCnt int64 // once, and the probability is racing with seccompMmapHandler is very low the // spinlock-like way looks more reasonable. func seccompMmapSync() { - for atomic.LoadInt64(&seccompMmapHandlerCnt) != 0 { + for seccompMmapHandlerCnt.Load() != 0 { runtime.Gosched() } } @@ -206,7 +207,7 @@ func seccompMmapHandler(context unsafe.Pointer) { return } - atomic.AddInt64(&seccompMmapHandlerCnt, 1) + seccompMmapHandlerCnt.Add(1) for i := uint32(0); i < atomic.LoadUint32(&machinePoolLen); i++ { m := machinePool[i].Load() if m == nil { @@ -235,5 +236,5 @@ func seccompMmapHandler(context unsafe.Pointer) { virtual += length } } - atomic.AddInt64(&seccompMmapHandlerCnt, -1) + seccompMmapHandlerCnt.Add(-1) } diff --git a/pkg/sentry/socket/BUILD b/pkg/sentry/socket/BUILD index 00f925166..a64c0b63c 100644 --- a/pkg/sentry/socket/BUILD +++ b/pkg/sentry/socket/BUILD @@ -11,6 +11,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/hostarch", "//pkg/marshal", diff --git a/pkg/sentry/socket/socket.go b/pkg/sentry/socket/socket.go index bf0599568..20abf7a63 100644 --- a/pkg/sentry/socket/socket.go +++ b/pkg/sentry/socket/socket.go @@ -20,11 +20,11 @@ package socket import ( "bytes" "fmt" - "sync/atomic" "time" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/marshal" @@ -485,32 +485,32 @@ type SendReceiveTimeout struct { // send is length of the send timeout in nanoseconds. // // send must be accessed atomically. - send int64 + send atomicbitops.Int64 // recv is length of the receive timeout in nanoseconds. // // recv must be accessed atomically. - recv int64 + recv atomicbitops.Int64 } // SetRecvTimeout implements Socket.SetRecvTimeout. func (to *SendReceiveTimeout) SetRecvTimeout(nanoseconds int64) { - atomic.StoreInt64(&to.recv, nanoseconds) + to.recv.Store(nanoseconds) } // RecvTimeout implements Socket.RecvTimeout. func (to *SendReceiveTimeout) RecvTimeout() int64 { - return atomic.LoadInt64(&to.recv) + return to.recv.Load() } // SetSendTimeout implements Socket.SetSendTimeout. func (to *SendReceiveTimeout) SetSendTimeout(nanoseconds int64) { - atomic.StoreInt64(&to.send, nanoseconds) + to.send.Store(nanoseconds) } // SendTimeout implements Socket.SendTimeout. func (to *SendReceiveTimeout) SendTimeout() int64 { - return atomic.LoadInt64(&to.send) + return to.send.Load() } // GetSockOptEmitUnimplementedEvent emits unimplemented event if name is valid. diff --git a/pkg/sentry/socket/unix/transport/host.go b/pkg/sentry/socket/unix/transport/host.go index 4bd2c5671..2ce193818 100644 --- a/pkg/sentry/socket/unix/transport/host.go +++ b/pkg/sentry/socket/unix/transport/host.go @@ -16,10 +16,10 @@ package transport import ( "fmt" - "sync/atomic" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" @@ -78,7 +78,7 @@ type HostConnectedEndpoint struct { // GetSockOpt and message splitting/rejection in SendMsg, but do not // prevent lots of small messages from filling the real send buffer // size on the host. - sndbuf int64 `state:"nosave"` + sndbuf atomicbitops.Int64 `state:"nosave"` // stype is the type of Unix socket. stype linux.SockType @@ -117,7 +117,7 @@ func (c *HostConnectedEndpoint) initFromOptions() *syserr.Error { } c.stype = linux.SockType(stype) - atomic.StoreInt64(&c.sndbuf, int64(sndbuf)) + c.sndbuf.Store(int64(sndbuf)) return nil } @@ -314,14 +314,14 @@ func (c *HostConnectedEndpoint) RecvQueuedSize() int64 { // SendMaxQueueSize implements Receiver.SendMaxQueueSize. func (c *HostConnectedEndpoint) SendMaxQueueSize() int64 { - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // RecvMaxQueueSize implements Receiver.RecvMaxQueueSize. func (c *HostConnectedEndpoint) RecvMaxQueueSize() int64 { // N.B. Unix sockets don't use the receive buffer. We'll claim it is // the same size as the send buffer. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } func (c *HostConnectedEndpoint) destroyLocked() { @@ -344,7 +344,7 @@ func (c *HostConnectedEndpoint) CloseUnread() {} func (c *HostConnectedEndpoint) SetSendBufferSize(v int64) (newSz int64) { // gVisor does not permit setting of SO_SNDBUF for host backed unix // domain sockets. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // SetReceiveBufferSize implements ConnectedEndpoint.SetReceiveBufferSize. @@ -352,7 +352,7 @@ func (c *HostConnectedEndpoint) SetReceiveBufferSize(v int64) (newSz int64) { // gVisor does not permit setting of SO_RCVBUF for host backed unix // domain sockets. Receive buffer does not have any effect for unix // sockets and we claim to be the same as send buffer. - return atomic.LoadInt64(&c.sndbuf) + return c.sndbuf.Load() } // SCMConnectedEndpoint represents an endpoint backed by a host fd that was diff --git a/pkg/sentry/usage/BUILD b/pkg/sentry/usage/BUILD index 8e2b3ed79..83a81f243 100644 --- a/pkg/sentry/usage/BUILD +++ b/pkg/sentry/usage/BUILD @@ -15,6 +15,7 @@ go_library( "//:sandbox", ], deps = [ + "//pkg/atomicbitops", "//pkg/bits", "//pkg/memutil", "//pkg/sync", diff --git a/pkg/sentry/usage/io.go b/pkg/sentry/usage/io.go index dfcd3a49d..15be485ae 100644 --- a/pkg/sentry/usage/io.go +++ b/pkg/sentry/usage/io.go @@ -14,77 +14,86 @@ package usage -import ( - "sync/atomic" -) +import "gvisor.dev/gvisor/pkg/atomicbitops" // IO contains I/O-related statistics. // // +stateify savable type IO struct { // CharsRead is the number of bytes read by read syscalls. - CharsRead uint64 + CharsRead atomicbitops.Uint64 // CharsWritten is the number of bytes written by write syscalls. - CharsWritten uint64 + CharsWritten atomicbitops.Uint64 // ReadSyscalls is the number of read syscalls. - ReadSyscalls uint64 + ReadSyscalls atomicbitops.Uint64 // WriteSyscalls is the number of write syscalls. - WriteSyscalls uint64 + WriteSyscalls atomicbitops.Uint64 // The following counter is only meaningful when Sentry has internal // pagecache. // BytesRead is the number of bytes actually read into pagecache. - BytesRead uint64 + BytesRead atomicbitops.Uint64 // BytesWritten is the number of bytes actually written from pagecache. - BytesWritten uint64 + BytesWritten atomicbitops.Uint64 // BytesWriteCancelled is the number of bytes not written out due to // truncation. - BytesWriteCancelled uint64 + BytesWriteCancelled atomicbitops.Uint64 +} + +// Clone turns other into a clone of i. +func (i *IO) Clone(other *IO) { + other.CharsRead.Store(i.CharsRead.Load()) + other.CharsWritten.Store(i.CharsWritten.Load()) + other.ReadSyscalls.Store(i.ReadSyscalls.Load()) + other.WriteSyscalls.Store(i.WriteSyscalls.Load()) + other.BytesRead.Store(i.BytesRead.Load()) + other.BytesWritten.Store(i.BytesWritten.Load()) + other.BytesWriteCancelled.Store(i.BytesWriteCancelled.Load()) } // AccountReadSyscall does the accounting for a read syscall. func (i *IO) AccountReadSyscall(bytes int64) { - atomic.AddUint64(&i.ReadSyscalls, 1) + i.ReadSyscalls.Add(1) if bytes > 0 { - atomic.AddUint64(&i.CharsRead, uint64(bytes)) + i.CharsRead.Add(uint64(bytes)) } } // AccountWriteSyscall does the accounting for a write syscall. func (i *IO) AccountWriteSyscall(bytes int64) { - atomic.AddUint64(&i.WriteSyscalls, 1) + i.WriteSyscalls.Add(1) if bytes > 0 { - atomic.AddUint64(&i.CharsWritten, uint64(bytes)) + i.CharsWritten.Add(uint64(bytes)) } } // AccountReadIO does the accounting for a read IO into the file system. func (i *IO) AccountReadIO(bytes int64) { if bytes > 0 { - atomic.AddUint64(&i.BytesRead, uint64(bytes)) + i.BytesRead.Add(uint64(bytes)) } } // AccountWriteIO does the accounting for a write IO into the file system. func (i *IO) AccountWriteIO(bytes int64) { if bytes > 0 { - atomic.AddUint64(&i.BytesWritten, uint64(bytes)) + i.BytesWritten.Add(uint64(bytes)) } } // Accumulate adds up io usages. func (i *IO) Accumulate(io *IO) { - atomic.AddUint64(&i.CharsRead, atomic.LoadUint64(&io.CharsRead)) - atomic.AddUint64(&i.CharsWritten, atomic.LoadUint64(&io.CharsWritten)) - atomic.AddUint64(&i.ReadSyscalls, atomic.LoadUint64(&io.ReadSyscalls)) - atomic.AddUint64(&i.WriteSyscalls, atomic.LoadUint64(&io.WriteSyscalls)) - atomic.AddUint64(&i.BytesRead, atomic.LoadUint64(&io.BytesRead)) - atomic.AddUint64(&i.BytesWritten, atomic.LoadUint64(&io.BytesWritten)) - atomic.AddUint64(&i.BytesWriteCancelled, atomic.LoadUint64(&io.BytesWriteCancelled)) + i.CharsRead.Add(io.CharsRead.Load()) + i.CharsWritten.Add(io.CharsWritten.Load()) + i.ReadSyscalls.Add(io.ReadSyscalls.Load()) + i.WriteSyscalls.Add(io.WriteSyscalls.Load()) + i.BytesRead.Add(io.BytesRead.Load()) + i.BytesWritten.Add(io.BytesWritten.Load()) + i.BytesWriteCancelled.Add(io.BytesWriteCancelled.Load()) } diff --git a/pkg/sentry/usage/memory.go b/pkg/sentry/usage/memory.go index d9df890c4..9966bd1e1 100644 --- a/pkg/sentry/usage/memory.go +++ b/pkg/sentry/usage/memory.go @@ -17,9 +17,9 @@ package usage import ( "fmt" "os" - "sync/atomic" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/bits" "gvisor.dev/gvisor/pkg/memutil" "gvisor.dev/gvisor/pkg/sync" @@ -76,16 +76,27 @@ const ( Mapped ) -// MemoryStats tracks application memory usage in bytes. All fields correspond to the +// memoryStats tracks application memory usage in bytes. All fields correspond to the // memory category with the same name. This object is thread-safe if accessed // through the provided methods. The public fields may be safely accessed // directly on a copy of the object obtained from Memory.Copy(). +type memoryStats struct { + System atomicbitops.Uint64 + Anonymous atomicbitops.Uint64 + PageCache atomicbitops.Uint64 + Tmpfs atomicbitops.Uint64 + // Lazily updated based on the value in RTMapped. + Mapped atomicbitops.Uint64 + Ramdiskfs atomicbitops.Uint64 +} + +// MemoryStats tracks application memory usage in bytes. All fields correspond +// to the memory category with the same name. type MemoryStats struct { System uint64 Anonymous uint64 PageCache uint64 Tmpfs uint64 - // Lazily updated based on the value in RTMapped. Mapped uint64 Ramdiskfs uint64 } @@ -102,14 +113,14 @@ type MemoryStats struct { // initially zeroed. Any added field will be ignored by an older API and will be // zero if read by a newer API. type RTMemoryStats struct { - RTMapped uint64 + RTMapped atomicbitops.Uint64 } // MemoryLocked is Memory with access methods. type MemoryLocked struct { mu sync.RWMutex - // MemoryStats records the memory stats. - MemoryStats + // memoryStats records the memory stats. + memoryStats // RTMemoryStats records the memory stats that need to be exposed through // shared page. *RTMemoryStats @@ -154,17 +165,17 @@ var MemoryAccounting *MemoryLocked func (m *MemoryLocked) incLocked(val uint64, kind MemoryKind) { switch kind { case System: - atomic.AddUint64(&m.System, val) + m.System.Add(val) case Anonymous: - atomic.AddUint64(&m.Anonymous, val) + m.Anonymous.Add(val) case PageCache: - atomic.AddUint64(&m.PageCache, val) + m.PageCache.Add(val) case Mapped: - atomic.AddUint64(&m.RTMapped, val) + m.RTMapped.Add(val) case Tmpfs: - atomic.AddUint64(&m.Tmpfs, val) + m.Tmpfs.Add(val) case Ramdiskfs: - atomic.AddUint64(&m.Ramdiskfs, val) + m.Ramdiskfs.Add(val) default: panic(fmt.Sprintf("invalid memory kind: %v", kind)) } @@ -182,17 +193,17 @@ func (m *MemoryLocked) Inc(val uint64, kind MemoryKind) { func (m *MemoryLocked) decLocked(val uint64, kind MemoryKind) { switch kind { case System: - atomic.AddUint64(&m.System, ^(val - 1)) + m.System.Add(^(val - 1)) case Anonymous: - atomic.AddUint64(&m.Anonymous, ^(val - 1)) + m.Anonymous.Add(^(val - 1)) case PageCache: - atomic.AddUint64(&m.PageCache, ^(val - 1)) + m.PageCache.Add(^(val - 1)) case Mapped: - atomic.AddUint64(&m.RTMapped, ^(val - 1)) + m.RTMapped.Add(^(val - 1)) case Tmpfs: - atomic.AddUint64(&m.Tmpfs, ^(val - 1)) + m.Tmpfs.Add(^(val - 1)) case Ramdiskfs: - atomic.AddUint64(&m.Ramdiskfs, ^(val - 1)) + m.Ramdiskfs.Add(^(val - 1)) default: panic(fmt.Sprintf("invalid memory kind: %v", kind)) } @@ -223,12 +234,12 @@ func (m *MemoryLocked) Move(val uint64, to MemoryKind, from MemoryKind) { // // Precondition: must be called when locked. func (m *MemoryLocked) totalLocked() (total uint64) { - total += atomic.LoadUint64(&m.System) - total += atomic.LoadUint64(&m.Anonymous) - total += atomic.LoadUint64(&m.PageCache) - total += atomic.LoadUint64(&m.RTMapped) - total += atomic.LoadUint64(&m.Tmpfs) - total += atomic.LoadUint64(&m.Ramdiskfs) + total += m.System.Load() + total += m.Anonymous.Load() + total += m.PageCache.Load() + total += m.RTMapped.Load() + total += m.Tmpfs.Load() + total += m.Ramdiskfs.Load() return } @@ -247,8 +258,14 @@ func (m *MemoryLocked) Total() uint64 { func (m *MemoryLocked) Copy() (MemoryStats, uint64) { m.mu.Lock() defer m.mu.Unlock() - ms := m.MemoryStats - ms.Mapped = m.RTMapped + ms := MemoryStats{ + System: m.System.RacyLoad(), + Anonymous: m.Anonymous.RacyLoad(), + PageCache: m.PageCache.RacyLoad(), + Tmpfs: m.Tmpfs.RacyLoad(), + Mapped: m.RTMapped.RacyLoad(), + Ramdiskfs: m.Ramdiskfs.RacyLoad(), + } return ms, m.totalLocked() } diff --git a/pkg/sentry/vfs/BUILD b/pkg/sentry/vfs/BUILD index 978b818cf..7bac5b13c 100644 --- a/pkg/sentry/vfs/BUILD +++ b/pkg/sentry/vfs/BUILD @@ -133,6 +133,7 @@ go_test( library = ":vfs", deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/sentry/contexttest", diff --git a/pkg/sentry/vfs/file_description_impl_util_test.go b/pkg/sentry/vfs/file_description_impl_util_test.go index 5b6acefb8..ea8fb69b8 100644 --- a/pkg/sentry/vfs/file_description_impl_util_test.go +++ b/pkg/sentry/vfs/file_description_impl_util_test.go @@ -18,10 +18,10 @@ import ( "bytes" "fmt" "io" - "sync/atomic" "testing" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/sentry/contexttest" @@ -39,12 +39,12 @@ type fileDescription struct { // genCount contains the number of times its DynamicBytesSource.Generate() // implementation has been called. type genCount struct { - count uint64 // accessed using atomic memory ops + count atomicbitops.Uint64 } // Generate implements DynamicBytesSource.Generate. func (g *genCount) Generate(ctx context.Context, buf *bytes.Buffer) error { - fmt.Fprintf(buf, "%d", atomic.AddUint64(&g.count, 1)) + fmt.Fprintf(buf, "%d", g.count.Add(1)) return nil } diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index ff64d494b..9a2bf2b0d 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -23,6 +23,7 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/refsvfs2" @@ -76,7 +77,7 @@ type Mount struct { // The lower 63 bits of refs are a reference count. The MSB of refs is set // if the Mount has been eagerly umounted, as by umount(2) without the // MNT_DETACH flag. refs is accessed using atomic memory operations. - refs int64 + refs atomicbitops.Int64 // children is the set of all Mounts for which Mount.key.parent is this // Mount. children is protected by VirtualFilesystem.mountMu. @@ -91,18 +92,18 @@ type Mount struct { // Mount.CheckBeginWrite() that have not yet been paired with a call to // Mount.EndWrite(). The MSB of writers is set if MS_RDONLY is in effect. // writers is accessed using atomic memory operations. - writers int64 + writers atomicbitops.Int64 } func newMount(vfs *VirtualFilesystem, fs *Filesystem, root *Dentry, mntns *MountNamespace, opts *MountOptions) *Mount { mnt := &Mount{ - ID: atomic.AddUint64(&vfs.lastMountID, 1), + ID: vfs.lastMountID.Add(1), Flags: opts.Flags, vfs: vfs, fs: fs, root: root, ns: mntns, - refs: 1, + refs: atomicbitops.FromInt64(1), } if opts.ReadOnly { mnt.setReadOnlyLocked(true) @@ -349,7 +350,7 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti if !vd.mount.umounted { expectedRefs = 2 } - if atomic.LoadInt64(&vd.mount.refs)&^math.MinInt64 != expectedRefs { // mask out MSB + if vd.mount.refs.Load()&^math.MinInt64 != expectedRefs { // mask out MSB vfs.mounts.seq.EndWrite() vfs.mountMu.Unlock() return linuxerr.EBUSY @@ -410,11 +411,11 @@ func (vfs *VirtualFilesystem) umountRecursiveLocked(mnt *Mount, opts *umountRecu } if opts.eager { for { - refs := atomic.LoadInt64(&mnt.refs) + refs := mnt.refs.Load() if refs < 0 { break } - if atomic.CompareAndSwapInt64(&mnt.refs, refs, refs|math.MinInt64) { + if mnt.refs.CompareAndSwap(refs, refs|math.MinInt64) { break } } @@ -494,11 +495,11 @@ func (vfs *VirtualFilesystem) disconnectLocked(mnt *Mount) VirtualDentry { // tryIncMountedRef does not require that a reference is held on mnt. func (mnt *Mount) tryIncMountedRef() bool { for { - r := atomic.LoadInt64(&mnt.refs) + r := mnt.refs.Load() if r <= 0 { // r < 0 => MSB set => eagerly unmounted return false } - if atomic.CompareAndSwapInt64(&mnt.refs, r, r+1) { + if mnt.refs.CompareAndSwap(r, r+1) { if mnt.LogRefs() { refsvfs2.LogTryIncRef(mnt, r+1) } @@ -511,7 +512,7 @@ func (mnt *Mount) tryIncMountedRef() bool { func (mnt *Mount) IncRef() { // In general, negative values for mnt.refs are valid because the MSB is // the eager-unmount bit. - r := atomic.AddInt64(&mnt.refs, 1) + r := mnt.refs.Add(1) if mnt.LogRefs() { refsvfs2.LogIncRef(mnt, r) } @@ -519,7 +520,7 @@ func (mnt *Mount) IncRef() { // DecRef decrements mnt's reference count. func (mnt *Mount) DecRef(ctx context.Context) { - r := atomic.AddInt64(&mnt.refs, -1) + r := mnt.refs.Add(-1) if mnt.LogRefs() { refsvfs2.LogDecRef(mnt, r) } @@ -554,7 +555,7 @@ func (mnt *Mount) RefType() string { // LeakMessage implements refsvfs2.CheckedObject.LeakMessage. func (mnt *Mount) LeakMessage() string { - return fmt.Sprintf("[vfs.Mount %p] reference count of %d instead of 0", mnt, atomic.LoadInt64(&mnt.refs)) + return fmt.Sprintf("[vfs.Mount %p] reference count of %d instead of 0", mnt, mnt.refs.Load()) } // LogRefs implements refsvfs2.CheckedObject.LogRefs. @@ -802,8 +803,8 @@ func (vfs *VirtualFilesystem) SetMountReadOnly(mnt *Mount, ro bool) error { // If CheckBeginWrite succeeds, EndWrite must be called when the write // operation is finished. func (mnt *Mount) CheckBeginWrite() error { - if atomic.AddInt64(&mnt.writers, 1) < 0 { - atomic.AddInt64(&mnt.writers, -1) + if mnt.writers.Add(1) < 0 { + mnt.writers.Add(-1) return linuxerr.EROFS } return nil @@ -812,29 +813,29 @@ func (mnt *Mount) CheckBeginWrite() error { // EndWrite indicates that a write operation signaled by a previous successful // call to CheckBeginWrite has finished. func (mnt *Mount) EndWrite() { - atomic.AddInt64(&mnt.writers, -1) + mnt.writers.Add(-1) } // Preconditions: VirtualFilesystem.mountMu must be locked. func (mnt *Mount) setReadOnlyLocked(ro bool) error { - if oldRO := atomic.LoadInt64(&mnt.writers) < 0; oldRO == ro { + if oldRO := mnt.writers.Load() < 0; oldRO == ro { return nil } if ro { - if !atomic.CompareAndSwapInt64(&mnt.writers, 0, math.MinInt64) { + if !mnt.writers.CompareAndSwap(0, math.MinInt64) { return linuxerr.EBUSY } return nil } // Unset MSB without dropping any temporary increments from failed calls to // mnt.CheckBeginWrite(). - atomic.AddInt64(&mnt.writers, math.MinInt64) + mnt.writers.Add(math.MinInt64) return nil } // ReadOnly returns true if mount is readonly. func (mnt *Mount) ReadOnly() bool { - return atomic.LoadInt64(&mnt.writers) < 0 + return mnt.writers.Load() < 0 } // Filesystem returns the mounted Filesystem. It does not take a reference on diff --git a/pkg/sentry/vfs/mount_unsafe.go b/pkg/sentry/vfs/mount_unsafe.go index c7a78d8f8..e7eaa838b 100644 --- a/pkg/sentry/vfs/mount_unsafe.go +++ b/pkg/sentry/vfs/mount_unsafe.go @@ -20,6 +20,7 @@ import ( "sync/atomic" "unsafe" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/gohacks" "gvisor.dev/gvisor/pkg/sync" ) @@ -92,7 +93,7 @@ type mountTable struct { // anyway (cf. runtime.bucketShift()), and length isn't used by lookup; // thus this bit packing gets us more bits for the length (vs. storing // length and cap in separate uint32s) for ~free. - size uint64 + size atomicbitops.Uint64 slots unsafe.Pointer `state:"nosave"` // []mountSlot; never nil after Init } @@ -146,7 +147,7 @@ func init() { // Init must be called exactly once on each mountTable before use. func (mt *mountTable) Init() { - mt.size = mtInitOrder + mt.size = atomicbitops.FromUint64(mtInitOrder) mt.slots = newMountTableSlots(mtInitCap) } @@ -167,7 +168,7 @@ func (mt *mountTable) Lookup(parent *Mount, point *Dentry) *Mount { loop: for { epoch := mt.seq.BeginRead() - size := atomic.LoadUint64(&mt.size) + size := mt.size.Load() slots := atomic.LoadPointer(&mt.slots) if !mt.seq.ReadOk(epoch) { continue @@ -209,7 +210,7 @@ loop: // Range calls f on each Mount in mt. If f returns false, Range stops iteration // and returns immediately. func (mt *mountTable) Range(f func(*Mount) bool) { - tcap := uintptr(1) << (mt.size & mtSizeOrderMask) + tcap := uintptr(1) << (mt.size.Load() & mtSizeOrderMask) slotPtr := mt.slots last := unsafe.Pointer(uintptr(mt.slots) + ((tcap - 1) * mountSlotBytes)) for { @@ -248,12 +249,12 @@ func (mt *mountTable) insertSeqed(mount *Mount) { // // (len+1) / cap <= mtMaxLoadNum / mtMaxLoadDen // (len+1) * mtMaxLoadDen <= mtMaxLoadNum * cap - tlen := mt.size >> mtSizeLenLSB - order := mt.size & mtSizeOrderMask + tlen := mt.size.RacyLoad() >> mtSizeLenLSB + order := mt.size.RacyLoad() & mtSizeOrderMask tcap := uintptr(1) << order if ((tlen + 1) * mtMaxLoadDen) <= (uint64(mtMaxLoadNum) << order) { // Atomically insert the new element into the table. - atomic.AddUint64(&mt.size, mtSizeLenOne) + mt.size.Add(mtSizeLenOne) mtInsertLocked(mt.slots, tcap, unsafe.Pointer(mount), hash) return } @@ -287,7 +288,7 @@ func (mt *mountTable) insertSeqed(mount *Mount) { // Insert the new element into the new table. mtInsertLocked(newSlots, newCap, unsafe.Pointer(mount), hash) // Switch to the new table. - atomic.AddUint64(&mt.size, mtSizeLenOne|mtSizeOrderOne) + mt.size.Add(mtSizeLenOne | mtSizeOrderOne) atomic.StorePointer(&mt.slots, newSlots) } @@ -342,7 +343,7 @@ func (mt *mountTable) Remove(mount *Mount) { // * mt must contain mount. func (mt *mountTable) removeSeqed(mount *Mount) { hash := mount.key.hash() - tcap := uintptr(1) << (mt.size & mtSizeOrderMask) + tcap := uintptr(1) << (mt.size.RacyLoad() & mtSizeOrderMask) mask := tcap - 1 slots := mt.slots off := (hash & mask) * mountSlotBytes @@ -372,7 +373,7 @@ func (mt *mountTable) removeSeqed(mount *Mount) { slot = nextSlot } atomic.StorePointer(&slot.value, nil) - atomic.AddUint64(&mt.size, mtSizeLenNegOne) + mt.size.Add(mtSizeLenNegOne) return } if checkInvariants && slotValue == nil { diff --git a/pkg/sentry/vfs/save_restore.go b/pkg/sentry/vfs/save_restore.go index 7d84c4c4e..42bf95ee2 100644 --- a/pkg/sentry/vfs/save_restore.go +++ b/pkg/sentry/vfs/save_restore.go @@ -122,7 +122,7 @@ func (mnt *Mount) loadKey(vd VirtualDentry) { mnt.setKey(vd) } // afterLoad is called by stateify. func (mnt *Mount) afterLoad() { - if atomic.LoadInt64(&mnt.refs) != 0 { + if mnt.refs.Load() != 0 { refsvfs2.Register(mnt) } } diff --git a/pkg/sentry/vfs/vfs.go b/pkg/sentry/vfs/vfs.go index a8b8d9ae5..00f8efce9 100644 --- a/pkg/sentry/vfs/vfs.go +++ b/pkg/sentry/vfs/vfs.go @@ -40,6 +40,7 @@ import ( "path" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -90,7 +91,7 @@ type VirtualFilesystem struct { // lastMountID is the last allocated mount ID. lastMountID is accessed // using atomic memory operations. - lastMountID uint64 + lastMountID atomicbitops.Uint64 // anonMount is a Mount, not included in mounts or mountpoints, // representing an anonFilesystem. anonMount is used to back diff --git a/pkg/shim/proc/BUILD b/pkg/shim/proc/BUILD index c8527a6d9..653596ea5 100644 --- a/pkg/shim/proc/BUILD +++ b/pkg/shim/proc/BUILD @@ -20,6 +20,7 @@ go_library( "//shim:__subpackages__", ], deps = [ + "//pkg/atomicbitops", "//pkg/cleanup", "//pkg/shim/runsc", "//pkg/shim/utils", diff --git a/pkg/shim/proc/io.go b/pkg/shim/proc/io.go index 0e8a1a8cb..65f35ad62 100644 --- a/pkg/shim/proc/io.go +++ b/pkg/shim/proc/io.go @@ -21,12 +21,12 @@ import ( "io" "os" "sync" - "sync/atomic" "github.com/containerd/containerd/log" "github.com/containerd/fifo" runc "github.com/containerd/go-runc" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/atomicbitops" ) // TODO(random-liu): This file can be a util. @@ -97,7 +97,7 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w } } else { if sameFile != nil { - sameFile.count++ + sameFile.count.Add(1) i.dest(sameFile, nil) continue } @@ -107,7 +107,7 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w if stdout == stderr { sameFile = &countingWriteCloser{ WriteCloser: fw, - count: 1, + count: atomicbitops.FromInt64(1), } } } @@ -134,11 +134,11 @@ func copyPipes(ctx context.Context, rio runc.IO, stdin, stdout, stderr string, w // countingWriteCloser masks io.Closer() until close has been invoked a certain number of times. type countingWriteCloser struct { io.WriteCloser - count int64 + count atomicbitops.Int64 } func (c *countingWriteCloser) Close() error { - if atomic.AddInt64(&c.count, -1) > 0 { + if c.count.Add(-1) > 0 { return nil } return c.WriteCloser.Close() diff --git a/pkg/syncevent/receiver.go b/pkg/syncevent/receiver.go index 5c86e5400..fbbeaecdd 100644 --- a/pkg/syncevent/receiver.go +++ b/pkg/syncevent/receiver.go @@ -15,8 +15,6 @@ package syncevent import ( - "sync/atomic" - "gvisor.dev/gvisor/pkg/atomicbitops" ) @@ -28,7 +26,7 @@ import ( type Receiver struct { // pending is the set of pending events. pending is accessed using atomic // memory operations. - pending uint64 + pending atomicbitops.Uint64 // cb is notified when new events become pending. cb is immutable after // Init(). @@ -54,12 +52,12 @@ func (r *Receiver) Init(cb ReceiverCallback) { // Pending returns the set of pending events. func (r *Receiver) Pending() Set { - return Set(atomic.LoadUint64(&r.pending)) + return Set(r.pending.Load()) } // Notify sets the given events as pending. func (r *Receiver) Notify(es Set) { - p := Set(atomic.LoadUint64(&r.pending)) + p := Set(r.pending.Load()) // Optimization: Skip the atomic CAS on r.pending if all events are // already pending. if p&es == es { @@ -68,7 +66,7 @@ func (r *Receiver) Notify(es Set) { // When this is uncontended (the common case), CAS is faster than // atomic-OR because the former is inlined and the latter (which we // implement in assembly ourselves) is not. - if !atomic.CompareAndSwapUint64(&r.pending, uint64(p), uint64(p|es)) { + if !r.pending.CompareAndSwap(uint64(p), uint64(p|es)) { // If the CAS fails, fall back to atomic-OR. atomicbitops.OrUint64(&r.pending, uint64(es)) } @@ -77,7 +75,7 @@ func (r *Receiver) Notify(es Set) { // Ack unsets the given events as pending. func (r *Receiver) Ack(es Set) { - p := Set(atomic.LoadUint64(&r.pending)) + p := Set(r.pending.Load()) // Optimization: Skip the atomic CAS on r.pending if all events are // already not pending. if p&es == 0 { @@ -86,7 +84,7 @@ func (r *Receiver) Ack(es Set) { // When this is uncontended (the common case), CAS is faster than // atomic-AND because the former is inlined and the latter (which we // implement in assembly ourselves) is not. - if !atomic.CompareAndSwapUint64(&r.pending, uint64(p), uint64(p&^es)) { + if !r.pending.CompareAndSwap(uint64(p), uint64(p&^es)) { // If the CAS fails, fall back to atomic-AND. atomicbitops.AndUint64(&r.pending, ^uint64(es)) } @@ -99,5 +97,5 @@ func (r *Receiver) Ack(es Set) { // followed by a conditional call to Ack when the caller expects events to be // pending (e.g. after a call to ReceiverCallback.NotifyPending()). func (r *Receiver) PendingAndAckAll() Set { - return Set(atomic.SwapUint64(&r.pending, 0)) + return Set(r.pending.Swap(0)) } diff --git a/pkg/tcpip/link/sharedmem/pipe/BUILD b/pkg/tcpip/link/sharedmem/pipe/BUILD index 87020ec08..13681ac54 100644 --- a/pkg/tcpip/link/sharedmem/pipe/BUILD +++ b/pkg/tcpip/link/sharedmem/pipe/BUILD @@ -10,7 +10,12 @@ go_library( "rx.go", "tx.go", ], - visibility = ["//visibility:public"], + visibility = [ + "//visibility:public", + ], + deps = [ + "//pkg/atomicbitops", + ], ) go_test( diff --git a/pkg/tcpip/link/sharedmem/pipe/pipe_unsafe.go b/pkg/tcpip/link/sharedmem/pipe/pipe_unsafe.go index 62d17029e..6d4a8d5ea 100644 --- a/pkg/tcpip/link/sharedmem/pipe/pipe_unsafe.go +++ b/pkg/tcpip/link/sharedmem/pipe/pipe_unsafe.go @@ -15,8 +15,9 @@ package pipe import ( - "sync/atomic" "unsafe" + + "gvisor.dev/gvisor/pkg/atomicbitops" ) func (p *pipe) write(idx uint64, v uint64) { @@ -25,11 +26,11 @@ func (p *pipe) write(idx uint64, v uint64) { } func (p *pipe) writeAtomic(idx uint64, v uint64) { - ptr := (*uint64)(unsafe.Pointer(&p.buffer[idx&offsetMask:][:8][0])) - atomic.StoreUint64(ptr, v) + ptr := (*atomicbitops.Uint64)(unsafe.Pointer(&p.buffer[idx&offsetMask:][:8][0])) + ptr.Store(v) } func (p *pipe) readAtomic(idx uint64) uint64 { - ptr := (*uint64)(unsafe.Pointer(&p.buffer[idx&offsetMask:][:8][0])) - return atomic.LoadUint64(ptr) + ptr := (*atomicbitops.Uint64)(unsafe.Pointer(&p.buffer[idx&offsetMask:][:8][0])) + return ptr.Load() } diff --git a/pkg/tcpip/socketops.go b/pkg/tcpip/socketops.go index 90a5f2d1e..9223afdd7 100644 --- a/pkg/tcpip/socketops.go +++ b/pkg/tcpip/socketops.go @@ -229,7 +229,7 @@ type SocketOptions struct { getSendBufferLimits GetSendBufferLimits `state:"manual"` // sendBufferSize determines the send buffer size for this socket. - sendBufferSize atomicbitops.AlignedAtomicInt64 + sendBufferSize atomicbitops.Int64 // getReceiveBufferLimits provides the handler to get the min, default and // max size for receive buffer. It is initialized at the creation time and @@ -237,7 +237,7 @@ type SocketOptions struct { getReceiveBufferLimits GetReceiveBufferLimits `state:"manual"` // receiveBufferSize determines the receive buffer size for this socket. - receiveBufferSize atomicbitops.AlignedAtomicInt64 + receiveBufferSize atomicbitops.Int64 // mu protects the access to the below fields. mu sync.Mutex `state:"nosave"` diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index fec566e4e..e4a3aeb06 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -58,10 +58,10 @@ type ResumableEndpoint interface { } // uniqueIDGenerator is a default unique ID generator. -type uniqueIDGenerator atomicbitops.AlignedAtomicUint64 +type uniqueIDGenerator atomicbitops.Uint64 func (u *uniqueIDGenerator) UniqueID() uint64 { - return ((*atomicbitops.AlignedAtomicUint64)(u)).Add(1) + return ((*atomicbitops.Uint64)(u)).Add(1) } // Stack is a networking stack, with all supported protocols, NICs, and route diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index ede0f5b9e..63096ce0a 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -1354,7 +1354,7 @@ type NetworkProtocolNumber uint32 // // +stateify savable type StatCounter struct { - count atomicbitops.AlignedAtomicUint64 + count atomicbitops.Uint64 } // Increment adds one to the counter. @@ -2278,12 +2278,10 @@ func (s Stats) FillIn() Stats { return s } -// Clone returns a copy of the TransportEndpointStats by atomically reading -// each field. -func (src *TransportEndpointStats) Clone() TransportEndpointStats { - var dst TransportEndpointStats - clone(reflect.ValueOf(&dst).Elem(), reflect.ValueOf(src).Elem()) - return dst +// Clone clones a copy of the TransportEndpointStats into dst by atomically +// reading each field. +func (src *TransportEndpointStats) Clone(dst *TransportEndpointStats) { + clone(reflect.ValueOf(dst).Elem(), reflect.ValueOf(src).Elem()) } func clone(dst reflect.Value, src reflect.Value) { diff --git a/pkg/tcpip/transport/testing/context/context.go b/pkg/tcpip/transport/testing/context/context.go index 1ece8a94b..64eec1591 100644 --- a/pkg/tcpip/transport/testing/context/context.go +++ b/pkg/tcpip/transport/testing/context/context.go @@ -18,6 +18,7 @@ package context import ( "bytes" + "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -208,8 +209,9 @@ func (c *Context) CreateRawEndpointForFlow(flow TestFlow, transport tcpip.Transp // CheckEndpointWriteStats checks that the write statistic related to the given // error has been incremented as expected. -func (c *Context) CheckEndpointWriteStats(incr uint64, want tcpip.TransportEndpointStats, err tcpip.Error) { - got := c.EP.Stats().(*tcpip.TransportEndpointStats).Clone() +func (c *Context) CheckEndpointWriteStats(incr uint64, want *tcpip.TransportEndpointStats, err tcpip.Error) { + var got tcpip.TransportEndpointStats + c.EP.Stats().(*tcpip.TransportEndpointStats).Clone(&got) switch err.(type) { case nil: want.PacketsSent.IncrementBy(incr) @@ -224,17 +226,18 @@ func (c *Context) CheckEndpointWriteStats(incr uint64, want tcpip.TransportEndpo default: want.SendErrors.SendToNetworkFailed.IncrementBy(incr) } - if got != want { - c.T.Errorf("Endpoint stats not matching for error %s: got %#v, want %#v", err, got, want) + if !reflect.DeepEqual(&got, want) { + c.T.Errorf("Endpoint stats not matching for error %s: got %#v, want %#v", err, &got, want) } } // CheckEndpointReadStats checks that the read statistic related to the given // error has been incremented as expected. -func (c *Context) CheckEndpointReadStats(incr uint64, want tcpip.TransportEndpointStats, err tcpip.Error) { +func (c *Context) CheckEndpointReadStats(incr uint64, want *tcpip.TransportEndpointStats, err tcpip.Error) { c.T.Helper() - got := c.EP.Stats().(*tcpip.TransportEndpointStats).Clone() + var got tcpip.TransportEndpointStats + c.EP.Stats().(*tcpip.TransportEndpointStats).Clone(&got) switch err.(type) { case nil, *tcpip.ErrWouldBlock: case *tcpip.ErrClosedForReceive: @@ -242,8 +245,8 @@ func (c *Context) CheckEndpointReadStats(incr uint64, want tcpip.TransportEndpoi default: c.T.Errorf("Endpoint error missing stats update for err %s", err) } - if got != want { - c.T.Errorf("Endpoint stats not matching for error %s: got %#v, want %#v", err, got, want) + if !reflect.DeepEqual(&got, want) { + c.T.Errorf("Endpoint stats not matching for error %s: got %#v, want %#v", err, &got, want) } } @@ -276,7 +279,8 @@ func (c *Context) readFromEndpoint(expectations readExpectations, checkers ...ch defer c.WQ.EventUnregister(&we) // Take a snapshot of the stats to validate them at the end of the test. - epstats := c.EP.Stats().(*tcpip.TransportEndpointStats).Clone() + var epstats tcpip.TransportEndpointStats + c.EP.Stats().(*tcpip.TransportEndpointStats).Clone(&epstats) var buf bytes.Buffer res, err := c.EP.Read(&buf, tcpip.ReadOptions{NeedRemoteAddr: true}) @@ -293,7 +297,7 @@ func (c *Context) readFromEndpoint(expectations readExpectations, checkers ...ch } if expectations.readShouldFail && err != nil { - c.CheckEndpointReadStats(1, epstats, err) + c.CheckEndpointReadStats(1, &epstats, err) return } @@ -329,7 +333,7 @@ func (c *Context) readFromEndpoint(expectations readExpectations, checkers ...ch f(c.T, res.ControlMessages) } - c.CheckEndpointReadStats(1, epstats, err) + c.CheckEndpointReadStats(1, &epstats, err) } // ReadFromEndpointExpectSuccess attempts to reads from the endpoint and diff --git a/pkg/tcpip/transport/udp/udp_test.go b/pkg/tcpip/transport/udp/udp_test.go index d154a3e27..aee793c31 100644 --- a/pkg/tcpip/transport/udp/udp_test.go +++ b/pkg/tcpip/transport/udp/udp_test.go @@ -426,7 +426,8 @@ func TestV4ReadBroadcastOnBoundToWildcard(t *testing.T) { func testFailingWrite(c *context.Context, flow context.TestFlow, payloadSize int, wantErr tcpip.Error) { c.T.Helper() // Take a snapshot of the stats to validate them at the end of the test. - epstats := c.EP.Stats().(*tcpip.TransportEndpointStats).Clone() + var epstats tcpip.TransportEndpointStats + c.EP.Stats().(*tcpip.TransportEndpointStats).Clone(&epstats) h := flow.MakeHeader4Tuple(context.Outgoing) writeDstAddr := flow.MapAddrIfApplicable(h.Dst.Addr) @@ -435,7 +436,7 @@ func testFailingWrite(c *context.Context, flow context.TestFlow, payloadSize int _, gotErr := c.EP.Write(&r, tcpip.WriteOptions{ To: &tcpip.FullAddress{Addr: writeDstAddr, Port: h.Dst.Port}, }) - c.CheckEndpointWriteStats(1, epstats, gotErr) + c.CheckEndpointWriteStats(1, &epstats, gotErr) if gotErr != wantErr { c.T.Fatalf("Write returned unexpected error: got %v, want %v", gotErr, wantErr) } @@ -468,7 +469,8 @@ func testWriteWithoutDestination(c *context.Context, flow context.TestFlow, chec func testWriteNoVerify(c *context.Context, flow context.TestFlow, setDest bool) buffer.View { c.T.Helper() // Take a snapshot of the stats to validate them at the end of the test. - epstats := c.EP.Stats().(*tcpip.TransportEndpointStats).Clone() + var epstats tcpip.TransportEndpointStats + c.EP.Stats().(*tcpip.TransportEndpointStats).Clone(&epstats) writeOpts := tcpip.WriteOptions{} if setDest { @@ -488,7 +490,7 @@ func testWriteNoVerify(c *context.Context, flow context.TestFlow, setDest bool) if n != int64(len(payload)) { c.T.Fatalf("Bad number of bytes written: got %v, want %v", n, len(payload)) } - c.CheckEndpointWriteStats(1, epstats, err) + c.CheckEndpointWriteStats(1, &epstats, err) return payload } diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD index 313881f80..48a3ec5a7 100644 --- a/runsc/sandbox/BUILD +++ b/runsc/sandbox/BUILD @@ -14,6 +14,7 @@ go_library( "//runsc:__subpackages__", ], deps = [ + "//pkg/atomicbitops", "//pkg/cleanup", "//pkg/control/client", "//pkg/control/server", diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 4eaeeefcf..6aab7c815 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -25,7 +25,6 @@ import ( "os/exec" "strconv" "strings" - "sync/atomic" "syscall" "time" @@ -33,6 +32,7 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/syndtr/gocapability/capability" "golang.org/x/sys/unix" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/control/client" "gvisor.dev/gvisor/pkg/control/server" @@ -54,16 +54,15 @@ import ( // pid is an atomic type that implements JSON marshal/unmarshal interfaces. type pid struct { - // +checkatomics - val int64 + val atomicbitops.Int64 } func (p *pid) store(pid int) { - atomic.StoreInt64(&p.val, int64(pid)) + p.val.Store(int64(pid)) } func (p *pid) load() int { - return int(atomic.LoadInt64(&p.val)) + return int(p.val.Load()) } // UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON. diff --git a/tools/checkaligned/BUILD b/tools/checkaligned/BUILD new file mode 100644 index 000000000..2e7e77207 --- /dev/null +++ b/tools/checkaligned/BUILD @@ -0,0 +1,14 @@ +load("//tools:defs.bzl", "go_library") + +package(licenses = ["notice"]) + +go_library( + name = "checkaligned", + srcs = ["checkaligned.go"], + nogo = False, + stateify = False, + visibility = ["//tools/nogo:__subpackages__"], + deps = [ + "@org_golang_x_tools//go/analysis:go_default_library", + ], +) diff --git a/tools/checkaligned/checkaligned.go b/tools/checkaligned/checkaligned.go new file mode 100644 index 000000000..0c4977af8 --- /dev/null +++ b/tools/checkaligned/checkaligned.go @@ -0,0 +1,86 @@ +// Copyright 2022 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package checkaligned ensures that atomic (u)int64 operations happen +// exclusively via the atomicbitops package. +package checkaligned + +import ( + "fmt" + "go/ast" + + "golang.org/x/tools/go/analysis" +) + +// Analyzer defines the entrypoint. +var Analyzer = &analysis.Analyzer{ + Name: "checkaligned", + Doc: "prohibits direct use of 64 bit atomic operations", + Run: run, +} + +// blocklist lists prohibited identifiers in the atomic package. +// +// TODO(b/228378998): We should do this for 32 bit values too. Can also further +// genericize this to ban other things we don't like (e.g. os.File). +var blocklist = []string{ + "AddInt64", + "AddUint64", + "CompareAndSwapInt64", + "CompareAndSwapUint64", + "LoadInt64", + "LoadUint64", + "StoreInt64", + "StoreUint64", + "SwapInt64", + "SwapUint64", +} + +func run(pass *analysis.Pass) (interface{}, error) { + // atomicbitops uses 64 bit values safely. + if pass.Pkg.Name() == "atomicbitops" { + return nil, nil + } + + for _, file := range pass.Files { + ast.Inspect(file, func(node ast.Node) bool { + // Only look at selector expressions (e.g. "foo.Bar"). + selExpr, ok := node.(*ast.SelectorExpr) + if !ok { + return true + } + + // Package names are always identifiers and do not refer to objects. + pkgIdent, ok := selExpr.X.(*ast.Ident) + if !ok || pkgIdent.Obj != nil { + return true + } + + // Please don't trick this checker by renaming the atomic import. + if pkgIdent.Name != "atomic" { + return false + } + + for _, blocked := range blocklist { + if selExpr.Sel.Name == blocked { + pass.Reportf(selExpr.Pos(), fmt.Sprintf("don't call atomic.%s; use the atomicbitops package instead", blocked)) + } + } + + return false + }) + } + + return nil, nil +} diff --git a/tools/nogo/check/BUILD b/tools/nogo/check/BUILD index 1f602ac56..84d768154 100644 --- a/tools/nogo/check/BUILD +++ b/tools/nogo/check/BUILD @@ -13,6 +13,7 @@ go_library( visibility = ["//tools/nogo:__subpackages__"], deps = [ "//runsc/flag", + "//tools/checkaligned", "//tools/checkescape", "//tools/checkinfo", "//tools/checklinkname", diff --git a/tools/nogo/check/analyzers.go b/tools/nogo/check/analyzers.go index dc3f3bb06..1108ca840 100644 --- a/tools/nogo/check/analyzers.go +++ b/tools/nogo/check/analyzers.go @@ -49,6 +49,7 @@ import ( "honnef.co/go/tools/staticcheck" "honnef.co/go/tools/stylecheck" + "gvisor.dev/gvisor/tools/checkaligned" "gvisor.dev/gvisor/tools/checkescape" "gvisor.dev/gvisor/tools/checkinfo" "gvisor.dev/gvisor/tools/checklinkname" @@ -182,6 +183,7 @@ func init() { register(&plainAnalyzer{checkunsafe.Analyzer}) register(&plainAnalyzer{checklinkname.Analyzer}) register(&plainAnalyzer{checklocks.Analyzer}) + register(&plainAnalyzer{checkaligned.Analyzer}) // Add all staticcheck analyzers. for _, a := range staticcheck.Analyzers {