diff options
| -rw-r--r-- | pkg/refs/refcounter.go | 7 | ||||
| -rw-r--r-- | pkg/refs_vfs2/BUILD | 28 | ||||
| -rw-r--r-- | pkg/refs_vfs2/refs.go | 36 | ||||
| -rw-r--r-- | pkg/refs_vfs2/refs_template.go | 133 | ||||
| -rw-r--r-- | pkg/sentry/fsimpl/tmpfs/BUILD | 13 | ||||
| -rw-r--r-- | pkg/sentry/fsimpl/tmpfs/tmpfs.go | 27 |
6 files changed, 21 insertions, 223 deletions
diff --git a/pkg/refs/refcounter.go b/pkg/refs/refcounter.go index 3f39edb66..61790221b 100644 --- a/pkg/refs/refcounter.go +++ b/pkg/refs/refcounter.go @@ -215,8 +215,6 @@ type AtomicRefCount struct { // LeakMode configures the leak checker. type LeakMode uint32 -// TODO(gvisor.dev/issue/1624): Simplify down to two modes once vfs1 ref -// counting is gone. const ( // UninitializedLeakChecking indicates that the leak checker has not yet been initialized. UninitializedLeakChecking LeakMode = iota @@ -246,11 +244,6 @@ func SetLeakMode(mode LeakMode) { atomic.StoreUint32(&leakMode, uint32(mode)) } -// GetLeakMode returns the current leak mode. -func GetLeakMode() LeakMode { - return LeakMode(atomic.LoadUint32(&leakMode)) -} - const maxStackFrames = 40 type fileLine struct { diff --git a/pkg/refs_vfs2/BUILD b/pkg/refs_vfs2/BUILD deleted file mode 100644 index 7f180c7bd..000000000 --- a/pkg/refs_vfs2/BUILD +++ /dev/null @@ -1,28 +0,0 @@ -load("//tools:defs.bzl", "go_library") -load("//tools/go_generics:defs.bzl", "go_template") - -package(licenses = ["notice"]) - -go_template( - name = "refs_template", - srcs = [ - "refs_template.go", - ], - types = [ - "T", - ], - visibility = ["//pkg/sentry:internal"], - deps = [ - "//pkg/log", - "//pkg/refs", - ], -) - -go_library( - name = "refs", - srcs = [ - "refs.go", - ], - visibility = ["//pkg/sentry:internal"], - deps = ["//pkg/context"], -) diff --git a/pkg/refs_vfs2/refs.go b/pkg/refs_vfs2/refs.go deleted file mode 100644 index ee01b17b0..000000000 --- a/pkg/refs_vfs2/refs.go +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2020 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 refs defines an interface for a reference-counted object. -package refs - -import ( - "gvisor.dev/gvisor/pkg/context" -) - -// RefCounter is the interface to be implemented by objects that are reference -// counted. -type RefCounter interface { - // IncRef increments the reference counter on the object. - IncRef() - - // DecRef decrements the object's reference count. Users of refs_template.Refs - // may specify a destructor to be called once the reference count reaches zero. - DecRef(ctx context.Context) - - // TryIncRef attempts to increment the reference count, but may fail if all - // references have already been dropped, in which case it returns false. If - // true is returned, then a valid reference is now held on the object. - TryIncRef() bool -} diff --git a/pkg/refs_vfs2/refs_template.go b/pkg/refs_vfs2/refs_template.go deleted file mode 100644 index 3e5b458c7..000000000 --- a/pkg/refs_vfs2/refs_template.go +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2020 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 refs_template defines a template that can be used by reference counted -// objects. -package refs_template - -import ( - "runtime" - "sync/atomic" - - "gvisor.dev/gvisor/pkg/log" - refs_vfs1 "gvisor.dev/gvisor/pkg/refs" -) - -// T is the type of the reference counted object. It is only used to customize -// debug output when leak checking. -type T interface{} - -// ownerType is used to customize logging. Note that we use a pointer to T so -// that we do not copy the entire object when passed as a format parameter. -var ownerType *T - -// Refs implements refs.RefCounter. It keeps a reference count using atomic -// operations and calls the destructor when the count reaches zero. -// -// Note that the number of references is actually refCount + 1 so that a default -// zero-value Refs object contains one reference. -// -// +stateify savable -type Refs struct { - // refCount is composed of two fields: - // - // [32-bit speculative references]:[32-bit real references] - // - // 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 -} - -func (r *Refs) finalize() { - var note string - switch refs_vfs1.GetLeakMode() { - case refs_vfs1.NoLeakChecking: - return - case refs_vfs1.UninitializedLeakChecking: - note = "(Leak checker uninitialized): " - } - if n := r.ReadRefs(); n != 0 { - log.Warningf("%sAtomicRefCount %p owned by %T garbage collected with ref count of %d (want 0)", note, r, ownerType, n) - } -} - -// EnableLeakCheck checks for reference leaks when Refs gets garbage collected. -func (r *Refs) EnableLeakCheck() { - if refs_vfs1.GetLeakMode() != refs_vfs1.NoLeakChecking { - runtime.SetFinalizer(r, (*Refs).finalize) - } -} - -// 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 { - // Account for the internal -1 offset on refcounts. - return atomic.LoadInt64(&r.refCount) + 1 -} - -// IncRef implements refs.RefCounter.IncRef. -// -//go:nosplit -func (r *Refs) IncRef() { - if v := atomic.AddInt64(&r.refCount, 1); v <= 0 { - panic("Incrementing non-positive ref count") - } -} - -// TryIncRef implements refs.RefCounter.TryIncRef. -// -// To do this safely without a loop, a speculative reference is first acquired -// on the object. This allows multiple concurrent TryIncRef calls to distinguish -// other TryIncRef calls from genuine references held. -// -//go:nosplit -func (r *Refs) TryIncRef() bool { - const speculativeRef = 1 << 32 - v := atomic.AddInt64(&r.refCount, speculativeRef) - if int32(v) < 0 { - // This object has already been freed. - atomic.AddInt64(&r.refCount, -speculativeRef) - return false - } - - // Turn into a real reference. - atomic.AddInt64(&r.refCount, -speculativeRef+1) - return true -} - -// DecRef implements refs.RefCounter.DecRef. -// -// Note that speculative references are counted here. Since they were added -// prior to real references reaching zero, they will successfully convert to -// real references. In other words, we see speculative references only in the -// following case: -// -// A: TryIncRef [speculative increase => sees non-negative references] -// B: DecRef [real decrease] -// A: TryIncRef [transform speculative to real] -// -//go:nosplit -func (r *Refs) DecRef(destroy func()) { - switch v := atomic.AddInt64(&r.refCount, -1); { - case v < -1: - panic("Decrementing non-positive ref count") - - case v == -1: - // Call the destructor. - if destroy != nil { - destroy() - } - } -} diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index 5cd428d64..e73732a6b 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -26,17 +26,6 @@ go_template_instance( }, ) -go_template_instance( - name = "inode_refs", - out = "inode_refs.go", - package = "tmpfs", - prefix = "inode", - template = "//pkg/refs_vfs2:refs_template", - types = { - "T": "inode", - }, -) - go_library( name = "tmpfs", srcs = [ @@ -45,7 +34,6 @@ go_library( "directory.go", "filesystem.go", "fstree.go", - "inode_refs.go", "named_pipe.go", "regular_file.go", "socket_file.go", @@ -59,7 +47,6 @@ go_library( "//pkg/context", "//pkg/fspath", "//pkg/log", - "//pkg/refs", "//pkg/safemem", "//pkg/sentry/arch", "//pkg/sentry/fs", diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 5640380dc..68e615e8b 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -285,10 +285,13 @@ type inode struct { // fs is the owning filesystem. fs is immutable. fs *filesystem + // refs is a reference count. refs is accessed using atomic memory + // operations. + // // A reference is held on all inodes as long as they are reachable in the // filesystem tree, i.e. nlink is nonzero. This reference is dropped when // nlink reaches 0. - refs inodeRefs + refs int64 // xattrs implements extended attributes. // @@ -324,6 +327,7 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth panic("file type is required in FileMode") } i.fs = fs + i.refs = 1 i.mode = uint32(mode) i.uid = uint32(kuid) i.gid = uint32(kgid) @@ -335,7 +339,6 @@ func (i *inode) init(impl interface{}, fs *filesystem, kuid auth.KUID, kgid auth i.mtime = now // i.nlink initialized by caller i.impl = impl - i.refs.EnableLeakCheck() } // incLinksLocked increments i's link count. @@ -366,15 +369,25 @@ func (i *inode) decLinksLocked(ctx context.Context) { } func (i *inode) incRef() { - i.refs.IncRef() + if atomic.AddInt64(&i.refs, 1) <= 1 { + panic("tmpfs.inode.incRef() called without holding a reference") + } } func (i *inode) tryIncRef() bool { - return i.refs.TryIncRef() + for { + refs := atomic.LoadInt64(&i.refs) + if refs == 0 { + return false + } + if atomic.CompareAndSwapInt64(&i.refs, refs, refs+1) { + return true + } + } } func (i *inode) decRef(ctx context.Context) { - i.refs.DecRef(func() { + if refs := atomic.AddInt64(&i.refs, -1); refs == 0 { i.watches.HandleDeletion(ctx) if regFile, ok := i.impl.(*regularFile); ok { // Release memory used by regFile to store data. Since regFile is @@ -382,7 +395,9 @@ func (i *inode) decRef(ctx context.Context) { // metadata. regFile.data.DropAll(regFile.memFile) } - }) + } else if refs < 0 { + panic("tmpfs.inode.decRef() called without holding a reference") + } } func (i *inode) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { |