Skip to content

Commit ffc0480

Browse files
gvisor-botgvisor-bot
committed
Merge pull request #16557 from EtiennePerot:restore-bimodal
PiperOrigin-RevId: 1008644382
2 parents ec8390e + c63a70c commit ffc0480

6 files changed

Lines changed: 164 additions & 25 deletions

File tree

‎pkg/sentry/mm/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ go_test(
197197
"//pkg/sentry/memmap",
198198
"//pkg/sentry/pgalloc",
199199
"//pkg/sentry/platform",
200+
"//pkg/sentry/state/stateio",
201+
"//pkg/sentry/usage",
200202
"//pkg/usermem",
201203
"@org_golang_x_sys//unix:go_default_library",
202204
],

‎pkg/sentry/mm/address_space.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ func (mm *MemoryManager) UserModifiedGS(ctx context.Context) error {
4646
return nil
4747
}
4848

49+
// asyncLoadMapUnit bounds the ranges that are mapped at once while async page
50+
// loading is in progress, in order to avoid calling
51+
// platform.AddressSpace.MapFile() => mf.DataFD() or mf.MapInternal() with
52+
// unnecessarily large ranges, resulting in unnecessarily long waits.
53+
const asyncLoadMapUnit = 32 << 20
54+
4955
// mapASLocked maps addresses in ar into mm.as.
5056
//
5157
// Preconditions:
@@ -82,11 +88,7 @@ func (mm *MemoryManager) mapASLocked(ctx context.Context, pseg pmaIterator, ar h
8288
// Limit the range we map to ar, aligned to mapUnit.
8389
setMapUnit(mapUnit)
8490
} else if mf, ok := pseg.ValuePtr().file.(*pgalloc.MemoryFile); ok && mf.IsAsyncLoading() {
85-
// Impose an arbitrary mapUnit in order to avoid calling
86-
// platform.AddressSpace.MapFile() => mf.DataFD() or mf.MapInternal()
87-
// with unnecessarily large ranges, resulting in unnecessarily long
88-
// waits.
89-
setMapUnit(32 << 20)
91+
setMapUnit(asyncLoadMapUnit)
9092
}
9193
if checkInvariants {
9294
if !mapAR.IsSupersetOf(ar) {

‎pkg/sentry/mm/io.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -786,16 +786,16 @@ func (mm *MemoryManager) getIOMappingsLocked(pseg pmaIterator, ar hostarch.AddrR
786786
}
787787

788788
if ar.End <= pseg.End() {
789-
// Since only one pma is involved, we can use pma.internalMappings
789+
// Since only one pma is involved, we can return its internal mappings
790790
// directly, avoiding a slice allocation.
791-
if err := pseg.getInternalMappingsLocked(); err != nil {
791+
ims, err := pseg.getInternalMappingsLocked(ar)
792+
if err != nil {
792793
if _, ok := err.(memmap.BufferedIOFallbackErr); ok {
793794
goto slowPath
794795
}
795796
return safemem.BlockSeq{}, nil, err
796797
}
797-
offset := uint64(ar.Start - pseg.Start())
798-
return pseg.ValuePtr().internalMappings.DropFirst64(offset).TakeFirst64(uint64(ar.Length())), nil, nil
798+
return ims, nil, nil
799799
}
800800

801801
slowPath:
@@ -872,10 +872,9 @@ func (mm *MemoryManager) getVecIOMappingsLocked(ars hostarch.AddrRangeSeq, at ho
872872
func (mm *MemoryManager) getIOMappingsTrackedLocked(pseg pmaIterator, ar hostarch.AddrRange, at hostarch.AccessType, ims []safemem.Block, t *ioBufTracker, unbufBytes uint64) ([]safemem.Block, *ioBufTracker, uint64, error) {
873873
for {
874874
pmaAR := ar.Intersect(pseg.Range())
875-
if err := pseg.getInternalMappingsLocked(); err == nil {
876-
// Iterate the subset of the PMA's cached internal mappings that
877-
// correspond to pmaAR, and append them to ims.
878-
for pims := pseg.ValuePtr().internalMappings.DropFirst64(uint64(pmaAR.Start - pseg.Start())).TakeFirst64(uint64(pmaAR.Length())); !pims.IsEmpty(); pims = pims.Tail() {
875+
if pims, err := pseg.getInternalMappingsLocked(pmaAR); err == nil {
876+
// Append the internal mappings that correspond to pmaAR to ims.
877+
for ; !pims.IsEmpty(); pims = pims.Tail() {
879878
ims = append(ims, pims.Head())
880879
}
881880
unbufBytes += uint64(pmaAR.Length())

‎pkg/sentry/mm/lifecycle.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -382,11 +382,12 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) {
382382
// +checklocks:mm.activeMu
383383
// +checklocks:mm2.activeMu
384384
func (mm *MemoryManager) forkCopyPMALocked(mm2 *MemoryManager, srcpseg pmaIterator, dstpgap pmaGapIterator, memCgID uint32) (pmaGapIterator, error) {
385-
if err := srcpseg.getInternalMappingsLocked(); err != nil {
385+
copyAR := srcpseg.Range()
386+
ims, err := srcpseg.getInternalMappingsLocked(copyAR)
387+
if err != nil {
386388
return dstpgap, err
387389
}
388-
copyAR := srcpseg.Range()
389-
reader := safemem.BlockSeqReader{Blocks: mm.internalMappingsLocked(srcpseg, copyAR)}
390+
reader := safemem.BlockSeqReader{Blocks: ims}
390391
huge := mm.mf.HugepagesEnabled() && copyAR.IsHugePageAligned()
391392
fr, err := mm.mf.Allocate(uint64(copyAR.Length()), pgalloc.AllocOpts{
392393
Kind: usage.Anonymous,

‎pkg/sentry/mm/mm_test.go‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ package mm
1616

1717
import (
1818
"bytes"
19+
"io"
1920
"os"
2021
"testing"
22+
"time"
2123

2224
"golang.org/x/sys/unix"
2325
"gvisor.dev/gvisor/pkg/context"
@@ -32,6 +34,8 @@ import (
3234
"gvisor.dev/gvisor/pkg/sentry/memmap"
3335
"gvisor.dev/gvisor/pkg/sentry/pgalloc"
3436
"gvisor.dev/gvisor/pkg/sentry/platform"
37+
"gvisor.dev/gvisor/pkg/sentry/state/stateio"
38+
"gvisor.dev/gvisor/pkg/sentry/usage"
3539
"gvisor.dev/gvisor/pkg/usermem"
3640
)
3741

@@ -1080,3 +1084,124 @@ func TestSealedMapping(t *testing.T) {
10801084
defer mm2.DecUsers(ctx)
10811085
wantError(t, linuxerr.EPERM, "child MUnmap(sealed)", mm2.MUnmap(ctx, sealedAddr, pageSize))
10821086
}
1087+
1088+
// gatedReaderAt blocks reads past offset from until `gate` is closed.
1089+
type gatedReaderAt struct {
1090+
io.ReaderAt
1091+
from int64
1092+
gate chan struct{}
1093+
}
1094+
1095+
// ReadAt implements `io.ReaderAt.ReadAt`.
1096+
func (g *gatedReaderAt) ReadAt(p []byte, off int64) (int, error) {
1097+
if off+int64(len(p)) > g.from {
1098+
<-g.gate
1099+
}
1100+
return g.ReaderAt.ReadAt(p, off)
1101+
}
1102+
1103+
func newTestMemoryFile(t *testing.T) *pgalloc.MemoryFile {
1104+
memfd, err := memutil.CreateMemFD("mm-test-memory", 0)
1105+
if err != nil {
1106+
t.Fatalf("CreateMemFD: got err %v, want nil", err)
1107+
}
1108+
memfile := os.NewFile(uintptr(memfd), "mm-test-memory")
1109+
mf, err := pgalloc.NewMemoryFile(memfile, pgalloc.MemoryFileOpts{
1110+
DisableMemoryAccounting: true,
1111+
})
1112+
if err != nil {
1113+
memfile.Close()
1114+
t.Fatalf("NewMemoryFile: got err %v, want nil", err)
1115+
}
1116+
t.Cleanup(mf.Destroy)
1117+
return mf
1118+
}
1119+
1120+
// TestIODuringAsyncPageLoad tests that I/O to part of a pma whose pages are
1121+
// being loaded asynchronously does not wait for the rest of the pma to load.
1122+
func TestIODuringAsyncPageLoad(t *testing.T) {
1123+
const (
1124+
size = 2 * asyncLoadMapUnit
1125+
gated = asyncLoadMapUnit
1126+
)
1127+
ctx := contexttest.Context(t)
1128+
1129+
src := newTestMemoryFile(t)
1130+
fr, err := src.Allocate(size, pgalloc.AllocOpts{Kind: usage.Anonymous})
1131+
if err != nil {
1132+
t.Fatalf("Allocate got err %v want nil", err)
1133+
}
1134+
if _, err := unix.Pwrite(src.FD(), bytes.Repeat([]byte{1}, size), int64(fr.Start)); err != nil {
1135+
t.Fatalf("Pwrite got err %v want nil", err)
1136+
}
1137+
var metadata, pages bytes.Buffer
1138+
saveDone := make(chan error, 1)
1139+
apfs, err := pgalloc.StartAsyncPagesFileSave(stateio.NewIOWriter(&pages, 1<<20, 1, 1), func(err error) { saveDone <- err })
1140+
if err != nil {
1141+
t.Fatalf("StartAsyncPagesFileSave got err %v want nil", err)
1142+
}
1143+
if err := src.SaveTo(ctx, &metadata, &pgalloc.SaveOpts{PagesFile: apfs}); err != nil {
1144+
t.Fatalf("SaveTo got err %v want nil", err)
1145+
}
1146+
apfs.MemoryFilesDone()
1147+
if err := <-saveDone; err != nil {
1148+
t.Fatalf("async page save got err %v want nil", err)
1149+
}
1150+
1151+
mf := newTestMemoryFile(t)
1152+
gate := make(chan struct{})
1153+
loadDone := make(chan error, 1)
1154+
apfl, err := pgalloc.StartAsyncPagesFileLoad(stateio.NewIOReader(&gatedReaderAt{bytes.NewReader(pages.Bytes()), gated, gate}, 1<<20, 1, 2), func(err error) { loadDone <- err }, nil, pgalloc.AsyncPagesFileLoadOpts{})
1155+
if err != nil {
1156+
t.Fatalf("StartAsyncPagesFileLoad got err %v want nil", err)
1157+
}
1158+
if err := mf.LoadFrom(ctx, &metadata, &pgalloc.LoadOpts{PagesFile: apfl}); err != nil {
1159+
t.Fatalf("LoadFrom got err %v want nil", err)
1160+
}
1161+
apfl.MemoryFilesDone()
1162+
1163+
// Map the restored pages with a single pma.
1164+
mm := testMemoryManager(ctx, t)
1165+
mm.mf = mf // mm has no mappings yet.
1166+
defer mm.DecUsers(ctx)
1167+
addr, err := mm.MMap(ctx, memmap.MMapOpts{
1168+
Length: size,
1169+
Private: true,
1170+
Perms: hostarch.ReadWrite,
1171+
MaxPerms: hostarch.AnyAccess,
1172+
})
1173+
if err != nil {
1174+
t.Fatalf("MMap got err %v want nil", err)
1175+
}
1176+
ar := hostarch.AddrRange{addr, addr + size}
1177+
mm.activeMu.Lock()
1178+
mm.pmas.Insert(mm.pmas.FindGap(ar.Start), ar, pma{
1179+
file: mf,
1180+
off: fr.Start,
1181+
translatePerms: hostarch.AnyAccess,
1182+
effectivePerms: hostarch.ReadWrite,
1183+
maxPerms: hostarch.AnyAccess,
1184+
private: true,
1185+
})
1186+
mm.addRSSLocked(ar)
1187+
mm.activeMu.Unlock()
1188+
1189+
timer := time.AfterFunc(10*time.Second, func() {
1190+
t.Errorf("CopyIn waited for pages past offset %#x", gated)
1191+
close(gate)
1192+
})
1193+
b := make([]byte, 8)
1194+
_, err = mm.CopyIn(ctx, addr+pageSizeAddr, b, usermem.IOOpts{})
1195+
if timer.Stop() {
1196+
close(gate)
1197+
}
1198+
if err := <-loadDone; err != nil {
1199+
t.Errorf("async page load got err %v want nil", err)
1200+
}
1201+
if err != nil {
1202+
t.Fatalf("CopyIn got err %v want nil", err)
1203+
}
1204+
if want := bytes.Repeat([]byte{1}, len(b)); !bytes.Equal(b, want) {
1205+
t.Errorf("CopyIn got %v want %v", b, want)
1206+
}
1207+
}

‎pkg/sentry/mm/pma.go‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -465,12 +465,13 @@ func (mm *MemoryManager) getPMAsInternalLocked(ctx context.Context, vseg vmaIter
465465
copyAR = pseg.Range().Intersect(hugeMaskAR)
466466
}
467467
// Get internal mappings from the pma to copy from.
468-
if err := pseg.getInternalMappingsLocked(); err != nil {
468+
ims, err := pseg.getInternalMappingsLocked(copyAR)
469+
if err != nil {
469470
return pstart, pseg.PrevGap(), err
470471
}
471472
// Copy contents.
472473
huge := mm.mf.HugepagesEnabled() && copyAR.IsHugePageAligned()
473-
reader := safemem.BlockSeqReader{Blocks: mm.internalMappingsLocked(pseg, copyAR)}
474+
reader := safemem.BlockSeqReader{Blocks: ims}
474475
fr, err := mm.mf.Allocate(uint64(copyAR.Length()), pgalloc.AllocOpts{
475476
Kind: usage.Anonymous,
476477
MemCgID: memCgID,
@@ -1041,12 +1042,18 @@ func (mm *MemoryManager) findOrSeekPrevUpperBoundPMA(addr hostarch.Addr, pgap pm
10411042
return mm.pmas.UpperBoundSegment(addr)
10421043
}
10431044

1044-
// getInternalMappingsLocked ensures that pseg.ValuePtr().internalMappings is
1045-
// non-empty. The generated iterator identifies a set node and index, not its
1046-
// MemoryManager, so checklocks cannot name the owning mutex.
1045+
// getInternalMappingsLocked returns internal mappings for addresses in ar.
1046+
// Unless async page loading is in progress and the pma is larger than
1047+
// `asyncLoadMapUnit`, it caches internal mappings for the whole pma in
1048+
// pseg.ValuePtr().internalMappings. The generated iterator
1049+
// identifies a set node and index, not its MemoryManager, so checklocks cannot
1050+
// name the owning mutex.
10471051
//
1048-
// Preconditions: the owning MemoryManager's activeMu is held for writing.
1049-
func (pseg pmaIterator) getInternalMappingsLocked() error {
1052+
// Preconditions:
1053+
// - The owning MemoryManager's activeMu is held for writing.
1054+
// - pseg.Range().IsSupersetOf(ar).
1055+
// - ar.Length() != 0.
1056+
func (pseg pmaIterator) getInternalMappingsLocked(ar hostarch.AddrRange) (safemem.BlockSeq, error) {
10501057
pma := pseg.ValuePtr()
10511058
if pma.internalMappings.IsEmpty() {
10521059
// This must use maxPerms (instead of perms) because some permission
@@ -1056,13 +1063,16 @@ func (pseg pmaIterator) getInternalMappingsLocked() error {
10561063
perms := pma.maxPerms
10571064
// We will never execute application code through an internal mapping.
10581065
perms.Execute = false
1066+
if mf, ok := pma.file.(*pgalloc.MemoryFile); ok && mf.IsAsyncLoading() && pseg.Range().Length() > asyncLoadMapUnit {
1067+
return mf.MapInternal(pseg.fileRangeOf(ar), perms)
1068+
}
10591069
ims, err := pma.file.MapInternal(pseg.fileRange(), perms)
10601070
if err != nil {
1061-
return err
1071+
return safemem.BlockSeq{}, err
10621072
}
10631073
pma.internalMappings = ims
10641074
}
1065-
return nil
1075+
return pma.internalMappings.DropFirst64(uint64(ar.Start - pseg.Start())).TakeFirst64(uint64(ar.Length())), nil
10661076
}
10671077

10681078
func (pseg pmaIterator) fileRange() memmap.FileRange {

0 commit comments

Comments
 (0)