diff --git a/pkg/sentry/mm/BUILD b/pkg/sentry/mm/BUILD index b7d782b7f..8fa6821dd 100644 --- a/pkg/sentry/mm/BUILD +++ b/pkg/sentry/mm/BUILD @@ -1,8 +1,51 @@ load("//tools:defs.bzl", "go_library", "go_test") load("//tools/go_generics:defs.bzl", "go_template_instance") +load("//pkg/sync/locking:locking.bzl", "declare_mutex", "declare_rwmutex") package(licenses = ["notice"]) +declare_mutex( + name = "aio_context_mutex", + out = "aio_context_mutex.go", + package = "mm", + prefix = "aioContext", +) + +declare_mutex( + name = "aio_manager_mutex", + out = "aio_manager_mutex.go", + package = "mm", + prefix = "aioManager", +) + +declare_rwmutex( + name = "mapping_mutex", + out = "mapping_mutex.go", + package = "mm", + prefix = "mapping", +) + +declare_rwmutex( + name = "active_mutex", + out = "active_mutex.go", + package = "mm", + prefix = "active", +) + +declare_mutex( + name = "metadata_mutex", + out = "metadata_mutex.go", + package = "mm", + prefix = "metadata", +) + +declare_mutex( + name = "private_refs_mutex", + out = "private_refs_mutex.go", + package = "mm", + prefix = "privateRefs", +) + go_template_instance( name = "file_refcount_set", out = "file_refcount_set.go", @@ -98,19 +141,25 @@ go_template_instance( go_library( name = "mm", srcs = [ + "active_mutex.go", "address_space.go", "aio_context.go", + "aio_context_mutex.go", "aio_context_state.go", + "aio_manager_mutex.go", "aio_mappable_refs.go", "debug.go", "file_refcount_set.go", "io.go", "io_list.go", "lifecycle.go", + "mapping_mutex.go", "metadata.go", + "metadata_mutex.go", "mm.go", "pma.go", "pma_set.go", + "private_refs_mutex.go", "procfs.go", "save_restore.go", "shm.go", @@ -144,6 +193,7 @@ go_library( "//pkg/sentry/platform", "//pkg/sentry/usage", "//pkg/sync", + "//pkg/sync/locking", "//pkg/tcpip/buffer", "//pkg/usermem", ], diff --git a/pkg/sentry/mm/aio_context.go b/pkg/sentry/mm/aio_context.go index a10587b0f..610556922 100644 --- a/pkg/sentry/mm/aio_context.go +++ b/pkg/sentry/mm/aio_context.go @@ -22,7 +22,6 @@ import ( "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" ) @@ -31,7 +30,7 @@ import ( // +stateify savable type aioManager struct { // mu protects below. - mu sync.Mutex `state:"nosave"` + mu aioManagerMutex `state:"nosave"` // aioContexts is the set of asynchronous I/O contexts. contexts map[uint64]*AIOContext @@ -108,7 +107,7 @@ type AIOContext struct { requestReady chan struct{} `state:"nosave"` // mu protects below. - mu sync.Mutex `state:"nosave"` + mu aioContextMutex `state:"nosave"` // results is the set of completed requests. results ioList diff --git a/pkg/sentry/mm/debug.go b/pkg/sentry/mm/debug.go index c273c982e..da7fd3f80 100644 --- a/pkg/sentry/mm/debug.go +++ b/pkg/sentry/mm/debug.go @@ -40,10 +40,11 @@ func (mm *MemoryManager) String() string { // DebugString returns a string containing information about mm for debugging. func (mm *MemoryManager) DebugString(ctx context.Context) string { - mm.mappingMu.RLock() - defer mm.mappingMu.RUnlock() - mm.activeMu.RLock() - defer mm.activeMu.RUnlock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.mappingMu.RLockBypass() + defer mm.mappingMu.RUnlockBypass() + mm.activeMu.RLockBypass() + defer mm.activeMu.RUnlockBypass() return mm.debugStringLocked(ctx) } diff --git a/pkg/sentry/mm/lifecycle.go b/pkg/sentry/mm/lifecycle.go index 3c1f4d033..e8872188b 100644 --- a/pkg/sentry/mm/lifecycle.go +++ b/pkg/sentry/mm/lifecycle.go @@ -141,8 +141,8 @@ func (mm *MemoryManager) Fork(ctx context.Context) (*MemoryManager, error) { // mm/memory.c:copy_page_range().) mm2.activeMu.Lock() defer mm2.activeMu.Unlock() - mm.activeMu.Lock() - defer mm.activeMu.Unlock() + mm.activeMu.NestedLock() + defer mm.activeMu.NestedUnlock() if dontforks { defer mm.pmas.MergeRange(mm.applicationAddrRange()) } diff --git a/pkg/sentry/mm/mm.go b/pkg/sentry/mm/mm.go index 16f6e58e4..0634953a5 100644 --- a/pkg/sentry/mm/mm.go +++ b/pkg/sentry/mm/mm.go @@ -47,7 +47,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/memmap" "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/platform" - "gvisor.dev/gvisor/pkg/sync" ) // MemoryManager implements a virtual address space. @@ -83,7 +82,7 @@ type MemoryManager struct { users atomicbitops.Int32 // mappingMu is analogous to Linux's struct mm_struct::mmap_sem. - mappingMu sync.RWMutex `state:"nosave"` + mappingMu mappingRWMutex `state:"nosave"` // vmas stores virtual memory areas. Since vmas are stored by value, // clients should usually use vmaIterator.ValuePtr() instead of @@ -126,7 +125,7 @@ type MemoryManager struct { // activeMu is loosely analogous to Linux's struct // mm_struct::page_table_lock. - activeMu sync.RWMutex `state:"nosave"` + activeMu activeRWMutex `state:"nosave"` // pmas stores platform mapping areas used to implement vmas. Since pmas // are stored by value, clients should usually use pmaIterator.ValuePtr() @@ -198,7 +197,7 @@ type MemoryManager struct { // by metadataMu. dumpability atomicbitops.Int32 - metadataMu sync.Mutex `state:"nosave"` + metadataMu metadataMutex `state:"nosave"` // argv is the application argv. This is set up by the loader and may be // modified by prctl(PR_SET_MM_ARG_START/PR_SET_MM_ARG_END). No @@ -482,7 +481,7 @@ type pma struct { // +stateify savable type privateRefs struct { - mu sync.Mutex `state:"nosave"` + mu privateRefsMutex `state:"nosave"` // refs maps offsets into MemoryManager.mfp.MemoryFile() to the number of // pmas (or, equivalently, MemoryManagers) that share ownership of the diff --git a/pkg/sentry/mm/procfs.go b/pkg/sentry/mm/procfs.go index f1440e884..57166c178 100644 --- a/pkg/sentry/mm/procfs.go +++ b/pkg/sentry/mm/procfs.go @@ -60,8 +60,9 @@ func (mm *MemoryManager) NeedsUpdate(generation int64) bool { // ReadMapsDataInto is called by fsimpl/proc.mapsData.Generate to // implement /proc/[pid]/maps. func (mm *MemoryManager) ReadMapsDataInto(ctx context.Context, buf *bytes.Buffer) { - mm.mappingMu.RLock() - defer mm.mappingMu.RUnlock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.mappingMu.RLockBypass() + defer mm.mappingMu.RUnlockBypass() var start hostarch.Addr for vseg := mm.vmas.LowerBoundSegment(start); vseg.Ok(); vseg = vseg.NextSegment() { @@ -85,8 +86,9 @@ func (mm *MemoryManager) ReadMapsDataInto(ctx context.Context, buf *bytes.Buffer // ReadMapsSeqFileData is called by fs/proc.mapsData.ReadSeqFileData to // implement /proc/[pid]/maps. func (mm *MemoryManager) ReadMapsSeqFileData(ctx context.Context, handle seqfile.SeqHandle) ([]seqfile.SeqData, int64) { - mm.mappingMu.RLock() - defer mm.mappingMu.RUnlock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.mappingMu.RLockBypass() + defer mm.mappingMu.RUnlockBypass() var data []seqfile.SeqData var start hostarch.Addr if handle != nil { @@ -175,8 +177,9 @@ func (mm *MemoryManager) appendVMAMapsEntryLocked(ctx context.Context, vseg vmaI // ReadSmapsDataInto is called by fsimpl/proc.smapsData.Generate to // implement /proc/[pid]/maps. func (mm *MemoryManager) ReadSmapsDataInto(ctx context.Context, buf *bytes.Buffer) { - mm.mappingMu.RLock() - defer mm.mappingMu.RUnlock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.mappingMu.RLockBypass() + defer mm.mappingMu.RUnlockBypass() var start hostarch.Addr for vseg := mm.vmas.LowerBoundSegment(start); vseg.Ok(); vseg = vseg.NextSegment() { @@ -193,8 +196,9 @@ func (mm *MemoryManager) ReadSmapsDataInto(ctx context.Context, buf *bytes.Buffe // ReadSmapsSeqFileData is called by fs/proc.smapsData.ReadSeqFileData to // implement /proc/[pid]/smaps. func (mm *MemoryManager) ReadSmapsSeqFileData(ctx context.Context, handle seqfile.SeqHandle) ([]seqfile.SeqData, int64) { - mm.mappingMu.RLock() - defer mm.mappingMu.RUnlock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.mappingMu.RLockBypass() + defer mm.mappingMu.RUnlockBypass() var data []seqfile.SeqData var start hostarch.Addr if handle != nil { @@ -238,7 +242,8 @@ func (mm *MemoryManager) vmaSmapsEntryIntoLocked(ctx context.Context, vseg vmaIt // requiring it to be locked as a precondition, to reduce the latency // impact of reading /proc/[pid]/smaps on concurrent performance-sensitive // operations requiring activeMu for writing like faults. - mm.activeMu.RLock() + // FIXME(b/207524689): replace RLockBypass with RLock. + mm.activeMu.RLockBypass() var rss uint64 var anon uint64 vsegAR := vseg.Range() @@ -250,7 +255,7 @@ func (mm *MemoryManager) vmaSmapsEntryIntoLocked(ctx context.Context, vseg vmaIt anon += size } } - mm.activeMu.RUnlock() + mm.activeMu.RUnlockBypass() fmt.Fprintf(b, "Size: %8d kB\n", vseg.Range().Length()/1024) fmt.Fprintf(b, "Rss: %8d kB\n", rss/1024) diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index 911a58f47..cae187153 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -1285,8 +1285,9 @@ func (mm *MemoryManager) ResidentSetSize() uint64 { // MaxResidentSetSize returns the value advertised as mm's max RSS in bytes. func (mm *MemoryManager) MaxResidentSetSize() uint64 { - mm.activeMu.RLock() - defer mm.activeMu.RUnlock() + // FIXME(b/229424837): Repalce RLockBypass with RLock. + mm.activeMu.RLockBypass() + defer mm.activeMu.RUnlockBypass() return mm.maxRSS }