Skip to content

Commit 63c9278

Browse files
committed
Merge pull request #11360 from Snowflake-Labs:xuzhoyin-vmas-fix
PiperOrigin-RevId: 720834745
2 parents 740b954 + 84d67ab commit 63c9278

File tree

3 files changed

+109
-11
lines changed

3 files changed

+109
-11
lines changed

pkg/sentry/mm/mm_test.go

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,23 @@ import (
2929
"gvisor.dev/gvisor/pkg/usermem"
3030
)
3131

32-
func testMemoryManager(ctx context.Context) *MemoryManager {
32+
func testMemoryManagerWithMmapDirection(ctx context.Context, mmapDirection arch.MmapDirection) *MemoryManager {
3333
p := platform.FromContext(ctx)
3434
mm := NewMemoryManager(p, pgalloc.MemoryFileFromContext(ctx), false)
3535
mm.layout = arch.MmapLayout{
36-
MinAddr: p.MinUserAddress(),
37-
MaxAddr: p.MaxUserAddress(),
38-
BottomUpBase: p.MinUserAddress(),
39-
TopDownBase: p.MaxUserAddress(),
36+
MinAddr: p.MinUserAddress(),
37+
MaxAddr: p.MaxUserAddress(),
38+
BottomUpBase: p.MinUserAddress(),
39+
TopDownBase: p.MaxUserAddress(),
40+
DefaultDirection: mmapDirection,
4041
}
4142
return mm
4243
}
4344

45+
func testMemoryManager(ctx context.Context) *MemoryManager {
46+
return testMemoryManagerWithMmapDirection(ctx, arch.MmapBottomUp)
47+
}
48+
4449
func (mm *MemoryManager) realUsageAS() uint64 {
4550
return uint64(mm.vmas.Span())
4651
}
@@ -272,3 +277,66 @@ func TestAIOLookupAfterDestroy(t *testing.T) {
272277
t.Errorf("AIOContext found even after AIOContext manager is destroyed")
273278
}
274279
}
280+
281+
func TestGetAllocationDirection(t *testing.T) {
282+
testCases := []struct {
283+
name string
284+
mmapDirection arch.MmapDirection
285+
ar hostarch.AddrRange
286+
vma *vma
287+
expected pgalloc.Direction
288+
}{
289+
{
290+
"No last fault in vma with mmap direction BottomUp",
291+
arch.MmapBottomUp,
292+
hostarch.AddrRange{123, 456},
293+
&vma{lastFault: 0},
294+
pgalloc.BottomUp,
295+
},
296+
{
297+
"No last fault in vma with mmap direction TopDown",
298+
arch.MmapTopDown,
299+
hostarch.AddrRange{123, 456},
300+
&vma{lastFault: 0},
301+
pgalloc.TopDown,
302+
},
303+
{
304+
"Last fault in vma equals to addr range, with mmap direction BottomUp",
305+
arch.MmapBottomUp,
306+
hostarch.AddrRange{123, 456},
307+
&vma{lastFault: 123},
308+
pgalloc.BottomUp,
309+
},
310+
{
311+
"Last fault in vma equals to addr range, with mmap direction TopDown",
312+
arch.MmapTopDown,
313+
hostarch.AddrRange{123, 456},
314+
&vma{lastFault: 123},
315+
pgalloc.TopDown,
316+
},
317+
{
318+
"Last fault in vma greater than addr range",
319+
arch.MmapTopDown,
320+
hostarch.AddrRange{123, 456},
321+
&vma{lastFault: 456},
322+
pgalloc.TopDown,
323+
},
324+
{
325+
"Last fault in vma smaller than addr range",
326+
arch.MmapTopDown,
327+
hostarch.AddrRange{123, 456},
328+
&vma{lastFault: 100},
329+
pgalloc.BottomUp,
330+
},
331+
}
332+
for _, test := range testCases {
333+
t.Run(test.name, func(t *testing.T) {
334+
ctx := contexttest.Context(t)
335+
mm := testMemoryManagerWithMmapDirection(ctx, test.mmapDirection)
336+
actual := mm.getAllocationDirection(test.ar, test.vma)
337+
if actual != test.expected {
338+
t.Errorf("Unexpected allocation direction. Expected: %s, Actual: %s", test.expected, actual)
339+
}
340+
})
341+
}
342+
}

