From 019e0c930183cf42e7ebaf22c0466d5deff2d15c Mon Sep 17 00:00:00 2001 From: Kevin Krakauer Date: Tue, 5 Apr 2022 15:13:30 -0700 Subject: [PATCH] use atomicbitops, not sync/atomic, in refsvfs2 Since refsvfs2 are used in netstack, we should use atomicbitops to avoid breaking 32-bit builds. On 64-bit builds there is no performance difference. PiperOrigin-RevId: 439687980 --- pkg/lisafs/BUILD | 1 + pkg/refsvfs2/refs_template.go | 18 +++++++++--------- pkg/sentry/fsimpl/cgroupfs/BUILD | 1 + pkg/sentry/fsimpl/devpts/BUILD | 1 + pkg/sentry/fsimpl/fuse/BUILD | 1 + pkg/sentry/fsimpl/host/BUILD | 1 + pkg/sentry/fsimpl/kernfs/BUILD | 2 ++ pkg/sentry/fsimpl/mqfs/BUILD | 1 + pkg/sentry/fsimpl/proc/BUILD | 1 + pkg/sentry/fsimpl/sys/BUILD | 1 + pkg/sentry/fsimpl/tmpfs/BUILD | 1 + pkg/sentry/kernel/BUILD | 1 + pkg/sentry/kernel/shm/BUILD | 1 + pkg/sentry/socket/unix/BUILD | 1 + pkg/sentry/socket/unix/transport/BUILD | 1 + pkg/sentry/vfs/BUILD | 1 + pkg/tcpip/link/tun/BUILD | 1 + pkg/tcpip/transport/tcp/BUILD | 1 + 18 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pkg/lisafs/BUILD b/pkg/lisafs/BUILD index fbe853b79..46d4f7b66 100644 --- a/pkg/lisafs/BUILD +++ b/pkg/lisafs/BUILD @@ -88,6 +88,7 @@ go_library( marshal = True, deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/cleanup", "//pkg/context", "//pkg/fdchannel", diff --git a/pkg/refsvfs2/refs_template.go b/pkg/refsvfs2/refs_template.go index 55b0a60a1..c88870660 100644 --- a/pkg/refsvfs2/refs_template.go +++ b/pkg/refsvfs2/refs_template.go @@ -18,8 +18,8 @@ package refs_template import ( "fmt" - "sync/atomic" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/refsvfs2" ) @@ -57,13 +57,13 @@ type Refs struct { // Speculative references are used for TryIncRef, to avoid a CompareAndSwap // loop. See IncRef, DecRef and TryIncRef for details of how these fields are // used. - refCount int64 + refCount atomicbitops.AlignedAtomicInt64 } // InitRefs initializes r with one reference and, if enabled, activates leak // checking. func (r *Refs) InitRefs() { - atomic.StoreInt64(&r.refCount, 1) + r.refCount.Store(1) refsvfs2.Register(r) } @@ -85,14 +85,14 @@ func (r *Refs) LogRefs() bool { // ReadRefs returns the current number of references. The returned count is // inherently racy and is unsafe to use without external synchronization. func (r *Refs) ReadRefs() int64 { - return atomic.LoadInt64(&r.refCount) + return r.refCount.Load() } // IncRef implements refs.RefCounter.IncRef. // //go:nosplit func (r *Refs) IncRef() { - v := atomic.AddInt64(&r.refCount, 1) + v := r.refCount.Add(1) if enableLogging { refsvfs2.LogIncRef(r, v) } @@ -110,14 +110,14 @@ func (r *Refs) IncRef() { //go:nosplit func (r *Refs) TryIncRef() bool { const speculativeRef = 1 << 32 - if v := atomic.AddInt64(&r.refCount, speculativeRef); int32(v) == 0 { + if v := r.refCount.Add(speculativeRef); int32(v) == 0 { // This object has already been freed. - atomic.AddInt64(&r.refCount, -speculativeRef) + r.refCount.Add(-speculativeRef) return false } // Turn into a real reference. - v := atomic.AddInt64(&r.refCount, -speculativeRef+1) + v := r.refCount.Add(-speculativeRef + 1) if enableLogging { refsvfs2.LogTryIncRef(r, v) } @@ -137,7 +137,7 @@ func (r *Refs) TryIncRef() bool { // //go:nosplit func (r *Refs) DecRef(destroy func()) { - v := atomic.AddInt64(&r.refCount, -1) + v := r.refCount.Add(-1) if enableLogging { refsvfs2.LogDecRef(r, v) } diff --git a/pkg/sentry/fsimpl/cgroupfs/BUILD b/pkg/sentry/fsimpl/cgroupfs/BUILD index 60ee5ede2..68e49b367 100644 --- a/pkg/sentry/fsimpl/cgroupfs/BUILD +++ b/pkg/sentry/fsimpl/cgroupfs/BUILD @@ -30,6 +30,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/bitmap", "//pkg/context", "//pkg/coverage", diff --git a/pkg/sentry/fsimpl/devpts/BUILD b/pkg/sentry/fsimpl/devpts/BUILD index e0b879339..9ceaf7584 100644 --- a/pkg/sentry/fsimpl/devpts/BUILD +++ b/pkg/sentry/fsimpl/devpts/BUILD @@ -28,6 +28,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/log", diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD index 4a8356eb3..1a97f240f 100644 --- a/pkg/sentry/fsimpl/fuse/BUILD +++ b/pkg/sentry/fsimpl/fuse/BUILD @@ -46,6 +46,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/fsimpl/host/BUILD b/pkg/sentry/fsimpl/host/BUILD index 03e3e684d..6a636514e 100644 --- a/pkg/sentry/fsimpl/host/BUILD +++ b/pkg/sentry/fsimpl/host/BUILD @@ -29,6 +29,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fdnotifier", diff --git a/pkg/sentry/fsimpl/kernfs/BUILD b/pkg/sentry/fsimpl/kernfs/BUILD index 4b577ea43..2574e9c9f 100644 --- a/pkg/sentry/fsimpl/kernfs/BUILD +++ b/pkg/sentry/fsimpl/kernfs/BUILD @@ -103,6 +103,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", @@ -134,6 +135,7 @@ go_test( deps = [ ":kernfs", "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/fsimpl/mqfs/BUILD b/pkg/sentry/fsimpl/mqfs/BUILD index 332c9b504..9125a118d 100644 --- a/pkg/sentry/fsimpl/mqfs/BUILD +++ b/pkg/sentry/fsimpl/mqfs/BUILD @@ -26,6 +26,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/refsvfs2", diff --git a/pkg/sentry/fsimpl/proc/BUILD b/pkg/sentry/fsimpl/proc/BUILD index 95cfbdc42..4e2ebe234 100644 --- a/pkg/sentry/fsimpl/proc/BUILD +++ b/pkg/sentry/fsimpl/proc/BUILD @@ -80,6 +80,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/fsimpl/sys/BUILD b/pkg/sentry/fsimpl/sys/BUILD index ab21f028e..146839f0b 100644 --- a/pkg/sentry/fsimpl/sys/BUILD +++ b/pkg/sentry/fsimpl/sys/BUILD @@ -24,6 +24,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/coverage", "//pkg/errors/linuxerr", diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index 1e7a817e8..7fab2e5a4 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -56,6 +56,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/kernel/BUILD b/pkg/sentry/kernel/BUILD index 9f8c0eeec..e313d69fa 100644 --- a/pkg/sentry/kernel/BUILD +++ b/pkg/sentry/kernel/BUILD @@ -220,6 +220,7 @@ go_library( "//pkg/abi", "//pkg/abi/linux", "//pkg/abi/linux/errno", + "//pkg/atomicbitops", "//pkg/bitmap", "//pkg/bits", "//pkg/bpf", diff --git a/pkg/sentry/kernel/shm/BUILD b/pkg/sentry/kernel/shm/BUILD index 2547957ba..17e3ea4b9 100644 --- a/pkg/sentry/kernel/shm/BUILD +++ b/pkg/sentry/kernel/shm/BUILD @@ -27,6 +27,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/hostarch", diff --git a/pkg/sentry/socket/unix/BUILD b/pkg/sentry/socket/unix/BUILD index 7b546c04d..f7d155f83 100644 --- a/pkg/sentry/socket/unix/BUILD +++ b/pkg/sentry/socket/unix/BUILD @@ -38,6 +38,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/socket/unix/transport/BUILD b/pkg/sentry/socket/unix/transport/BUILD index 1df738d24..c69e7efe4 100644 --- a/pkg/sentry/socket/unix/transport/BUILD +++ b/pkg/sentry/socket/unix/transport/BUILD @@ -57,6 +57,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fdnotifier", diff --git a/pkg/sentry/vfs/BUILD b/pkg/sentry/vfs/BUILD index 914574543..978b818cf 100644 --- a/pkg/sentry/vfs/BUILD +++ b/pkg/sentry/vfs/BUILD @@ -94,6 +94,7 @@ go_library( visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fd", diff --git a/pkg/tcpip/link/tun/BUILD b/pkg/tcpip/link/tun/BUILD index 445227d61..eca872d48 100644 --- a/pkg/tcpip/link/tun/BUILD +++ b/pkg/tcpip/link/tun/BUILD @@ -25,6 +25,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/log", diff --git a/pkg/tcpip/transport/tcp/BUILD b/pkg/tcpip/transport/tcp/BUILD index 524e744f2..7e176d31f 100644 --- a/pkg/tcpip/transport/tcp/BUILD +++ b/pkg/tcpip/transport/tcp/BUILD @@ -71,6 +71,7 @@ go_library( imports = ["gvisor.dev/gvisor/pkg/tcpip/buffer"], visibility = ["//visibility:public"], deps = [ + "//pkg/atomicbitops", "//pkg/log", "//pkg/rand", "//pkg/refsvfs2",