From d30c39364daa7c6b5cca3af7fd9b5935409a6f30 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Wed, 6 Sep 2023 10:51:43 -0700 Subject: [PATCH] Split the ring implementation into its own package. The ring data structure's requires it's own implementation of Entry. With the new version you can more easily be part of multiple rings at once. Also, a user who wants to use ilist probably doesn't need ring as well, so it makes sense to separate the two. PiperOrigin-RevId: 563151782 --- pkg/ilist/BUILD | 47 +++++++++++++++++ pkg/ilist/list.go | 45 ---------------- pkg/ilist/list_test.go | 57 -------------------- pkg/ilist/ring.go | 84 ++++++++++++++++++++++++++++++ pkg/ilist/ring_test.go | 97 +++++++++++++++++++++++++++++++++++ pkg/sentry/vfs/BUILD | 14 +++-- pkg/sentry/vfs/mount.go | 8 +-- pkg/sentry/vfs/propagation.go | 8 +-- 8 files changed, 240 insertions(+), 120 deletions(-) create mode 100644 pkg/ilist/ring.go create mode 100644 pkg/ilist/ring_test.go diff --git a/pkg/ilist/BUILD b/pkg/ilist/BUILD index 3d7db3d8b..4c9b6bb56 100644 --- a/pkg/ilist/BUILD +++ b/pkg/ilist/BUILD @@ -57,3 +57,50 @@ go_template( ], visibility = ["//visibility:public"], ) + +go_template( + name = "generic_ring", + srcs = [ + "ring.go", + ], + opt_types = [ + "Container", + ], + visibility = ["//visibility:public"], +) + +go_template_instance( + name = "test_ring", + out = "test_ring.go", + package = "ring", + prefix = "test", + template = "//pkg/ilist:generic_ring", + types = { + "Container": "*testContainer", + }, +) + +go_library( + name = "ring", + srcs = [ + "interface_ring.go", + ], + visibility = ["//visibility:public"], +) + +go_template_instance( + name = "interface_ring", + out = "interface_ring.go", + package = "ring", + template = ":generic_ring", + types = {}, +) + +go_test( + name = "ring_test", + srcs = [ + "ring_test.go", + "test_ring.go", + ], + library = ":ring", +) diff --git a/pkg/ilist/list.go b/pkg/ilist/list.go index 52d4df8d0..0d6e14481 100644 --- a/pkg/ilist/list.go +++ b/pkg/ilist/list.go @@ -271,48 +271,3 @@ func (e *Entry) SetNext(elem Element) { func (e *Entry) SetPrev(elem Element) { e.prev = elem } - -// RingInit instantiates an Element to be an item in a ring (circularly-linked -// list). -// -//go:nosplit -func RingInit(e Element) { - linker := ElementMapper{}.linkerFor(e) - linker.SetNext(e) - linker.SetPrev(e) -} - -// RingAdd adds new to old's ring. -// -//go:nosplit -func RingAdd(old Element, new Element) { - oldLinker := ElementMapper{}.linkerFor(old) - newLinker := ElementMapper{}.linkerFor(new) - next := oldLinker.Next() - prev := old - - next.SetPrev(new) - newLinker.SetNext(next) - newLinker.SetPrev(prev) - oldLinker.SetNext(new) -} - -// RingRemove removes e from its ring. -// -//go:nosplit -func RingRemove(e Element) { - eLinker := ElementMapper{}.linkerFor(e) - next := eLinker.Next() - prev := eLinker.Prev() - next.SetPrev(prev) - prev.SetNext(next) - RingInit(e) -} - -// RingEmpty returns true if there are no other elements in the list. -// -//go:nosplit -func RingEmpty(e Element) bool { - linker := ElementMapper{}.linkerFor(e) - return linker.Next() == e -} diff --git a/pkg/ilist/list_test.go b/pkg/ilist/list_test.go index c8521856b..3f9abfb56 100644 --- a/pkg/ilist/list_test.go +++ b/pkg/ilist/list_test.go @@ -183,63 +183,6 @@ func TestReset(t *testing.T) { } } -func TestRingAdd(t *testing.T) { - e1 := &testEntry{value: 1} - e2 := &testEntry{value: 2} - e3 := &testEntry{value: 3} - - RingInit(e1) - RingAdd(e1, e2) - RingAdd(e1, e3) - - sum := 0 - for e := e1.Next(); e != e1; e = e.Next() { - sum += e.(*testEntry).value - } - if sum != 5 { - t.Errorf("wrong sum: want 5, got %d", sum) - } -} - -func TestRingRemove(t *testing.T) { - e1 := &testEntry{value: 1} - e2 := &testEntry{value: 2} - e3 := &testEntry{value: 3} - - RingInit(e1) - RingAdd(e1, e2) - RingAdd(e2, e3) - RingRemove(e2) - - sum := 0 - for e := e1.Next(); e != e1; e = e.Next() { - sum += e.(*testEntry).value - } - if sum != 3 { - t.Errorf("wrong sum: want 3, got %d", sum) - } -} - -func TestRingEmpty(t *testing.T) { - e1 := &testEntry{value: 1} - e2 := &testEntry{value: 2} - e3 := &testEntry{value: 3} - - RingInit(e1) - RingAdd(e1, e2) - RingAdd(e2, e3) - RingRemove(e3) - RingRemove(e2) - - sum := 0 - for e := e1.Next(); e != e1; e = e.Next() { - sum += e.(*testEntry).value - } - if sum != 0 { - t.Errorf("wrong sum: want 0, got %d", sum) - } -} - func BenchmarkIterateForward(b *testing.B) { var l List for i := 0; i < 1000000; i++ { diff --git a/pkg/ilist/ring.go b/pkg/ilist/ring.go new file mode 100644 index 000000000..daa4f042b --- /dev/null +++ b/pkg/ilist/ring.go @@ -0,0 +1,84 @@ +// Copyright 2023 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 ring is an implementation of an intrusive circular linked list. +package ring + +// Container is the type that holds the list entries. +type Container any + +// Entry is an element in the circular linked list. +// +// +stateify savable +type Entry struct { + next *Entry + prev *Entry + container Container +} + +// Init instantiates an Element to be an item in a ring (circularly-linked +// list). +// +//go:nosplit +func (e *Entry) Init(container Container) { + e.next = e + e.prev = e + e.container = container +} + +// Add adds new to old's ring. +// +//go:nosplit +func (e *Entry) Add(new *Entry) { + next := e.next + prev := e + + next.prev = new + new.next = next + new.prev = prev + e.next = new +} + +// Remove removes e from its ring and reinitializes it. +// +//go:nosplit +func (e *Entry) Remove() { + next := e.next + prev := e.prev + + next.prev = prev + prev.next = next + e.Init(e.container) +} + +// Empty returns true if there are no other elements in the ring. +// +//go:nosplit +func (e *Entry) Empty() bool { + return e.next == e +} + +// Next returns the next containing object pointed to by the list. +// +//go:nosplit +func (e *Entry) Next() Container { + return e.next.container +} + +// Prev returns the previous containing object pointed to by the list. +// +//go:nosplit +func (e *Entry) Prev() Container { + return e.prev.container +} diff --git a/pkg/ilist/ring_test.go b/pkg/ilist/ring_test.go new file mode 100644 index 000000000..2bdd81cb4 --- /dev/null +++ b/pkg/ilist/ring_test.go @@ -0,0 +1,97 @@ +// Copyright 2023 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 ring + +import ( + "testing" +) + +type testContainer struct { + value int + entry testEntry +} + +func newContainer(value int) *testContainer { + c := &testContainer{value: value} + c.entry.Init(c) + return c +} + +func TestAdd(t *testing.T) { + e1 := newContainer(1) + e2 := newContainer(2) + e3 := newContainer(3) + + e1.entry.Add(&e2.entry) + e1.entry.Add(&e3.entry) + + sum := 0 + want := 6 + e := e1 + for { + sum += e.value + e = e.entry.Next() + if e == e1 { + break + } + } + if sum != want { + t.Errorf("wrong sum: want %d, got %d", want, sum) + } +} + +func TestRemove(t *testing.T) { + e1 := newContainer(1) + e2 := newContainer(2) + e3 := newContainer(3) + + e1.entry.Add(&e2.entry) + e2.entry.Add(&e3.entry) + e2.entry.Remove() + + sum := 0 + want := 4 + e := e1 + for { + sum += e.value + e = e.entry.Next() + if e == e1 { + break + } + } + if sum != want { + t.Errorf("wrong sum: want %d, got %d", want, sum) + } +} + +func TestEmpty(t *testing.T) { + head := newContainer(1) + e2 := newContainer(2) + e3 := newContainer(3) + + head.entry.Add(&e2.entry) + e2.entry.Add(&e3.entry) + e3.entry.Remove() + e2.entry.Remove() + + sum := 0 + want := 0 + for e := head.entry.Next(); e != head; e = e.entry.Next() { + sum += e.value + } + if sum != want { + t.Errorf("wrong sum: want %d, got %d", want, sum) + } +} diff --git a/pkg/sentry/vfs/BUILD b/pkg/sentry/vfs/BUILD index 3ebbb541d..0b0277ffe 100644 --- a/pkg/sentry/vfs/BUILD +++ b/pkg/sentry/vfs/BUILD @@ -54,15 +54,13 @@ go_template_instance( ) go_template_instance( - name = "shared_list", - out = "shared_list.go", + name = "mount_ring", + out = "mount_ring.go", package = "vfs", - prefix = "shared", - template = "//pkg/ilist:generic_list", + prefix = "mount", + template = "//pkg/ilist:generic_ring", types = { - "Element": "*Mount", - "Linker": "*sharedEntry", - "ElementMapper": "sharedMapper", + "Container": "*Mount", }, ) @@ -144,6 +142,7 @@ go_library( "lock.go", "mount.go", "mount_namespace_refs.go", + "mount_ring.go", "mount_unsafe.go", "namespace.go", "opath.go", @@ -153,7 +152,6 @@ go_library( "propagation.go", "resolving_path.go", "save_restore.go", - "shared_list.go", "vfs.go", "virtual_filesystem_mutex.go", ], diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index d38119046..02cf2c540 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -92,7 +92,7 @@ type Mount struct { // sharedEntry represents an entry in a circular list (ring) of mounts in a // shared peer group. - sharedEntry + sharedEntry mountEntry // groupID is the ID for this mount's shared peer group. If the mount is not // in a peer group, this is 0. @@ -110,10 +110,6 @@ type Mount struct { writers atomicbitops.Int64 } -type sharedMapper struct{} - -func (sharedMapper) linkerFor(mnt *Mount) *sharedEntry { return &mnt.sharedEntry } - func newMount(vfs *VirtualFilesystem, fs *Filesystem, root *Dentry, mntns *MountNamespace, opts *MountOptions) *Mount { mnt := &Mount{ ID: vfs.lastMountID.Add(1), @@ -128,7 +124,7 @@ func newMount(vfs *VirtualFilesystem, fs *Filesystem, root *Dentry, mntns *Mount if opts.ReadOnly { mnt.setReadOnlyLocked(true) } - sharedRingInit(mnt) + mnt.sharedEntry.Init(mnt) refs.Register(mnt) return mnt } diff --git a/pkg/sentry/vfs/propagation.go b/pkg/sentry/vfs/propagation.go index d9f4e3531..1d34d9740 100644 --- a/pkg/sentry/vfs/propagation.go +++ b/pkg/sentry/vfs/propagation.go @@ -62,15 +62,15 @@ func (vfs *VirtualFilesystem) setPropagation(mnt *Mount, pflag uint32) error { return err } mnt.groupID = id - sharedRingInit(mnt) + mnt.sharedEntry.Init(mnt) mnt.isShared = true } case linux.MS_PRIVATE: if mnt.isShared { - if sharedRingEmpty(mnt) { + if mnt.sharedEntry.Empty() { vfs.freeGroupID(mnt.groupID) } - sharedRingRemove(mnt) + mnt.sharedEntry.Remove() mnt.groupID = 0 mnt.isShared = false } @@ -85,7 +85,7 @@ func (vfs *VirtualFilesystem) setPropagation(mnt *Mount, pflag uint32) error { // // +checklocks:vfs.mountMu func (vfs *VirtualFilesystem) addPeer(mnt *Mount, new *Mount) { - sharedRingAdd(mnt, new) + mnt.sharedEntry.Add(&new.sharedEntry) new.isShared = true new.groupID = mnt.groupID }