pkg/sentry/mm/pma.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"gvisor.dev/gvisor/pkg/hostarch"
2525
"gvisor.dev/gvisor/pkg/safecopy"
2626
"gvisor.dev/gvisor/pkg/safemem"
27+
"gvisor.dev/gvisor/pkg/sentry/arch"
2728
"gvisor.dev/gvisor/pkg/sentry/memmap"
2829
"gvisor.dev/gvisor/pkg/sentry/pgalloc"
2930
"gvisor.dev/gvisor/pkg/sentry/usage"
@@ -185,6 +186,33 @@ func (mm *MemoryManager) getVecPMAsLocked(ctx context.Context, ars hostarch.Addr
185186
return ars, nil
186187
}
187188

189+
// Gets the default memory file offset allocation direction aligned with
190+
// default address space allocation direction.
191+
func (mm *MemoryManager) getDefaultAllocationDirection() pgalloc.Direction {
192+
if mm.layout.DefaultDirection == arch.MmapTopDown {
193+
return pgalloc.TopDown
194+
}
195+
return pgalloc.BottomUp
196+
}
197+
198+
// Gets the memory file offset allocation direction based on address space allocation direction
199+
// and memory access order.
200+
func (mm *MemoryManager) getAllocationDirection(ar hostarch.AddrRange, vma *vma) pgalloc.Direction {
201+
lastFault := atomic.LoadUintptr(&vma.lastFault)
202+
arStart := uintptr(ar.Start)
203+
// If this VMA does not have last page fault or last page fault equals to arStart,
204+
// use the default allocation direction.
205+
if lastFault == 0 || lastFault == arStart {
206+
return mm.getDefaultAllocationDirection()
207+
}
208+
// Detect cases where memory is accessed downwards and change memory file
209+
// allocation order to increase the chances that pages are coalesced.
210+
if arStart < lastFault {
211+
return pgalloc.TopDown
212+
}
213+
return pgalloc.BottomUp
214+
}
215+
188216
// getPMAsInternalLocked is equivalent to getPMAsLocked, with the following
189217
// exceptions:
190218
//
@@ -220,12 +248,7 @@ func (mm *MemoryManager) getPMAsInternalLocked(ctx context.Context, vseg vmaIter
220248

221249
vma := vseg.ValuePtr()
222250
memCgID := pgalloc.MemoryCgroupIDFromContext(ctx)
223-
allocDir := pgalloc.BottomUp
224-
if uintptr(ar.Start) < atomic.LoadUintptr(&vma.lastFault) {
225-
// Detect cases where memory is accessed downwards and change memory file
226-
// allocation order to increase the chances that pages are coalesced.
227-
allocDir = pgalloc.TopDown
228-
}
251+
allocDir := mm.getAllocationDirection(ar, vma)
229252
atomic.StoreUintptr(&vma.lastFault, uintptr(ar.Start))
230253

231254
// Limit the range we allocate to ar, aligned to hugepage boundaries.

pkg/sentry/mm/vma.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,13 @@ func (vmaSetFunctions) Merge(ar1 hostarch.AddrRange, vma1 vma, ar2 hostarch.Addr
484484
// need to worry about whether we're in a mm.mappingMu critical section.
485485
vma2.id.DecRef(context.Background())
486486
}
487+
488+
// If the existing vma (vma2) has non-zero lastFault address,
489+
// we should preserve it to the resulting merged-VMA
490+
if vma1.lastFault == 0 {
491+
vma1.lastFault = vma2.lastFault
492+
}
493+
487494
return vma1, true
488495
}
489496

0 commit comments

Comments
 (0)