From 1d536f8139ab034e90261a931217440aa7994d5c Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Wed, 10 Nov 2021 17:10:18 -0800 Subject: [PATCH] Reduce host VMA fragmentation MemoryFile allocated offsets from the top down. This can generate fragmentation when allocations happen in the opposite direction. Given that applications control the order of allocations, use simple heuristics to track the last faulted address to determine whether memory file should be allocated from the top-down or bottom-up. Here is the number of PMAs used across all processes in some common workloads: Workload | Before | After | Diff ---------|---------|---------|------ rustc | 135,269 | 1,090 | 0.8% mysql | 16,466 | 4,576 | 28% nginx | 1,738 | 1,609 | 93% jenkins | 13,830 | 6,380 | 46% redis | 334 | 308 | 92% absl | 297,419 | 292,121 | 98% Here is the direction of PMA allocations: Workload | BottomUp | TopDown | ---------|-----------|---------| mysql | 13,849 | 1,990 | nginx | 783 | 628 | jenkins | 13,922 | 2,333 | redis | 88 | 109 | absl | 212,403 | 44,019 | Tests were done on Intel x64. PiperOrigin-RevId: 409014690 --- pkg/sentry/fs/tmpfs/BUILD | 1 + pkg/sentry/fs/tmpfs/inode_file.go | 3 +- pkg/sentry/fsimpl/tmpfs/regular_file.go | 2 +- pkg/sentry/kernel/kcov.go | 2 +- pkg/sentry/kernel/shm/shm.go | 2 +- pkg/sentry/kernel/task_run.go | 2 +- pkg/sentry/kernel/timekeeper_test.go | 2 +- pkg/sentry/loader/vdso.go | 4 +- pkg/sentry/mm/aio_context.go | 2 +- pkg/sentry/mm/lifecycle.go | 2 +- pkg/sentry/mm/mm.go | 31 ++++ pkg/sentry/mm/mm_test.go | 2 +- pkg/sentry/mm/pma.go | 13 +- pkg/sentry/mm/special_mappable.go | 2 +- pkg/sentry/mm/syscalls.go | 10 +- pkg/sentry/mm/vma.go | 10 +- pkg/sentry/pgalloc/pgalloc.go | 74 +++++++- pkg/sentry/pgalloc/pgalloc_test.go | 230 +++++++++++++++++++----- 18 files changed, 313 insertions(+), 81 deletions(-) diff --git a/pkg/sentry/fs/tmpfs/BUILD b/pkg/sentry/fs/tmpfs/BUILD index 511fffb43..459df854a 100644 --- a/pkg/sentry/fs/tmpfs/BUILD +++ b/pkg/sentry/fs/tmpfs/BUILD @@ -28,6 +28,7 @@ go_library( "//pkg/sentry/kernel/pipe", "//pkg/sentry/kernel/time", "//pkg/sentry/memmap", + "//pkg/sentry/pgalloc", "//pkg/sentry/socket/unix/transport", "//pkg/sentry/usage", "//pkg/sync", diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go index 1974523bf..f4bd03089 100644 --- a/pkg/sentry/fs/tmpfs/inode_file.go +++ b/pkg/sentry/fs/tmpfs/inode_file.go @@ -30,6 +30,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/kernel" ktime "gvisor.dev/gvisor/pkg/sentry/kernel/time" "gvisor.dev/gvisor/pkg/sentry/memmap" + "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/usage" "gvisor.dev/gvisor/pkg/sync" "gvisor.dev/gvisor/pkg/usermem" @@ -528,7 +529,7 @@ func (rw *fileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, error) case gap.Ok(): // Allocate memory for the write. gapMR := gap.Range().Intersect(pgMR) - fr, err := mf.Allocate(gapMR.Length(), rw.f.memUsage) + fr, err := mf.Allocate(gapMR.Length(), pgalloc.AllocOpts{Kind: rw.f.memUsage}) if err != nil { return done, err } diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 453e1aa61..3a30e043f 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -657,7 +657,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, case gap.Ok(): // Allocate memory for the write. gapMR := gap.Range().Intersect(pgMR) - fr, err := rw.file.memFile.Allocate(gapMR.Length(), rw.file.memoryUsageKind) + fr, err := rw.file.memFile.Allocate(gapMR.Length(), pgalloc.AllocOpts{Kind: rw.file.memoryUsageKind}) if err != nil { retErr = err goto exitLoop diff --git a/pkg/sentry/kernel/kcov.go b/pkg/sentry/kernel/kcov.go index e8a71bec1..125ba5b1d 100644 --- a/pkg/sentry/kernel/kcov.go +++ b/pkg/sentry/kernel/kcov.go @@ -242,7 +242,7 @@ func (kcov *Kcov) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) erro if kcov.mappable == nil { // Set up the kcov area. - fr, err := kcov.mfp.MemoryFile().Allocate(kcov.size*8, usage.Anonymous) + fr, err := kcov.mfp.MemoryFile().Allocate(kcov.size*8, pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { return err } diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go index bb9a129ab..26d88c48d 100644 --- a/pkg/sentry/kernel/shm/shm.go +++ b/pkg/sentry/kernel/shm/shm.go @@ -207,7 +207,7 @@ func (r *Registry) newShmLocked(ctx context.Context, pid int32, key ipc.Key, cre } effectiveSize := uint64(hostarch.Addr(size).MustRoundUp()) - fr, err := mfp.MemoryFile().Allocate(effectiveSize, usage.Anonymous) + fr, err := mfp.MemoryFile().Allocate(effectiveSize, pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { return nil, err } diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index 7b336a46b..be5d96ff4 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -330,7 +330,7 @@ func (app *runApp) execute(t *Task) taskRunState { } // Faults are common, log only at debug level. - t.Debugf("Unhandled user fault: addr=%x ip=%x access=%v err=%v", addr, t.Arch().IP(), at, err) + t.Debugf("Unhandled user fault: addr=%x ip=%x access=%v sig=%v err=%v", addr, t.Arch().IP(), at, sig, err) t.DebugDumpState() // Continue to signal handling. diff --git a/pkg/sentry/kernel/timekeeper_test.go b/pkg/sentry/kernel/timekeeper_test.go index b6039505a..18358561d 100644 --- a/pkg/sentry/kernel/timekeeper_test.go +++ b/pkg/sentry/kernel/timekeeper_test.go @@ -54,7 +54,7 @@ func (c *mockClocks) GetTime(id sentrytime.ClockID) (int64, error) { func stateTestClocklessTimekeeper(tb testing.TB) *Timekeeper { ctx := contexttest.Context(tb) mfp := pgalloc.MemoryFileProviderFromContext(ctx) - fr, err := mfp.MemoryFile().Allocate(hostarch.PageSize, usage.Anonymous) + fr, err := mfp.MemoryFile().Allocate(hostarch.PageSize, pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { tb.Fatalf("failed to allocate memory: %v", err) } diff --git a/pkg/sentry/loader/vdso.go b/pkg/sentry/loader/vdso.go index bcee6aef6..80b238709 100644 --- a/pkg/sentry/loader/vdso.go +++ b/pkg/sentry/loader/vdso.go @@ -195,7 +195,7 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { } mf := mfp.MemoryFile() - vdso, err := mf.Allocate(uint64(size), usage.System) + vdso, err := mf.Allocate(uint64(size), pgalloc.AllocOpts{Kind: usage.System}) if err != nil { return nil, fmt.Errorf("unable to allocate VDSO memory: %v", err) } @@ -213,7 +213,7 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { } // Finally, allocate a param page for this VDSO. - paramPage, err := mf.Allocate(hostarch.PageSize, usage.System) + paramPage, err := mf.Allocate(hostarch.PageSize, pgalloc.AllocOpts{Kind: usage.System}) if err != nil { mf.DecRef(vdso) return nil, fmt.Errorf("unable to allocate VDSO param page: %v", err) diff --git a/pkg/sentry/mm/aio_context.go b/pkg/sentry/mm/aio_context.go index d71d64580..a10587b0f 100644 --- a/pkg/sentry/mm/aio_context.go +++ b/pkg/sentry/mm/aio_context.go @@ -254,7 +254,7 @@ type aioMappable struct { var aioRingBufferSize = uint64(hostarch.Addr(linux.AIORingSize).MustRoundUp()) func newAIOMappable(mfp pgalloc.MemoryFileProvider) (*aioMappable, error) { - fr, err := mfp.MemoryFile().Allocate(aioRingBufferSize, usage.Anonymous) + fr, err := mfp.MemoryFile().Allocate(aioRingBufferSize, pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { return nil, err } diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index a79ef9223..b2aa930cb 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -93,7 +93,7 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) { dontforks := false dstvgap := mm2.vmas.FirstGap() for srcvseg := mm.vmas.FirstSegment(); srcvseg.Ok(); srcvseg = srcvseg.NextSegment() { - vma := srcvseg.Value() // makes a copy of the vma + vma := srcvseg.ValuePtr().copy() vmaAR := srcvseg.Range() if vma.dontfork { diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go index 0fca59b64..095f8efd2 100644 --- a/pkg/sentry/mm/mm.go +++ b/pkg/sentry/mm/mm.go @@ -36,6 +36,8 @@ package mm import ( + "sync/atomic" + "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/safemem" @@ -254,6 +256,9 @@ type MemoryManager struct { // vma represents a virtual memory area. // +// Note: new fields added to this struct must be added to vma.Copy and +// vmaSetFunctions.Merge. +// // +stateify savable type vma struct { // mappable is the virtual memory object mapped by this vma. If mappable is @@ -314,6 +319,13 @@ type vma struct { // If hint is non-empty, it is a description of the vma printed in // /proc/[pid]/maps. hint takes priority over id.MappedName(). hint string + + // lastFault records the last address that was paged faulted. It hints at + // which direction addresses in this vma are being accessed. + // + // This field can be read atomically, and written with mm.activeMu locked for + // writing and mm.mapping locked. + lastFault uintptr } const ( @@ -404,6 +416,25 @@ func (v *vma) loadRealPerms(b int) { } } +func (v *vma) copy() vma { + return vma{ + mappable: v.mappable, + off: v.off, + realPerms: v.realPerms, + effectivePerms: v.effectivePerms, + maxPerms: v.maxPerms, + private: v.private, + growsDown: v.growsDown, + dontfork: v.dontfork, + mlockMode: v.mlockMode, + numaPolicy: v.numaPolicy, + numaNodemask: v.numaNodemask, + id: v.id, + hint: v.hint, + lastFault: atomic.LoadUintptr(&v.lastFault), + } +} + // pma represents a platform mapping area. // // +stateify savable diff --git a/pkg/sentry/mm/mm_test.go b/pkg/sentry/mm/mm_test.go index 84cb8158d..4fced2730 100644 --- a/pkg/sentry/mm/mm_test.go +++ b/pkg/sentry/mm/mm_test.go @@ -73,7 +73,7 @@ func TestUsageASUpdates(t *testing.T) { func (mm *MemoryManager) realDataAS() uint64 { var sz uint64 for seg := mm.vmas.FirstSegment(); seg.Ok(); seg = seg.NextSegment() { - vma := seg.Value() + vma := seg.ValuePtr() if vma.isPrivateDataLocked() { sz += uint64(seg.Range().Length()) } diff --git a/pkg/sentry/mm/pma.go b/pkg/sentry/mm/pma.go index 05cdcd8ae..a6638a522 100644 --- a/pkg/sentry/mm/pma.go +++ b/pkg/sentry/mm/pma.go @@ -16,6 +16,7 @@ package mm import ( "fmt" + "sync/atomic" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" @@ -23,6 +24,7 @@ import ( "gvisor.dev/gvisor/pkg/safecopy" "gvisor.dev/gvisor/pkg/safemem" "gvisor.dev/gvisor/pkg/sentry/memmap" + "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/usage" ) @@ -204,6 +206,15 @@ func (mm *MemoryManager) getPMAsInternalLocked(ctx context.Context, vseg vmaIter } } + opts := pgalloc.AllocOpts{Kind: usage.Anonymous, Dir: pgalloc.BottomUp} + vma := vseg.ValuePtr() + if uintptr(ar.Start) < atomic.LoadUintptr(&vma.lastFault) { + // Detect cases where memory is accessed downwards and change memory file + // allocation order to increase the chances that pages are coalesced. + opts.Dir = pgalloc.TopDown + } + atomic.StoreUintptr(&vma.lastFault, uintptr(ar.Start)) + mf := mm.mfp.MemoryFile() // Limit the range we allocate to ar, aligned to privateAllocUnit. maskAR := privateAligned(ar) @@ -230,7 +241,7 @@ func (mm *MemoryManager) getPMAsInternalLocked(ctx context.Context, vseg vmaIter if vma.mappable == nil { // Private anonymous mappings get pmas by allocating. allocAR := optAR.Intersect(maskAR) - fr, err := mf.Allocate(uint64(allocAR.Length()), usage.Anonymous) + fr, err := mf.Allocate(uint64(allocAR.Length()), opts) if err != nil { return pstart, pgap, err } diff --git a/pkg/sentry/mm/special_mappable.go b/pkg/sentry/mm/special_mappable.go index 69c6e77a7..727284fb2 100644 --- a/pkg/sentry/mm/special_mappable.go +++ b/pkg/sentry/mm/special_mappable.go @@ -150,7 +150,7 @@ func NewSharedAnonMappable(length uint64, mfp pgalloc.MemoryFileProvider) (*Spec if !ok { return nil, linuxerr.EINVAL } - fr, err := mfp.MemoryFile().Allocate(uint64(alignedLen), usage.Anonymous) + fr, err := mfp.MemoryFile().Allocate(uint64(alignedLen), pgalloc.AllocOpts{Kind: usage.Anonymous}) if err != nil { return nil, err } diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index dc12ad357..7b1865567 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -518,9 +518,8 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS // // We can't use createVMALocked because it calls Mappable.AddMapping, // whereas we've already called Mappable.CopyMapping (which is - // consistent with Linux). Call vseg.Value() (rather than - // vseg.ValuePtr()) to make a copy of the vma. - vma := vseg.Value() + // consistent with Linux). + vma := vseg.ValuePtr().copy() if vma.mappable != nil { vma.off = vseg.mappableOffsetAt(oldAR.Start) } @@ -553,11 +552,8 @@ func (mm *MemoryManager) MRemap(ctx context.Context, oldAddr hostarch.Addr, oldS // 2. We can't call vma.mappable.RemoveMapping, because pmas are still at // oldAR, so calling RemoveMapping could cause us to miss an invalidation // overlapping oldAR. - // - // Call vseg.Value() (rather than vseg.ValuePtr()) to make a copy of the - // vma. vseg = mm.vmas.Isolate(vseg, oldAR) - vma := vseg.Value() + vma := vseg.ValuePtr().copy() mm.vmas.Remove(vseg) vseg = mm.vmas.Insert(mm.vmas.FindGap(newAR.Start), newAR, vma) mm.usageAS = mm.usageAS - uint64(oldAR.Length()) + uint64(newAR.Length()) diff --git a/pkg/sentry/mm/vma.go b/pkg/sentry/mm/vma.go index e34b7a2f7..e0e80a8c7 100644 --- a/pkg/sentry/mm/vma.go +++ b/pkg/sentry/mm/vma.go @@ -16,6 +16,7 @@ package mm import ( "fmt" + "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" @@ -412,15 +413,15 @@ func (mm *MemoryManager) removeVMAsLocked(ctx context.Context, ar hostarch.AddrR // canWriteMappableLocked is equivalent to Linux's VM_SHARED. // // Preconditions: mm.mappingMu must be locked. -func (vma *vma) canWriteMappableLocked() bool { - return !vma.private && vma.maxPerms.Write +func (v *vma) canWriteMappableLocked() bool { + return !v.private && v.maxPerms.Write } // isPrivateDataLocked identify the data segments - private, writable, not stack // // Preconditions: mm.mappingMu must be locked. -func (vma *vma) isPrivateDataLocked() bool { - return vma.realPerms.Write && vma.private && !vma.growsDown +func (v *vma) isPrivateDataLocked() bool { + return v.realPerms.Write && v.private && !v.growsDown } // vmaSetFunctions implements segment.Functions for vmaSet. @@ -438,6 +439,7 @@ func (vmaSetFunctions) ClearValue(vma *vma) { vma.mappable = nil vma.id = nil vma.hint = "" + atomic.StoreUintptr(&vma.lastFault, 0) } func (vmaSetFunctions) Merge(ar1 hostarch.AddrRange, vma1 vma, ar2 hostarch.AddrRange, vma2 vma) (vma, bool) { diff --git a/pkg/sentry/pgalloc/pgalloc.go b/pkg/sentry/pgalloc/pgalloc.go index 68e17d343..de22a34d2 100644 --- a/pkg/sentry/pgalloc/pgalloc.go +++ b/pkg/sentry/pgalloc/pgalloc.go @@ -41,6 +41,27 @@ import ( "gvisor.dev/gvisor/pkg/sync" ) +// Direction describes how to allocate offsets from MemoryFile. +type Direction int + +const ( + // BottomUp allocates offsets in increasing offsets. + BottomUp Direction = iota + // TopDown allocates offsets in decreasing offsets. + TopDown +) + +// String implements fmt.Stringer. +func (d Direction) String() string { + switch d { + case BottomUp: + return "up" + case TopDown: + return "down" + } + panic(fmt.Sprintf("invalid direction: %d", d)) +} + // MemoryFile is a memmap.File whose pages may be allocated to arbitrary // users. type MemoryFile struct { @@ -141,7 +162,7 @@ type MemoryFile struct { // is protected by mu. reclaimable bool - // relcaim is the collection of regions for reclaim. relcaim is protected + // reclaim is the collection of regions for reclaim. reclaim is protected // by mu. reclaim reclaimSet @@ -378,6 +399,12 @@ func (f *MemoryFile) Destroy() { f.reclaimCond.Signal() } +// AllocOpts are options used in MemoryFile.Allocate. +type AllocOpts struct { + Kind usage.MemoryKind + Dir Direction +} + // Allocate returns a range of initially-zeroed pages of the given length with // the given accounting kind and a single reference held by the caller. When // the last reference on an allocated page is released, ownership of the page @@ -385,7 +412,7 @@ func (f *MemoryFile) Destroy() { // to Allocate. // // Preconditions: length must be page-aligned and non-zero. -func (f *MemoryFile) Allocate(length uint64, kind usage.MemoryKind) (memmap.FileRange, error) { +func (f *MemoryFile) Allocate(length uint64, opts AllocOpts) (memmap.FileRange, error) { if length == 0 || length%hostarch.PageSize != 0 { panic(fmt.Sprintf("invalid allocation length: %#x", length)) } @@ -401,7 +428,7 @@ func (f *MemoryFile) Allocate(length uint64, kind usage.MemoryKind) (memmap.File } // Find a range in the underlying file. - fr, ok := findAvailableRange(&f.usage, f.fileSize, length, alignment) + fr, ok := f.findAvailableRange(length, alignment, opts.Dir) if !ok { return memmap.FileRange{}, linuxerr.ENOMEM } @@ -429,7 +456,7 @@ func (f *MemoryFile) Allocate(length uint64, kind usage.MemoryKind) (memmap.File } // Mark selected pages as in use. if !f.usage.Add(fr, usageInfo{ - kind: kind, + kind: opts.Kind, refs: 1, }) { panic(fmt.Sprintf("allocating %v: failed to insert into usage set:\n%v", fr, &f.usage)) @@ -448,7 +475,14 @@ func (f *MemoryFile) Allocate(length uint64, kind usage.MemoryKind) (memmap.File // space for mappings to be allocated downwards. // // Precondition: alignment must be a power of 2. -func findAvailableRange(usage *usageSet, fileSize int64, length, alignment uint64) (memmap.FileRange, bool) { +func (f *MemoryFile) findAvailableRange(length, alignment uint64, dir Direction) (memmap.FileRange, bool) { + if dir == BottomUp { + return findAvailableRangeBottomUp(&f.usage, length, alignment) + } + return findAvailableRangeTopDown(&f.usage, f.fileSize, length, alignment) +} + +func findAvailableRangeTopDown(usage *usageSet, fileSize int64, length, alignment uint64) (memmap.FileRange, bool) { alignmentMask := alignment - 1 // Search for space in existing gaps, starting at the current end of the @@ -510,6 +544,27 @@ func findAvailableRange(usage *usageSet, fileSize int64, length, alignment uint6 } } +func findAvailableRangeBottomUp(usage *usageSet, length, alignment uint64) (memmap.FileRange, bool) { + alignmentMask := alignment - 1 + for gap := usage.FirstGap(); gap.Ok(); gap = gap.NextLargeEnoughGap(length) { + // Align the start address and check if allocation still fits in the gap. + start := (gap.Start() + alignmentMask) &^ alignmentMask + + // File offsets are int64s. Since length must be strictly positive, end + // cannot legitimately be 0. + end := start + length + if end < start || int64(end) <= 0 { + return memmap.FileRange{}, false + } + if end <= gap.End() { + return memmap.FileRange{start, end}, true + } + } + + // NextLargeEnoughGap should have returned a gap at the end. + panic(fmt.Sprintf("NextLargeEnoughGap didn't return a gap at the end, length: %d", length)) +} + // AllocateAndFill allocates memory of the given kind and fills it by calling // r.ReadToBlocks() repeatedly until either length bytes are read or a non-nil // error is returned. It returns the memory filled by r, truncated down to the @@ -520,7 +575,7 @@ func findAvailableRange(usage *usageSet, fileSize int64, length, alignment uint6 // * length > 0. // * length must be page-aligned. func (f *MemoryFile) AllocateAndFill(length uint64, kind usage.MemoryKind, r safemem.Reader) (memmap.FileRange, error) { - fr, err := f.Allocate(length, kind) + fr, err := f.Allocate(length, AllocOpts{Kind: kind}) if err != nil { return memmap.FileRange{}, err } @@ -1144,9 +1199,10 @@ func (f *MemoryFile) findReclaimable() (memmap.FileRange, bool) { } f.reclaimCond.Wait() } - // Allocate works from the back of the file inwards, so reclaim - // preserves this order to minimize the cost of the search. - if seg := f.reclaim.LastSegment(); seg.Ok() { + // Most allocations are done upwards, with exceptions being stacks and some + // allocators that allocate top-down. Reclaim preserves this order to + // minimize the cost of the search. + if seg := f.reclaim.FirstSegment(); seg.Ok() { fr := seg.Range() f.reclaim.Remove(seg) return fr, true diff --git a/pkg/sentry/pgalloc/pgalloc_test.go b/pkg/sentry/pgalloc/pgalloc_test.go index 8d2b7eb5e..56d0285e8 100644 --- a/pkg/sentry/pgalloc/pgalloc_test.go +++ b/pkg/sentry/pgalloc/pgalloc_test.go @@ -15,6 +15,7 @@ package pgalloc import ( + "fmt" "testing" "gvisor.dev/gvisor/pkg/hostarch" @@ -28,23 +29,45 @@ const ( func TestFindUnallocatedRange(t *testing.T) { for _, test := range []struct { - desc string + name string usage *usageSegmentDataSlices fileSize int64 length uint64 alignment uint64 - start uint64 + direction Direction + want uint64 expectFail bool }{ { - desc: "Initial allocation succeeds", + name: "Initial allocation succeeds", usage: &usageSegmentDataSlices{}, length: page, alignment: page, - start: chunkSize - page, // Grows by chunkSize, allocate down. + direction: BottomUp, + want: 0, }, { - desc: "Allocation finds empty space at start of file", + name: "Initial allocation succeeds", + usage: &usageSegmentDataSlices{}, + length: page, + alignment: page, + direction: TopDown, + want: chunkSize - page, // Grows by chunkSize, allocate down. + }, + { + name: "Allocation begins at start of file", + usage: &usageSegmentDataSlices{ + Start: []uint64{page}, + End: []uint64{2 * page}, + Values: []usageInfo{{refs: 1}}, + }, + length: page, + alignment: page, + direction: BottomUp, + want: 0, + }, + { + name: "Allocation finds empty space at start of file", usage: &usageSegmentDataSlices{ Start: []uint64{page}, End: []uint64{2 * page}, @@ -53,10 +76,10 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 2 * page, length: page, alignment: page, - start: 0, + direction: TopDown, }, { - desc: "Allocation finds empty space at end of file", + name: "Allocation finds empty space at end of file", usage: &usageSegmentDataSlices{ Start: []uint64{0}, End: []uint64{page}, @@ -65,10 +88,23 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 2 * page, length: page, alignment: page, - start: page, + direction: TopDown, + want: page, }, { - desc: "In-use frames are not allocatable", + name: "In-use frames are not allocatable", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, page}, + End: []uint64{page, 2 * page}, + Values: []usageInfo{{refs: 1}, {refs: 2}}, + }, + length: page, + alignment: page, + direction: BottomUp, + want: 2 * page, + }, + { + name: "In-use frames are not allocatable", usage: &usageSegmentDataSlices{ Start: []uint64{0, page}, End: []uint64{page, 2 * page}, @@ -77,10 +113,23 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 2 * page, length: page, alignment: page, - start: 3 * page, // Double fileSize, allocate top-down. + direction: TopDown, + want: 3 * page, // Double fileSize, allocate top-down. }, { - desc: "Reclaimable frames are not allocatable", + name: "Reclaimable frames are not allocatable", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, page, 2 * page}, + End: []uint64{page, 2 * page, 3 * page}, + Values: []usageInfo{{refs: 1}, {refs: 0}, {refs: 1}}, + }, + length: page, + alignment: page, + direction: BottomUp, + want: 3 * page, + }, + { + name: "Reclaimable frames are not allocatable", usage: &usageSegmentDataSlices{ Start: []uint64{0, page, 2 * page}, End: []uint64{page, 2 * page, 3 * page}, @@ -89,10 +138,23 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 3 * page, length: page, alignment: page, - start: 5 * page, // Double fileSize, grow down. + direction: TopDown, + want: 5 * page, // Double fileSize, grow down. }, { - desc: "Gaps between in-use frames are allocatable", + name: "Gaps between in-use frames are allocatable", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, 2 * page}, + End: []uint64{page, 3 * page}, + Values: []usageInfo{{refs: 1}, {refs: 1}}, + }, + length: page, + alignment: page, + direction: BottomUp, + want: page, + }, + { + name: "Gaps between in-use frames are allocatable", usage: &usageSegmentDataSlices{ Start: []uint64{0, 2 * page}, End: []uint64{page, 3 * page}, @@ -101,10 +163,23 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 3 * page, length: page, alignment: page, - start: page, + direction: TopDown, + want: page, }, { - desc: "Inadequately-sized gaps are rejected", + name: "Inadequately-sized gaps are rejected", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, 2 * page}, + End: []uint64{page, 3 * page}, + Values: []usageInfo{{refs: 1}, {refs: 1}}, + }, + length: 2 * page, + alignment: page, + direction: BottomUp, + want: 3 * page, + }, + { + name: "Inadequately-sized gaps are rejected", usage: &usageSegmentDataSlices{ Start: []uint64{0, 2 * page}, End: []uint64{page, 3 * page}, @@ -113,10 +188,25 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 3 * page, length: 2 * page, alignment: page, - start: 4 * page, // Double fileSize, grow down. + direction: TopDown, + want: 4 * page, // Double fileSize, grow down. }, { - desc: "Alignment is honored at end of file", + name: "Alignment is honored at end of file", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, hugepage + page}, + // Hugepage-sized gap here that shouldn't be allocated from + // since it's incorrectly aligned. + End: []uint64{page, hugepage + 2*page}, + Values: []usageInfo{{refs: 1}, {refs: 1}}, + }, + length: hugepage, + alignment: hugepage, + direction: BottomUp, + want: 2 * hugepage, + }, + { + name: "Alignment is honored at end of file", usage: &usageSegmentDataSlices{ Start: []uint64{0, hugepage + page}, // Hugepage-sized gap here that shouldn't be allocated from @@ -127,10 +217,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: hugepage + 2*page, length: hugepage, alignment: hugepage, - start: 3 * hugepage, // Double fileSize until alignment is satisfied, grow down. + direction: TopDown, + want: 3 * hugepage, // Double fileSize until alignment is satisfied, grow down. }, { - desc: "Alignment is honored before end of file", + name: "Alignment is honored before end of file", usage: &usageSegmentDataSlices{ Start: []uint64{0, 2*hugepage + page}, // Page will need to be shifted down from top. @@ -140,18 +231,29 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 2*hugepage + 2*page, length: hugepage, alignment: hugepage, - start: hugepage, + direction: TopDown, + want: hugepage, }, { - desc: "Allocation doubles file size more than once if necessary", + name: "Allocation doubles file size more than once if necessary", usage: &usageSegmentDataSlices{}, fileSize: page, length: 4 * page, alignment: page, - start: 0, + direction: BottomUp, + want: 0, }, { - desc: "Allocations are compact if possible", + name: "Allocation doubles file size more than once if necessary", + usage: &usageSegmentDataSlices{}, + fileSize: page, + length: 4 * page, + alignment: page, + direction: TopDown, + want: 0, + }, + { + name: "Allocations are compact if possible", usage: &usageSegmentDataSlices{ Start: []uint64{page, 3 * page}, End: []uint64{2 * page, 4 * page}, @@ -160,10 +262,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 4 * page, length: page, alignment: page, - start: 2 * page, + direction: TopDown, + want: 2 * page, }, { - desc: "Top-down allocation within one gap", + name: "Top-down allocation within one gap", usage: &usageSegmentDataSlices{ Start: []uint64{page, 4 * page, 7 * page}, End: []uint64{2 * page, 5 * page, 8 * page}, @@ -172,10 +275,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 8 * page, length: page, alignment: page, - start: 6 * page, + direction: TopDown, + want: 6 * page, }, { - desc: "Top-down allocation between multiple gaps", + name: "Top-down allocation between multiple gaps", usage: &usageSegmentDataSlices{ Start: []uint64{page, 3 * page, 5 * page}, End: []uint64{2 * page, 4 * page, 6 * page}, @@ -184,10 +288,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 6 * page, length: page, alignment: page, - start: 4 * page, + direction: TopDown, + want: 4 * page, }, { - desc: "Top-down allocation with large top gap", + name: "Top-down allocation with large top gap", usage: &usageSegmentDataSlices{ Start: []uint64{page, 3 * page}, End: []uint64{2 * page, 4 * page}, @@ -196,10 +301,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: 8 * page, length: page, alignment: page, - start: 7 * page, + direction: TopDown, + want: 7 * page, }, { - desc: "Gaps found with possible overflow", + name: "Gaps found with possible overflow", usage: &usageSegmentDataSlices{ Start: []uint64{page, topPage - page}, End: []uint64{2 * page, topPage}, @@ -208,10 +314,11 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: topPage, length: page, alignment: page, - start: topPage - 2*page, + direction: TopDown, + want: topPage - 2*page, }, { - desc: "Overflow detected", + name: "Overflow detected", usage: &usageSegmentDataSlices{ Start: []uint64{page}, End: []uint64{topPage}, @@ -220,26 +327,53 @@ func TestFindUnallocatedRange(t *testing.T) { fileSize: topPage, length: 2 * page, alignment: page, + direction: BottomUp, expectFail: true, }, + { + name: "Overflow detected", + usage: &usageSegmentDataSlices{ + Start: []uint64{page}, + End: []uint64{topPage}, + Values: []usageInfo{{refs: 1}}, + }, + fileSize: topPage, + length: 2 * page, + alignment: page, + direction: TopDown, + expectFail: true, + }, + { + name: "start may be in the middle of segment", + usage: &usageSegmentDataSlices{ + Start: []uint64{0, 3 * page}, + End: []uint64{2 * page, 4 * page}, + Values: []usageInfo{{refs: 1}, {refs: 2}}, + }, + length: page, + alignment: page, + direction: BottomUp, + want: 2 * page, + }, } { - t.Run(test.desc, func(t *testing.T) { - var usage usageSet - if err := usage.ImportSortedSlices(test.usage); err != nil { + name := fmt.Sprintf("%s (%v)", test.name, test.direction) + t.Run(name, func(t *testing.T) { + f := MemoryFile{fileSize: test.fileSize} + if err := f.usage.ImportSortedSlices(test.usage); err != nil { t.Fatalf("Failed to initialize usage from %v: %v", test.usage, err) } - fr, ok := findAvailableRange(&usage, test.fileSize, test.length, test.alignment) - if !test.expectFail && !ok { - t.Fatalf("findAvailableRange(%v, %x, %x, %x): got %x, false wanted %x, true", test.usage, test.fileSize, test.length, test.alignment, fr.Start, test.start) - } - if test.expectFail && ok { - t.Fatalf("findAvailableRange(%v, %x, %x, %x): got %x, true wanted %x, false", test.usage, test.fileSize, test.length, test.alignment, fr.Start, test.start) - } - if ok && fr.Start != test.start { - t.Errorf("findAvailableRange(%v, %x, %x, %x): got start=%x, wanted %x", test.usage, test.fileSize, test.length, test.alignment, fr.Start, test.start) - } - if ok && fr.End != test.start+test.length { - t.Errorf("findAvailableRange(%v, %x, %x, %x): got end=%x, wanted %x", test.usage, test.fileSize, test.length, test.alignment, fr.End, test.start+test.length) + if fr, ok := f.findAvailableRange(test.length, test.alignment, test.direction); ok { + if test.expectFail { + t.Fatalf("findAvailableRange(%v, %x, %x, %x, %v): got: %x, want: fail", test.usage, test.fileSize, test.length, test.alignment, test.direction, fr.Start) + } + if fr.Start != test.want { + t.Errorf("findAvailableRange(%v, %x, %x, %x, %v): got: start=%x, want: %x", test.usage, test.fileSize, test.length, test.alignment, test.direction, fr.Start, test.want) + } + if fr.End != test.want+test.length { + t.Errorf("findAvailableRange(%v, %x, %x, %x, %v): got: end=%x, want: %x", test.usage, test.fileSize, test.length, test.alignment, test.direction, fr.End, test.want+test.length) + } + } else if !test.expectFail { + t.Fatalf("findAvailableRange(%v, %x, %x, %x, %v): failed, want: %x", test.usage, test.fileSize, test.length, test.alignment, test.direction, test.want) } }) }