From d17ca0c82c346763855f7a4e82a8acaf89b3347e Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 14 Nov 2024 20:43:10 +0800 Subject: [PATCH 01/24] Annotate mmap ranges using PR_SET_VMA We demand that every invocation of `mmap` within mmtk-core to be accompanied with an "annotation" for the purpose of the mmap. On Linux, we will use `PR_SET_VMA` to set the annotation after `mmap` so that it can be seen in `/proc/pid/maps`. This will greatly improve the debugging experience. --- src/policy/lockfreeimmortalspace.rs | 13 +- .../marksweepspace/malloc_ms/metadata.rs | 4 +- src/policy/sft_map.rs | 2 +- src/policy/space.rs | 21 ++- src/util/heap/layout/byte_map_mmapper.rs | 44 ++++-- src/util/heap/layout/fragmented_mapper.rs | 35 ++++- src/util/heap/layout/mmapper.rs | 22 ++- src/util/memory.rs | 144 +++++++++++++++--- src/util/metadata/side_metadata/global.rs | 54 +++++-- src/util/metadata/side_metadata/helpers.rs | 5 +- .../side_metadata/side_metadata_tests.rs | 17 ++- src/util/raw_memory_freelist.rs | 10 +- 12 files changed, 295 insertions(+), 76 deletions(-) diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index 05cf448001..d9ef34ae26 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -13,6 +13,7 @@ use crate::util::heap::gc_trigger::GCTrigger; use crate::util::heap::layout::vm_layout::vm_layout; use crate::util::heap::PageResource; use crate::util::heap::VMRequest; +use crate::util::memory::MmapAnno; use crate::util::memory::MmapStrategy; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::metadata::side_metadata::SideMetadataSanity; @@ -241,10 +242,18 @@ impl LockFreeImmortalSpace { *args.options.transparent_hugepages, crate::util::memory::MmapProtection::ReadWrite, ); - crate::util::memory::dzmmap_noreplace(start, aligned_total_bytes, strategy).unwrap(); + crate::util::memory::dzmmap_noreplace( + start, + aligned_total_bytes, + strategy, + &MmapAnno::Space { + name: "LockFreeImmortalSpace", + }, + ) + .unwrap(); if space .metadata - .try_map_metadata_space(start, aligned_total_bytes) + .try_map_metadata_space(start, aligned_total_bytes, "mmtk:LockFreeImmortalSpace") .is_err() { // TODO(Javad): handle meta space allocation failure diff --git a/src/policy/marksweepspace/malloc_ms/metadata.rs b/src/policy/marksweepspace/malloc_ms/metadata.rs index 7fb5b5738a..faf779bb27 100644 --- a/src/policy/marksweepspace/malloc_ms/metadata.rs +++ b/src/policy/marksweepspace/malloc_ms/metadata.rs @@ -100,7 +100,7 @@ fn map_active_chunk_metadata(chunk_start: Address) { ); assert!( - CHUNK_METADATA.try_map_metadata_space(start, size).is_ok(), + CHUNK_METADATA.try_map_metadata_space(start, size, "mmtk:map_active_chunk_metadata:assert").is_ok(), "failed to mmap meta memory" ); } @@ -131,7 +131,7 @@ pub(super) fn map_meta_space(metadata: &SideMetadataContext, addr: Address, size // Note that this might fail. For example, we have marked a chunk as active but later we freed all // the objects in it, and unset its chunk bit. However, we do not free its metadata. So for the chunk, // its chunk bit is mapped, but not marked, and all its local metadata is also mapped. - let mmap_metadata_result = metadata.try_map_metadata_space(start, BYTES_IN_CHUNK); + let mmap_metadata_result = metadata.try_map_metadata_space(start, BYTES_IN_CHUNK, "mmtk:malloc_ms:map_meta_space"); debug_assert!( mmap_metadata_result.is_ok(), "mmap sidemetadata failed for chunk_start ({})", diff --git a/src/policy/sft_map.rs b/src/policy/sft_map.rs index 8fccf9cd5f..db1862715b 100644 --- a/src/policy/sft_map.rs +++ b/src/policy/sft_map.rs @@ -378,7 +378,7 @@ mod dense_chunk_map { global: vec![SFT_DENSE_CHUNK_MAP_INDEX], local: vec![], }; - if context.try_map_metadata_space(start, bytes).is_err() { + if context.try_map_metadata_space(start, bytes, "mmtk:SFTDenseChunkMap").is_err() { panic!("failed to mmap metadata memory"); } diff --git a/src/policy/space.rs b/src/policy/space.rs index 8048009b20..e125a7bc5e 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -144,12 +144,19 @@ pub trait Space: 'static + SFT + Sync + Downcast { if let Err(mmap_error) = self .common() .mmapper - .ensure_mapped(res.start, res.pages, self.common().mmap_strategy()) - .and( - self.common() - .metadata - .try_map_metadata_space(res.start, bytes), + .ensure_mapped( + res.start, + res.pages, + self.common().mmap_strategy(), + &memory::MmapAnno::Space { + name: self.get_name(), + }, ) + .and(self.common().metadata.try_map_metadata_space( + res.start, + bytes, + self.name(), + )) { memory::handle_mmap_error::(mmap_error, tls, res.start, bytes); } @@ -296,7 +303,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { if self .common() .metadata - .try_map_metadata_space(self.common().start, self.common().extent) + .try_map_metadata_space(self.common().start, self.common().extent, self.name()) .is_err() { // TODO(Javad): handle meta space allocation failure @@ -611,7 +618,7 @@ impl CommonSpace { // For contiguous space, we know its address range so we reserve metadata memory for its range. if rtn .metadata - .try_map_metadata_address_range(rtn.start, rtn.extent) + .try_map_metadata_address_range(rtn.start, rtn.extent, rtn.name) .is_err() { // TODO(Javad): handle meta space allocation failure diff --git a/src/util/heap/layout/byte_map_mmapper.rs b/src/util/heap/layout/byte_map_mmapper.rs index ff5dc8c8fe..08205882d6 100644 --- a/src/util/heap/layout/byte_map_mmapper.rs +++ b/src/util/heap/layout/byte_map_mmapper.rs @@ -1,5 +1,6 @@ use super::mmapper::MapState; use super::Mmapper; +use crate::util::memory::MmapAnno; use crate::util::Address; use crate::util::constants::*; @@ -44,7 +45,13 @@ impl Mmapper for ByteMapMmapper { } } - fn ensure_mapped(&self, start: Address, pages: usize, strategy: MmapStrategy) -> Result<()> { + fn ensure_mapped( + &self, + start: Address, + pages: usize, + strategy: MmapStrategy, + anno: &MmapAnno, + ) -> Result<()> { let start_chunk = Self::address_to_mmap_chunks_down(start); let end_chunk = Self::address_to_mmap_chunks_up(start + pages_to_bytes(pages)); trace!( @@ -62,7 +69,8 @@ impl Mmapper for ByteMapMmapper { let mmap_start = Self::mmap_chunks_to_address(chunk); let _guard = self.lock.lock().unwrap(); - MapState::transition_to_mapped(&self.mapped[chunk], mmap_start, strategy).unwrap(); + MapState::transition_to_mapped(&self.mapped[chunk], mmap_start, strategy, anno) + .unwrap(); } Ok(()) @@ -73,6 +81,7 @@ impl Mmapper for ByteMapMmapper { start: Address, pages: usize, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { let start_chunk = Self::address_to_mmap_chunks_down(start); let end_chunk = Self::address_to_mmap_chunks_up(start + pages_to_bytes(pages)); @@ -91,7 +100,8 @@ impl Mmapper for ByteMapMmapper { let mmap_start = Self::mmap_chunks_to_address(chunk); let _guard = self.lock.lock().unwrap(); - MapState::transition_to_quarantined(&self.mapped[chunk], mmap_start, strategy).unwrap(); + MapState::transition_to_quarantined(&self.mapped[chunk], mmap_start, strategy, anno) + .unwrap(); } Ok(()) @@ -172,6 +182,7 @@ impl Default for ByteMapMmapper { #[cfg(test)] mod tests { use super::ByteMapMmapper; + use crate::mmap_anno_test; use crate::util::heap::layout::Mmapper; use crate::util::Address; @@ -237,7 +248,7 @@ mod tests { || { let mmapper = ByteMapMmapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); for chunk in start_chunk..end_chunk { @@ -266,7 +277,7 @@ mod tests { || { let mmapper = ByteMapMmapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); for chunk in start_chunk..end_chunk { @@ -295,7 +306,7 @@ mod tests { || { let mmapper = ByteMapMmapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); let start_chunk = ByteMapMmapper::address_to_mmap_chunks_down(FIXED_ADDRESS); @@ -329,7 +340,12 @@ mod tests { // map 2 chunks let mmapper = ByteMapMmapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, test_memory_pages, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + test_memory_pages, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); // protect 1 chunk @@ -364,7 +380,12 @@ mod tests { // map 2 chunks let mmapper = ByteMapMmapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, test_memory_pages, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + test_memory_pages, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); // protect 1 chunk @@ -382,7 +403,12 @@ mod tests { // ensure mapped - this will unprotect the previously protected chunk mmapper - .ensure_mapped(FIXED_ADDRESS, protect_memory_pages_2, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + protect_memory_pages_2, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); assert_eq!( mmapper.mapped[chunk].load(Ordering::Relaxed), diff --git a/src/util/heap/layout/fragmented_mapper.rs b/src/util/heap/layout/fragmented_mapper.rs index 53ce26c441..29cdeedee1 100644 --- a/src/util/heap/layout/fragmented_mapper.rs +++ b/src/util/heap/layout/fragmented_mapper.rs @@ -3,7 +3,7 @@ use super::Mmapper; use crate::util::constants::BYTES_IN_PAGE; use crate::util::conversions; use crate::util::heap::layout::vm_layout::*; -use crate::util::memory::MmapStrategy; +use crate::util::memory::{MmapAnno, MmapStrategy}; use crate::util::Address; use atomic::{Atomic, Ordering}; use std::cell::UnsafeCell; @@ -94,6 +94,7 @@ impl Mmapper for FragmentedMapper { mut start: Address, pages: usize, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { debug_assert!(start.is_aligned_to(BYTES_IN_PAGE)); @@ -140,6 +141,7 @@ impl Mmapper for FragmentedMapper { state_slices.as_slice(), mmap_start, strategy, + anno, )?; } @@ -151,6 +153,7 @@ impl Mmapper for FragmentedMapper { mut start: Address, pages: usize, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { let end = start + conversions::pages_to_bytes(pages); // Iterate over the slabs covered @@ -176,7 +179,7 @@ impl Mmapper for FragmentedMapper { let mmap_start = Self::chunk_index_to_address(base, chunk); let _guard = self.lock.lock().unwrap(); - MapState::transition_to_mapped(entry, mmap_start, strategy)?; + MapState::transition_to_mapped(entry, mmap_start, strategy, anno)?; } start = high; } @@ -393,6 +396,7 @@ impl Default for FragmentedMapper { #[cfg(test)] mod tests { use super::*; + use crate::mmap_anno_test; use crate::util::constants::LOG_BYTES_IN_PAGE; use crate::util::heap::layout::vm_layout::MMAP_CHUNK_BYTES; use crate::util::memory; @@ -446,7 +450,7 @@ mod tests { || { let mmapper = FragmentedMapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); let chunks = pages_to_chunks_up(pages); @@ -474,7 +478,7 @@ mod tests { || { let mmapper = FragmentedMapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); let chunks = pages_to_chunks_up(pages); @@ -503,7 +507,7 @@ mod tests { || { let mmapper = FragmentedMapper::new(); mmapper - .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST) + .ensure_mapped(FIXED_ADDRESS, pages, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); let chunks = pages_to_chunks_up(pages); @@ -533,7 +537,12 @@ mod tests { let mmapper = FragmentedMapper::new(); let pages_per_chunk = MMAP_CHUNK_BYTES >> LOG_BYTES_IN_PAGE as usize; mmapper - .ensure_mapped(FIXED_ADDRESS, pages_per_chunk * 2, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + pages_per_chunk * 2, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); // protect 1 chunk @@ -564,7 +573,12 @@ mod tests { let mmapper = FragmentedMapper::new(); let pages_per_chunk = MMAP_CHUNK_BYTES >> LOG_BYTES_IN_PAGE as usize; mmapper - .ensure_mapped(FIXED_ADDRESS, pages_per_chunk * 2, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + pages_per_chunk * 2, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); // protect 1 chunk @@ -581,7 +595,12 @@ mod tests { // ensure mapped - this will unprotect the previously protected chunk mmapper - .ensure_mapped(FIXED_ADDRESS, pages_per_chunk * 2, MmapStrategy::TEST) + .ensure_mapped( + FIXED_ADDRESS, + pages_per_chunk * 2, + MmapStrategy::TEST, + mmap_anno_test!(), + ) .unwrap(); assert_eq!( get_chunk_map_state(&mmapper, FIXED_ADDRESS), diff --git a/src/util/heap/layout/mmapper.rs b/src/util/heap/layout/mmapper.rs index 9039bfe261..4a528a3298 100644 --- a/src/util/heap/layout/mmapper.rs +++ b/src/util/heap/layout/mmapper.rs @@ -37,6 +37,7 @@ pub trait Mmapper: Sync { start: Address, pages: usize, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()>; /// Ensure that a range of pages is mmapped (or equivalent). If the @@ -49,7 +50,13 @@ pub trait Mmapper: Sync { // NOTE: There is a monotonicity assumption so that only updates require lock // acquisition. // TODO: Fix the above to support unmapping. - fn ensure_mapped(&self, start: Address, pages: usize, strategy: MmapStrategy) -> Result<()>; + fn ensure_mapped( + &self, + start: Address, + pages: usize, + strategy: MmapStrategy, + anno: &MmapAnno, + ) -> Result<()>; /// Is the page pointed to by this address mapped? Returns true if /// the page at the given address is mapped. @@ -88,6 +95,7 @@ impl MapState { state: &Atomic, mmap_start: Address, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { trace!( "Trying to map {} - {}", @@ -95,9 +103,11 @@ impl MapState { mmap_start + MMAP_CHUNK_BYTES ); let res = match state.load(Ordering::Relaxed) { - MapState::Unmapped => dzmmap_noreplace(mmap_start, MMAP_CHUNK_BYTES, strategy), + MapState::Unmapped => dzmmap_noreplace(mmap_start, MMAP_CHUNK_BYTES, strategy, anno), MapState::Protected => munprotect(mmap_start, MMAP_CHUNK_BYTES, strategy.prot), - MapState::Quarantined => unsafe { dzmmap(mmap_start, MMAP_CHUNK_BYTES, strategy) }, + MapState::Quarantined => unsafe { + dzmmap(mmap_start, MMAP_CHUNK_BYTES, strategy, anno) + }, // might have become MapState::Mapped here MapState::Mapped => Ok(()), }; @@ -113,6 +123,7 @@ impl MapState { state: &Atomic, mmap_start: Address, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { trace!( "Trying to quarantine {} - {}", @@ -120,7 +131,7 @@ impl MapState { mmap_start + MMAP_CHUNK_BYTES ); let res = match state.load(Ordering::Relaxed) { - MapState::Unmapped => mmap_noreserve(mmap_start, MMAP_CHUNK_BYTES, strategy), + MapState::Unmapped => mmap_noreserve(mmap_start, MMAP_CHUNK_BYTES, strategy, anno), MapState::Quarantined => Ok(()), MapState::Mapped => { // If a chunk is mapped by us and we try to quarantine it, we simply don't do anything. @@ -157,6 +168,7 @@ impl MapState { state_slices: &[&[Atomic]], mmap_start: Address, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { trace!( "Trying to bulk-quarantine {} - {}", @@ -179,7 +191,7 @@ impl MapState { match group.key { MapState::Unmapped => { trace!("Trying to quarantine {} - {}", start_addr, end_addr); - mmap_noreserve(start_addr, end_addr - start_addr, strategy)?; + mmap_noreserve(start_addr, end_addr - start_addr, strategy, anno)?; for state in group { state.store(MapState::Quarantined, Ordering::Relaxed); diff --git a/src/util/memory.rs b/src/util/memory.rs index 771068aaff..9383876d52 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -4,6 +4,7 @@ use crate::util::Address; use crate::vm::{Collection, VMBinding}; use bytemuck::NoUninit; use libc::{PROT_EXEC, PROT_NONE, PROT_READ, PROT_WRITE}; +use std::ffi::CString; use std::io::{Error, Result}; use sysinfo::MemoryRefreshKind; use sysinfo::{RefreshKind, System}; @@ -84,6 +85,38 @@ pub enum HugePageSupport { TransparentHugePages, } +/// Annotation for an mmap entry. +/// +/// This is mainly for debugging purpose. On Linux, mmtk-core will use `prctl` with `PR_SET_VMA` +/// to set the human-readable name for the given mmap region. +pub enum MmapAnno<'a> { + Space { name: &'a str }, + SideMeta { space: &'a str, meta: &'a str }, + Test { file: &'a str, line: u32 }, + Misc { name: &'a str }, +} + +#[macro_export] +macro_rules! mmap_anno_test { + () => { + &$crate::util::memory::MmapAnno::Test { + file: file!(), + line: line!(), + } + }; +} + +impl<'a> std::fmt::Display for MmapAnno<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MmapAnno::Space { name } => write!(f, "mmtk:space:{name}"), + MmapAnno::SideMeta { space, meta } => write!(f, "mmtk:sidemeta:{space}:{meta}"), + MmapAnno::Test { file, line } => write!(f, "mmtk:test:{file}:{line}"), + MmapAnno::Misc { name } => write!(f, "mmtk:misc:{name}"), + } + } +} + /// Check the result from an mmap function in this module. /// Return true if the mmap has failed due to an existing conflicting mapping. pub(crate) fn result_is_mapped(result: Result<()>) -> bool { @@ -115,9 +148,14 @@ pub fn set(start: Address, val: u8, len: usize) { /// the memory has been reserved by mmtk (e.g. after the use of mmap_noreserve()). Otherwise using this function /// may corrupt others' data. #[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s -pub unsafe fn dzmmap(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> { +pub unsafe fn dzmmap( + start: Address, + size: usize, + strategy: MmapStrategy, + anno: &MmapAnno, +) -> Result<()> { let flags = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; - let ret = mmap_fixed(start, size, flags, strategy); + let ret = mmap_fixed(start, size, flags, strategy, anno); // We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed) #[cfg(not(target_os = "linux"))] if ret.is_ok() { @@ -129,9 +167,14 @@ pub unsafe fn dzmmap(start: Address, size: usize, strategy: MmapStrategy) -> Res /// This function mmaps the memory and guarantees to zero all mapped memory. /// This function will not overwrite existing memory mapping, and it will result Err if there is an existing mapping. #[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s -pub fn dzmmap_noreplace(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> { +pub fn dzmmap_noreplace( + start: Address, + size: usize, + strategy: MmapStrategy, + anno: &MmapAnno, +) -> Result<()> { let flags = MMAP_FLAGS; - let ret = mmap_fixed(start, size, flags, strategy); + let ret = mmap_fixed(start, size, flags, strategy, anno); // We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed) #[cfg(not(target_os = "linux"))] if ret.is_ok() { @@ -144,10 +187,15 @@ pub fn dzmmap_noreplace(start: Address, size: usize, strategy: MmapStrategy) -> /// This function does not reserve swap space for this mapping, which means there is no guarantee that writes to the /// mapping can always be successful. In case of out of physical memory, one may get a segfault for writing to the mapping. /// We can use this to reserve the address range, and then later overwrites the mapping with dzmmap(). -pub fn mmap_noreserve(start: Address, size: usize, mut strategy: MmapStrategy) -> Result<()> { +pub fn mmap_noreserve( + start: Address, + size: usize, + mut strategy: MmapStrategy, + anno: &MmapAnno, +) -> Result<()> { strategy.prot = MmapProtection::NoAccess; let flags = MMAP_FLAGS | libc::MAP_NORESERVE; - mmap_fixed(start, size, flags, strategy) + mmap_fixed(start, size, flags, strategy, anno) } fn mmap_fixed( @@ -155,6 +203,7 @@ fn mmap_fixed( size: usize, flags: libc::c_int, strategy: MmapStrategy, + anno: &MmapAnno, ) -> Result<()> { let ptr = start.to_mut_ptr(); let prot = strategy.prot.into_native_flags(); @@ -162,6 +211,21 @@ fn mmap_fixed( &|| unsafe { libc::mmap(start.to_mut_ptr(), size, prot, flags, -1, 0) }, ptr, )?; + let anno_str = anno.to_string(); + let anno_cstr = CString::new(anno_str).unwrap(); + wrap_libc_call( + &|| unsafe { + libc::prctl( + libc::PR_SET_VMA, + libc::PR_SET_VMA_ANON_NAME, + start.to_ptr::(), + size, + anno_cstr.as_ptr(), + ) + }, + 0, + ) + .unwrap(); match strategy.huge_page { HugePageSupport::No => Ok(()), HugePageSupport::TransparentHugePages => { @@ -232,7 +296,7 @@ pub fn handle_mmap_error( /// Note that the checking has a side effect that it will map the memory if it was unmapped. So we panic if it was unmapped. /// Be very careful about using this function. #[cfg(target_os = "linux")] -pub(crate) fn panic_if_unmapped(start: Address, size: usize) { +pub(crate) fn panic_if_unmapped(start: Address, size: usize, anno: &MmapAnno) { let flags = MMAP_FLAGS; match mmap_fixed( start, @@ -242,6 +306,7 @@ pub(crate) fn panic_if_unmapped(start: Address, size: usize) { huge_page: HugePageSupport::No, prot: MmapProtection::ReadWrite, }, + anno, ) { Ok(_) => panic!("{} of size {} is not mapped", start, size), Err(e) => { @@ -371,10 +436,14 @@ mod tests { serial_test(|| { with_cleanup( || { - let res = unsafe { dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST) }; + let res = unsafe { + dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST, mmap_anno_test!()) + }; assert!(res.is_ok()); // We can overwrite with dzmmap - let res = unsafe { dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST) }; + let res = unsafe { + dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST, mmap_anno_test!()) + }; assert!(res.is_ok()); }, || { @@ -389,7 +458,12 @@ mod tests { serial_test(|| { with_cleanup( || { - let res = dzmmap_noreplace(START, BYTES_IN_PAGE, MmapStrategy::TEST); + let res = dzmmap_noreplace( + START, + BYTES_IN_PAGE, + MmapStrategy::TEST, + mmap_anno_test!(), + ); assert!(res.is_ok()); let res = munmap(START, BYTES_IN_PAGE); assert!(res.is_ok()); @@ -408,10 +482,17 @@ mod tests { with_cleanup( || { // Make sure we mmapped the memory - let res = unsafe { dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST) }; + let res = unsafe { + dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST, mmap_anno_test!()) + }; assert!(res.is_ok()); // Use dzmmap_noreplace will fail - let res = dzmmap_noreplace(START, BYTES_IN_PAGE, MmapStrategy::TEST); + let res = dzmmap_noreplace( + START, + BYTES_IN_PAGE, + MmapStrategy::TEST, + mmap_anno_test!(), + ); assert!(res.is_err()); }, || { @@ -426,10 +507,13 @@ mod tests { serial_test(|| { with_cleanup( || { - let res = mmap_noreserve(START, BYTES_IN_PAGE, MmapStrategy::TEST); + let res = + mmap_noreserve(START, BYTES_IN_PAGE, MmapStrategy::TEST, mmap_anno_test!()); assert!(res.is_ok()); // Try reserve it - let res = unsafe { dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST) }; + let res = unsafe { + dzmmap(START, BYTES_IN_PAGE, MmapStrategy::TEST, mmap_anno_test!()) + }; assert!(res.is_ok()); }, || { @@ -447,7 +531,7 @@ mod tests { with_cleanup( || { // We expect this call to panic - panic_if_unmapped(START, BYTES_IN_PAGE); + panic_if_unmapped(START, BYTES_IN_PAGE, mmap_anno_test!()); }, || { assert!(munmap(START, BYTES_IN_PAGE).is_ok()); @@ -461,8 +545,14 @@ mod tests { serial_test(|| { with_cleanup( || { - assert!(dzmmap_noreplace(START, BYTES_IN_PAGE, MmapStrategy::TEST).is_ok()); - panic_if_unmapped(START, BYTES_IN_PAGE); + assert!(dzmmap_noreplace( + START, + BYTES_IN_PAGE, + MmapStrategy::TEST, + mmap_anno_test!() + ) + .is_ok()); + panic_if_unmapped(START, BYTES_IN_PAGE, mmap_anno_test!()); }, || { assert!(munmap(START, BYTES_IN_PAGE).is_ok()); @@ -479,10 +569,16 @@ mod tests { with_cleanup( || { // map 1 page from START - assert!(dzmmap_noreplace(START, BYTES_IN_PAGE, MmapStrategy::TEST).is_ok()); + assert!(dzmmap_noreplace( + START, + BYTES_IN_PAGE, + MmapStrategy::TEST, + mmap_anno_test!(), + ) + .is_ok()); // check if the next page is mapped - which should panic - panic_if_unmapped(START + BYTES_IN_PAGE, BYTES_IN_PAGE); + panic_if_unmapped(START + BYTES_IN_PAGE, BYTES_IN_PAGE, mmap_anno_test!()); }, || { assert!(munmap(START, BYTES_IN_PAGE * 2).is_ok()); @@ -501,10 +597,16 @@ mod tests { with_cleanup( || { // map 1 page from START - assert!(dzmmap_noreplace(START, BYTES_IN_PAGE, MmapStrategy::TEST).is_ok()); + assert!(dzmmap_noreplace( + START, + BYTES_IN_PAGE, + MmapStrategy::TEST, + mmap_anno_test!() + ) + .is_ok()); // check if the 2 pages from START are mapped. The second page is unmapped, so it should panic. - panic_if_unmapped(START, BYTES_IN_PAGE * 2); + panic_if_unmapped(START, BYTES_IN_PAGE * 2, mmap_anno_test!()); }, || { assert!(munmap(START, BYTES_IN_PAGE * 2).is_ok()); diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index 2a64b33848..29fca83037 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -2,7 +2,7 @@ use super::*; use crate::util::constants::{BYTES_IN_PAGE, BYTES_IN_WORD, LOG_BITS_IN_BYTE}; use crate::util::conversions::raw_align_up; use crate::util::heap::layout::vm_layout::BYTES_IN_CHUNK; -use crate::util::memory; +use crate::util::memory::{self, MmapAnno}; use crate::util::metadata::metadata_val_traits::*; #[cfg(feature = "vo_bit")] use crate::util::metadata::vo_bit::VO_BIT_SIDE_METADATA_SPEC; @@ -122,7 +122,13 @@ impl SideMetadataSpec { meta_start ); - memory::panic_if_unmapped(meta_start, BYTES_IN_PAGE); + memory::panic_if_unmapped( + meta_start, + BYTES_IN_PAGE, + &MmapAnno::Misc { + name: "assert_metadata_mapped", + }, + ); } /// Used only for debugging. @@ -1371,7 +1377,12 @@ impl SideMetadataContext { /// Tries to map the required metadata space and returns `true` is successful. /// This can be called at page granularity. - pub fn try_map_metadata_space(&self, start: Address, size: usize) -> Result<()> { + pub fn try_map_metadata_space( + &self, + start: Address, + size: usize, + space_name: &str, + ) -> Result<()> { debug!( "try_map_metadata_space({}, 0x{:x}, {}, {})", start, @@ -1382,14 +1393,19 @@ impl SideMetadataContext { // Page aligned debug_assert!(start.is_aligned_to(BYTES_IN_PAGE)); debug_assert!(size % BYTES_IN_PAGE == 0); - self.map_metadata_internal(start, size, false) + self.map_metadata_internal(start, size, false, space_name) } /// Tries to map the required metadata address range, without reserving swap-space/physical memory for it. /// This will make sure the address range is exclusive to the caller. This should be called at chunk granularity. /// /// NOTE: Accessing addresses in this range will produce a segmentation fault if swap-space is not mapped using the `try_map_metadata_space` function. - pub fn try_map_metadata_address_range(&self, start: Address, size: usize) -> Result<()> { + pub fn try_map_metadata_address_range( + &self, + start: Address, + size: usize, + name: &str, + ) -> Result<()> { debug!( "try_map_metadata_address_range({}, 0x{:x}, {}, {})", start, @@ -1400,7 +1416,7 @@ impl SideMetadataContext { // Chunk aligned debug_assert!(start.is_aligned_to(BYTES_IN_CHUNK)); debug_assert!(size % BYTES_IN_CHUNK == 0); - self.map_metadata_internal(start, size, true) + self.map_metadata_internal(start, size, true, name) } /// The internal function to mmap metadata @@ -1409,9 +1425,19 @@ impl SideMetadataContext { /// * `start` - The starting address of the source data. /// * `size` - The size of the source data (in bytes). /// * `no_reserve` - whether to invoke mmap with a noreserve flag (we use this flag to quarantine address range) - fn map_metadata_internal(&self, start: Address, size: usize, no_reserve: bool) -> Result<()> { + fn map_metadata_internal( + &self, + start: Address, + size: usize, + no_reserve: bool, + space_name: &str, + ) -> Result<()> { for spec in self.global.iter() { - match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve) { + let anno = MmapAnno::SideMeta { + space: space_name, + meta: spec.name, + }; + match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno) { Ok(_) => {} Err(e) => return Result::Err(e), } @@ -1434,7 +1460,11 @@ impl SideMetadataContext { // address space size as the current not-chunked approach. #[cfg(target_pointer_width = "64")] { - match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve) { + let anno = MmapAnno::SideMeta { + space: space_name, + meta: spec.name, + }; + match try_mmap_contiguous_metadata_space(start, size, spec, no_reserve, &anno) { Ok(_) => {} Err(e) => return Result::Err(e), } @@ -1556,6 +1586,7 @@ impl MetadataByteArrayRef { #[cfg(test)] mod tests { use super::*; + use crate::mmap_anno_test; use crate::util::metadata::side_metadata::SideMetadataContext; // offset is not used in these tests. @@ -1636,12 +1667,13 @@ mod tests { let data_addr = vm_layout::vm_layout().heap_start; // Make sure the address is mapped. crate::MMAPPER - .ensure_mapped(data_addr, 1, MmapStrategy::TEST) + .ensure_mapped(data_addr, 1, MmapStrategy::TEST, mmap_anno_test!()) .unwrap(); let meta_addr = address_to_meta_address(&spec, data_addr); with_cleanup( || { - let mmap_result = context.try_map_metadata_space(data_addr, BYTES_IN_PAGE); + let mmap_result = + context.try_map_metadata_space(data_addr, BYTES_IN_PAGE, "test_space"); assert!(mmap_result.is_ok()); f(&spec, data_addr, meta_addr); diff --git a/src/util/metadata/side_metadata/helpers.rs b/src/util/metadata/side_metadata/helpers.rs index 0f0a442d9a..b187af38ab 100644 --- a/src/util/metadata/side_metadata/helpers.rs +++ b/src/util/metadata/side_metadata/helpers.rs @@ -3,7 +3,7 @@ use super::SideMetadataSpec; use crate::util::constants::LOG_BYTES_IN_PAGE; use crate::util::constants::{BITS_IN_WORD, BYTES_IN_PAGE, LOG_BITS_IN_BYTE}; use crate::util::heap::layout::vm_layout::VMLayout; -use crate::util::memory::MmapStrategy; +use crate::util::memory::{MmapAnno, MmapStrategy}; #[cfg(target_pointer_width = "32")] use crate::util::metadata::side_metadata::address_to_chunked_meta_address; use crate::util::Address; @@ -126,6 +126,7 @@ pub(super) fn try_mmap_contiguous_metadata_space( size: usize, spec: &SideMetadataSpec, no_reserve: bool, + anno: &MmapAnno, ) -> Result { debug_assert!(start.is_aligned_to(BYTES_IN_PAGE)); debug_assert!(size % BYTES_IN_PAGE == 0); @@ -142,12 +143,14 @@ pub(super) fn try_mmap_contiguous_metadata_space( mmap_start, mmap_size >> LOG_BYTES_IN_PAGE, MmapStrategy::SIDE_METADATA, + anno, ) } else { MMAPPER.quarantine_address_range( mmap_start, mmap_size >> LOG_BYTES_IN_PAGE, MmapStrategy::SIDE_METADATA, + anno, ) } .map(|_| mmap_size) diff --git a/src/util/metadata/side_metadata/side_metadata_tests.rs b/src/util/metadata/side_metadata/side_metadata_tests.rs index ab1abe01d3..890ab1fdd3 100644 --- a/src/util/metadata/side_metadata/side_metadata_tests.rs +++ b/src/util/metadata/side_metadata/side_metadata_tests.rs @@ -230,7 +230,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(heap_start, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(heap_start, constants::BYTES_IN_PAGE, "test_space") .is_ok()); gspec.assert_metadata_mapped(heap_start); @@ -259,6 +259,7 @@ mod tests { .try_map_metadata_space( heap_start + vm_layout::BYTES_IN_CHUNK, vm_layout::BYTES_IN_CHUNK, + "test_space", ) .is_ok()); @@ -313,7 +314,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); let zero = @@ -380,7 +381,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); let zero = @@ -437,7 +438,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); let zero = @@ -519,7 +520,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); let zero = @@ -587,7 +588,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); // First 9 regions @@ -644,7 +645,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); assert!(metadata - .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE,) + .try_map_metadata_space(data_addr, constants::BYTES_IN_PAGE, "test_space",) .is_ok()); // First 9 regions @@ -753,7 +754,7 @@ mod tests { metadata_sanity.verify_metadata_context("NoPolicy", &metadata); metadata - .try_map_metadata_space(data_addr, total_size) + .try_map_metadata_space(data_addr, total_size, "test_space") .unwrap(); metadata_1_spec.bzero_metadata(data_addr, total_size); diff --git a/src/util/raw_memory_freelist.rs b/src/util/raw_memory_freelist.rs index 0b496ab47e..d7ff144dcd 100644 --- a/src/util/raw_memory_freelist.rs +++ b/src/util/raw_memory_freelist.rs @@ -3,6 +3,7 @@ use super::memory::MmapStrategy; use crate::util::address::Address; use crate::util::constants::*; use crate::util::conversions; +use crate::util::memory::MmapAnno; /** log2 of the number of bits used by a free list entry (two entries per unit) */ const LOG_ENTRY_BITS: usize = LOG_BITS_IN_INT as _; @@ -198,7 +199,14 @@ impl RawMemoryFreeList { } fn mmap(&self, start: Address, bytes: usize) { - let res = super::memory::dzmmap_noreplace(start, bytes, self.strategy); + let res = super::memory::dzmmap_noreplace( + start, + bytes, + self.strategy, + &MmapAnno::Misc { + name: "RawMemoryFreeList", + }, + ); assert!(res.is_ok(), "Can't get more space with mmap()"); } pub fn get_limit(&self) -> Address { From a453e326ca4dd2f4a4d121b19c57b83e7cee488d Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 14:39:30 +0800 Subject: [PATCH 02/24] Guard the annotation behind an option. --- src/mmtk.rs | 3 +++ src/util/memory.rs | 41 ++++++++++++++++++++++++++--------------- src/util/options.rs | 6 +++++- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/mmtk.rs b/src/mmtk.rs index d636e2472a..71be6fc19e 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -135,6 +135,9 @@ unsafe impl Send for MMTK {} impl MMTK { /// Create an MMTK instance. This is not public. Bindings should use [`MMTKBuilder::build`]. pub(crate) fn new(options: Arc) -> Self { + // Set the mmap annotation option now. SFT may map memory. + crate::util::memory::MMAP_ANNO.store(*options.mmap_anno, Ordering::SeqCst); + // Initialize SFT first in case we need to use this in the constructor. // The first call will initialize SFT map. Other calls will be blocked until SFT map is initialized. crate::policy::sft_map::SFTRefStorage::pre_use_check(); diff --git a/src/util/memory.rs b/src/util/memory.rs index 9383876d52..c71e52c4aa 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -6,6 +6,7 @@ use bytemuck::NoUninit; use libc::{PROT_EXEC, PROT_NONE, PROT_READ, PROT_WRITE}; use std::ffi::CString; use std::io::{Error, Result}; +use std::sync::atomic::AtomicBool; use sysinfo::MemoryRefreshKind; use sysinfo::{RefreshKind, System}; @@ -16,6 +17,14 @@ const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_F // MAP_FIXED is used instead of MAP_FIXED_NOREPLACE (which is not available on macOS). We are at the risk of overwriting pre-existing mappings. const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; +/// This static variable controls whether we annotate mmapped memory region using `prctl`. It can be +/// set via `Options::mmap_anno` or the `MMTK_MMAP_ANNO` environment variable. +/// +/// FIXME: Since it is set via `Options`, it is in theory a decision per MMTk instance. However, we +/// currently don't have a good design for multiple MMTk instances, so we use static variable for +/// now. +pub(crate) static MMAP_ANNO: AtomicBool = AtomicBool::new(true); + /// Strategy for performing mmap #[derive(Debug, Copy, Clone)] pub struct MmapStrategy { @@ -211,21 +220,23 @@ fn mmap_fixed( &|| unsafe { libc::mmap(start.to_mut_ptr(), size, prot, flags, -1, 0) }, ptr, )?; - let anno_str = anno.to_string(); - let anno_cstr = CString::new(anno_str).unwrap(); - wrap_libc_call( - &|| unsafe { - libc::prctl( - libc::PR_SET_VMA, - libc::PR_SET_VMA_ANON_NAME, - start.to_ptr::(), - size, - anno_cstr.as_ptr(), - ) - }, - 0, - ) - .unwrap(); + if MMAP_ANNO.load(std::sync::atomic::Ordering::SeqCst) { + let anno_str = anno.to_string(); + let anno_cstr = CString::new(anno_str).unwrap(); + wrap_libc_call( + &|| unsafe { + libc::prctl( + libc::PR_SET_VMA, + libc::PR_SET_VMA_ANON_NAME, + start.to_ptr::(), + size, + anno_cstr.as_ptr(), + ) + }, + 0, + ) + .unwrap(); + } match strategy.huge_page { HugePageSupport::No => Ok(()), HugePageSupport::TransparentHugePages => { diff --git a/src/util/options.rs b/src/util/options.rs index 49eab795e0..23c188c43e 100644 --- a/src/util/options.rs +++ b/src/util/options.rs @@ -864,7 +864,11 @@ options! { gc_trigger: GCTriggerSelector [env_var: true, command_line: true] [|v: &GCTriggerSelector| v.validate()] = GCTriggerSelector::FixedHeapSize((crate::util::memory::get_system_total_memory() as f64 * 0.5f64) as usize), /// Enable transparent hugepage support for MMTk spaces via madvise (only Linux is supported) /// This only affects the memory for MMTk spaces. - transparent_hugepages: bool [env_var: true, command_line: true] [|v: &bool| !v || cfg!(target_os = "linux")] = false + transparent_hugepages: bool [env_var: true, command_line: true] [|v: &bool| !v || cfg!(target_os = "linux")] = false, + /// Enable mmap annotation. By default, every time MMTk core calls `mmap`, it will annotate the + /// mapped memory range with human-readable names, which can be seen in `/proc/pid/maps`. + /// Setting this option to `false` will disable this annotation. + mmap_anno: bool [env_var: true, command_line: true] [always_valid] = true } #[cfg(test)] From 29724744ffb46aa280e95ae8e709f4751c00cd24 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 15:49:10 +0800 Subject: [PATCH 03/24] Make annotation linux-only --- src/util/memory.rs | 80 +++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index c71e52c4aa..d8d2c28d17 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -4,7 +4,6 @@ use crate::util::Address; use crate::vm::{Collection, VMBinding}; use bytemuck::NoUninit; use libc::{PROT_EXEC, PROT_NONE, PROT_READ, PROT_WRITE}; -use std::ffi::CString; use std::io::{Error, Result}; use std::sync::atomic::AtomicBool; use sysinfo::MemoryRefreshKind; @@ -212,7 +211,7 @@ fn mmap_fixed( size: usize, flags: libc::c_int, strategy: MmapStrategy, - anno: &MmapAnno, + _anno: &MmapAnno, ) -> Result<()> { let ptr = start.to_mut_ptr(); let prot = strategy.prot.into_native_flags(); @@ -220,10 +219,18 @@ fn mmap_fixed( &|| unsafe { libc::mmap(start.to_mut_ptr(), size, prot, flags, -1, 0) }, ptr, )?; + + #[cfg(target_os = "linux")] if MMAP_ANNO.load(std::sync::atomic::Ordering::SeqCst) { - let anno_str = anno.to_string(); - let anno_cstr = CString::new(anno_str).unwrap(); - wrap_libc_call( + // `PR_SET_VMA` is new in Linux 5.17. We compile against a version of the `libc` crate that + // has the `PR_SET_VMA_ANON_NAME` constant. When runnning on an older kernel, it will not + // recognize this attribute and will return `EINVAL`. However, `prctl` may return `EINVAL` + // for other reasons, too. That includes `start` being an invalid address, and the + // formatted `anno_cstr` being longer than 80 bytes including the trailing `'\0'`. But + // since this prctl is mainly used for debugging, we log the error instead of panicking. + let anno_str = _anno.to_string(); + let anno_cstr = std::ffi::CString::new(anno_str).unwrap(); + let result = wrap_libc_call( &|| unsafe { libc::prctl( libc::PR_SET_VMA, @@ -234,9 +241,12 @@ fn mmap_fixed( ) }, 0, - ) - .unwrap(); + ); + if let Err(e) = result { + debug!("Error while calling prctl: {e}"); + } } + match strategy.huge_page { HugePageSupport::No => Ok(()), HugePageSupport::TransparentHugePages => { @@ -304,40 +314,36 @@ pub fn handle_mmap_error( } /// Checks if the memory has already been mapped. If not, we panic. +/// /// Note that the checking has a side effect that it will map the memory if it was unmapped. So we panic if it was unmapped. /// Be very careful about using this function. -#[cfg(target_os = "linux")] -pub(crate) fn panic_if_unmapped(start: Address, size: usize, anno: &MmapAnno) { - let flags = MMAP_FLAGS; - match mmap_fixed( - start, - size, - flags, - MmapStrategy { - huge_page: HugePageSupport::No, - prot: MmapProtection::ReadWrite, - }, - anno, - ) { - Ok(_) => panic!("{} of size {} is not mapped", start, size), - Err(e) => { - assert!( - e.kind() == std::io::ErrorKind::AlreadyExists, - "Failed to check mapped: {:?}", - e - ); - } - } -} - -/// Checks if the memory has already been mapped. If not, we panic. +/// /// This function is currently left empty for non-linux, and should be implemented in the future. /// As the function is only used for assertions, MMTk will still run even if we never panic. -#[cfg(not(target_os = "linux"))] -pub(crate) fn panic_if_unmapped(_start: Address, _size: usize) { - // This is only used for assertions, so MMTk will still run even if we never panic. - // TODO: We need a proper implementation for this. As we do not have MAP_FIXED_NOREPLACE, we cannot use the same implementation as Linux. - // Possibly we can use posix_mem_offset for both OS/s. +pub(crate) fn panic_if_unmapped(_start: Address, _size: usize, _anno: &MmapAnno) { + #[cfg(target_os = "linux")] + { + let flags = MMAP_FLAGS; + match mmap_fixed( + _start, + _size, + flags, + MmapStrategy { + huge_page: HugePageSupport::No, + prot: MmapProtection::ReadWrite, + }, + _anno, + ) { + Ok(_) => panic!("{} of size {} is not mapped", _start, _size), + Err(e) => { + assert!( + e.kind() == std::io::ErrorKind::AlreadyExists, + "Failed to check mapped: {:?}", + e + ); + } + } + } } /// Unprotect the given memory (in page granularity) to allow access (PROT_READ/WRITE/EXEC). From a4187002e6d73e392792870e53bd3d0a53342230 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 16:22:23 +0800 Subject: [PATCH 04/24] Update comments --- src/mmtk.rs | 2 +- src/util/memory.rs | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/mmtk.rs b/src/mmtk.rs index 71be6fc19e..2523c18be0 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -135,7 +135,7 @@ unsafe impl Send for MMTK {} impl MMTK { /// Create an MMTK instance. This is not public. Bindings should use [`MMTKBuilder::build`]. pub(crate) fn new(options: Arc) -> Self { - // Set the mmap annotation option now. SFT may map memory. + // Set the static variable for mmap annotation. crate::util::memory::MMAP_ANNO.store(*options.mmap_anno, Ordering::SeqCst); // Initialize SFT first in case we need to use this in the constructor. diff --git a/src/util/memory.rs b/src/util/memory.rs index d8d2c28d17..57551bb2db 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -95,8 +95,18 @@ pub enum HugePageSupport { /// Annotation for an mmap entry. /// -/// This is mainly for debugging purpose. On Linux, mmtk-core will use `prctl` with `PR_SET_VMA` -/// to set the human-readable name for the given mmap region. +/// Invocations of [`mmap_fixed`] and other functions that may transitively call [`mmap_fixed`] +/// require an annotation that indicates the purpose of the memory mapping. +/// +/// This is for debugging. On Linux, mmtk-core will use `prctl` with `PR_SET_VMA` to set the +/// human-readable name for the given mmap region. The annotation is ignored on other platforms. +/// +/// Note that when using `Map32`, the discontiguous memory range is shared between different spaces. +/// Spaces may use `mmap` to map new chunks, but the same chunk may later be reused by other spaces. +/// The annotation only applies when `mmap` is called for a chunk for the first time, which reflects +/// which space first attempted the mmap, not which space is currently using the chunk. Use +/// [`crate::policy::space::print_vm_map`] to print a more accurate mapping between address ranges +/// and spaces. pub enum MmapAnno<'a> { Space { name: &'a str }, SideMeta { space: &'a str, meta: &'a str }, @@ -227,7 +237,7 @@ fn mmap_fixed( // recognize this attribute and will return `EINVAL`. However, `prctl` may return `EINVAL` // for other reasons, too. That includes `start` being an invalid address, and the // formatted `anno_cstr` being longer than 80 bytes including the trailing `'\0'`. But - // since this prctl is mainly used for debugging, we log the error instead of panicking. + // since this prctl is used for debugging, we log the error instead of panicking. let anno_str = _anno.to_string(); let anno_cstr = std::ffi::CString::new(anno_str).unwrap(); let result = wrap_libc_call( From 94cd2606d36eb4b815670e0ca45805182d10d576 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 16:34:18 +0800 Subject: [PATCH 05/24] Fix helpers_32 --- src/util/metadata/side_metadata/global.rs | 6 +++++- src/util/metadata/side_metadata/helpers_32.rs | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index 29fca83037..e0704bb4f1 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1484,7 +1484,11 @@ impl SideMetadataContext { lsize, max ); - match try_map_per_chunk_metadata_space(start, size, lsize, no_reserve) { + let anno = MmapAnno::SideMeta { + space: space_name, + meta: "all", + }; + match try_map_per_chunk_metadata_space(start, size, lsize, no_reserve, &anno) { Ok(_) => {} Err(e) => return Result::Err(e), } diff --git a/src/util/metadata/side_metadata/helpers_32.rs b/src/util/metadata/side_metadata/helpers_32.rs index ef413c0cc6..060ed11001 100644 --- a/src/util/metadata/side_metadata/helpers_32.rs +++ b/src/util/metadata/side_metadata/helpers_32.rs @@ -2,7 +2,8 @@ use super::SideMetadataSpec; use crate::util::{ constants::{self, LOG_BITS_IN_BYTE}, heap::layout::vm_layout::{BYTES_IN_CHUNK, CHUNK_MASK, LOG_BYTES_IN_CHUNK}, - memory, Address, + memory::{self, MmapAnno}, + Address, }; use std::io::Result; @@ -111,6 +112,7 @@ pub(super) fn try_map_per_chunk_metadata_space( size: usize, local_per_chunk: usize, no_reserve: bool, + anno: &MmapAnno, ) -> Result { let mut aligned_start = start.align_down(BYTES_IN_CHUNK); let aligned_end = (start + size).align_up(BYTES_IN_CHUNK); @@ -121,7 +123,7 @@ pub(super) fn try_map_per_chunk_metadata_space( let mut total_mapped = 0; while aligned_start < aligned_end { - let res = try_mmap_metadata_chunk(aligned_start, local_per_chunk, no_reserve); + let res = try_mmap_metadata_chunk(aligned_start, local_per_chunk, no_reserve, anno); if res.is_err() { if munmap_first_chunk.is_some() { let mut munmap_start = if munmap_first_chunk.unwrap() { @@ -174,6 +176,7 @@ pub(super) fn try_mmap_metadata_chunk( start: Address, local_per_chunk: usize, no_reserve: bool, + anno: &MmapAnno, ) -> Result<()> { debug_assert!(start.is_aligned_to(BYTES_IN_CHUNK)); @@ -185,12 +188,14 @@ pub(super) fn try_mmap_metadata_chunk( policy_meta_start, pages, memory::MmapStrategy::SIDE_METADATA, + anno, ) } else { MMAPPER.quarantine_address_range( policy_meta_start, pages, memory::MmapStrategy::SIDE_METADATA, + anno, ) } } From 3dfad91e38578b8e55f969c29eac9dc3d9dd590a Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 16:37:12 +0800 Subject: [PATCH 06/24] Fix vmspace --- src/policy/vmspace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/policy/vmspace.rs b/src/policy/vmspace.rs index 60199c0fde..770c362cfd 100644 --- a/src/policy/vmspace.rs +++ b/src/policy/vmspace.rs @@ -229,7 +229,7 @@ impl VMSpace { // Map side metadata self.common .metadata - .try_map_metadata_space(chunk_start, chunk_size) + .try_map_metadata_space(chunk_start, chunk_size, self.get_name()) .unwrap(); // Insert to vm map: it would be good if we can make VM map aware of the region. However, the region may be outside what we can map in our VM map implementation. // self.common.vm_map.insert(chunk_start, chunk_size, self.common.descriptor); From 12ce6ae5d93240daa77431e9da33c216b0e4f1c9 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 16:55:40 +0800 Subject: [PATCH 07/24] Fix space names --- src/policy/lockfreeimmortalspace.rs | 15 +++++++------ src/policy/marksweepspace/malloc_ms/global.rs | 2 +- .../marksweepspace/malloc_ms/metadata.rs | 21 ++++++++++++------- src/policy/sft_map.rs | 2 +- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index d9ef34ae26..a8dbe86747 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -247,18 +247,17 @@ impl LockFreeImmortalSpace { aligned_total_bytes, strategy, &MmapAnno::Space { - name: "LockFreeImmortalSpace", + name: space.get_name(), }, ) .unwrap(); - if space + space .metadata - .try_map_metadata_space(start, aligned_total_bytes, "mmtk:LockFreeImmortalSpace") - .is_err() - { - // TODO(Javad): handle meta space allocation failure - panic!("failed to mmap meta memory"); - } + .try_map_metadata_space(start, aligned_total_bytes, space.get_name()) + .unwrap_or_else(|e| { + // TODO(Javad): handle meta space allocation failure + panic!("failed to mmap meta memory: {e}") + }); space } diff --git a/src/policy/marksweepspace/malloc_ms/global.rs b/src/policy/marksweepspace/malloc_ms/global.rs index d6a3124a1a..9424a5069a 100644 --- a/src/policy/marksweepspace/malloc_ms/global.rs +++ b/src/policy/marksweepspace/malloc_ms/global.rs @@ -439,7 +439,7 @@ impl MallocSpace { fn map_metadata_and_update_bound(&self, addr: Address, size: usize) { // Map the metadata space for the range [addr, addr + size) - map_meta_space(&self.metadata, addr, size); + map_meta_space(&self.metadata, addr, size, self.get_name()); // Update the bounds of the max and min chunk addresses seen -- this is used later in the sweep // Lockless compare-and-swap loops perform better than a locking variant diff --git a/src/policy/marksweepspace/malloc_ms/metadata.rs b/src/policy/marksweepspace/malloc_ms/metadata.rs index faf779bb27..0bb208e9cd 100644 --- a/src/policy/marksweepspace/malloc_ms/metadata.rs +++ b/src/policy/marksweepspace/malloc_ms/metadata.rs @@ -76,7 +76,7 @@ fn is_meta_space_mapped_for_address(address: Address) -> bool { } /// Eagerly map the active chunk metadata surrounding `chunk_start` -fn map_active_chunk_metadata(chunk_start: Address) { +fn map_active_chunk_metadata(chunk_start: Address, space_name: &str) { debug_assert!(chunk_start.is_aligned_to(BYTES_IN_CHUNK)); // We eagerly map 16Gb worth of space for the chunk mark bytes on 64-bits // We require saturating subtractions in order to not overflow the chunk_start by @@ -99,16 +99,20 @@ fn map_active_chunk_metadata(chunk_start: Address) { chunk_start + (size / 2) ); - assert!( - CHUNK_METADATA.try_map_metadata_space(start, size, "mmtk:map_active_chunk_metadata:assert").is_ok(), - "failed to mmap meta memory" - ); + CHUNK_METADATA + .try_map_metadata_space(start, size, space_name) + .unwrap_or_else(|e| panic!("failed to mmap meta memory: {e}")); } /// We map the active chunk metadata (if not previously mapped), as well as the VO bit metadata /// and active page metadata here. Note that if [addr, addr + size) crosses multiple chunks, we /// will map for each chunk. -pub(super) fn map_meta_space(metadata: &SideMetadataContext, addr: Address, size: usize) { +pub(super) fn map_meta_space( + metadata: &SideMetadataContext, + addr: Address, + size: usize, + space_name: &str, +) { // In order to prevent race conditions, we synchronize on the lock first and then // check if we need to map the active chunk metadata for `chunk_start` let _lock = CHUNK_MAP_LOCK.lock().unwrap(); @@ -118,7 +122,7 @@ pub(super) fn map_meta_space(metadata: &SideMetadataContext, addr: Address, size // Check if the chunk bit metadata is mapped. If it is not mapped, map it. // Note that the chunk bit metadata is global. It may have been mapped because other policy mapped it. if !is_chunk_mapped(start) { - map_active_chunk_metadata(start); + map_active_chunk_metadata(start, space_name); } // If we have set the chunk bit, return. This is needed just in case another thread has done this before @@ -131,7 +135,8 @@ pub(super) fn map_meta_space(metadata: &SideMetadataContext, addr: Address, size // Note that this might fail. For example, we have marked a chunk as active but later we freed all // the objects in it, and unset its chunk bit. However, we do not free its metadata. So for the chunk, // its chunk bit is mapped, but not marked, and all its local metadata is also mapped. - let mmap_metadata_result = metadata.try_map_metadata_space(start, BYTES_IN_CHUNK, "mmtk:malloc_ms:map_meta_space"); + let mmap_metadata_result = + metadata.try_map_metadata_space(start, BYTES_IN_CHUNK, space_name); debug_assert!( mmap_metadata_result.is_ok(), "mmap sidemetadata failed for chunk_start ({})", diff --git a/src/policy/sft_map.rs b/src/policy/sft_map.rs index db1862715b..62f2b6e523 100644 --- a/src/policy/sft_map.rs +++ b/src/policy/sft_map.rs @@ -378,7 +378,7 @@ mod dense_chunk_map { global: vec![SFT_DENSE_CHUNK_MAP_INDEX], local: vec![], }; - if context.try_map_metadata_space(start, bytes, "mmtk:SFTDenseChunkMap").is_err() { + if context.try_map_metadata_space(start, bytes, "SFTDenseChunkMap").is_err() { panic!("failed to mmap metadata memory"); } From 851444cb716b74a97ec1d6ac3798faab50c1eb9b Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 17:00:51 +0800 Subject: [PATCH 08/24] Fix error handling --- src/policy/sft_map.rs | 8 +++++--- src/policy/space.rs | 24 ++++++++++-------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/policy/sft_map.rs b/src/policy/sft_map.rs index 62f2b6e523..c9db21830e 100644 --- a/src/policy/sft_map.rs +++ b/src/policy/sft_map.rs @@ -378,9 +378,11 @@ mod dense_chunk_map { global: vec![SFT_DENSE_CHUNK_MAP_INDEX], local: vec![], }; - if context.try_map_metadata_space(start, bytes, "SFTDenseChunkMap").is_err() { - panic!("failed to mmap metadata memory"); - } + context + .try_map_metadata_space(start, bytes, "SFTDenseChunkMap") + .unwrap_or_else(|e| { + panic!("failed to mmap metadata memory: {e}"); + }); self.update(space, start, bytes); } diff --git a/src/policy/space.rs b/src/policy/space.rs index e125a7bc5e..e7aae53d19 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -300,15 +300,13 @@ pub trait Space: 'static + SFT + Sync + Downcast { /// Ensure this space is marked as mapped -- used when the space is already /// mapped (e.g. for a vm image which is externally mmapped.) fn ensure_mapped(&self) { - if self - .common() + self.common() .metadata .try_map_metadata_space(self.common().start, self.common().extent, self.name()) - .is_err() - { - // TODO(Javad): handle meta space allocation failure - panic!("failed to mmap meta memory"); - } + .unwrap_or_else(|e| { + // TODO(Javad): handle meta space allocation failure + panic!("failed to mmap meta memory: {e}"); + }); self.common() .mmapper @@ -616,14 +614,12 @@ impl CommonSpace { } // For contiguous space, we know its address range so we reserve metadata memory for its range. - if rtn - .metadata + rtn.metadata .try_map_metadata_address_range(rtn.start, rtn.extent, rtn.name) - .is_err() - { - // TODO(Javad): handle meta space allocation failure - panic!("failed to mmap meta memory"); - } + .unwrap_or_else(|e| { + // TODO(Javad): handle meta space allocation failure + panic!("failed to mmap meta memory: {e}"); + }); debug!( "Created space {} [{}, {}) for {} bytes", From b1967092c80b8958416541469589957af2f3739d Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 20:08:55 +0800 Subject: [PATCH 09/24] Update comment --- src/util/memory.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index 57551bb2db..fdf09aff7f 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -101,12 +101,16 @@ pub enum HugePageSupport { /// This is for debugging. On Linux, mmtk-core will use `prctl` with `PR_SET_VMA` to set the /// human-readable name for the given mmap region. The annotation is ignored on other platforms. /// -/// Note that when using `Map32`, the discontiguous memory range is shared between different spaces. -/// Spaces may use `mmap` to map new chunks, but the same chunk may later be reused by other spaces. -/// The annotation only applies when `mmap` is called for a chunk for the first time, which reflects -/// which space first attempted the mmap, not which space is currently using the chunk. Use -/// [`crate::policy::space::print_vm_map`] to print a more accurate mapping between address ranges -/// and spaces. +/// Note that when using `Map32` (even when running on 64-bit architectures), the discontiguous +/// memory range is shared between different spaces. Spaces may use `mmap` to map new chunks, but +/// the same chunk may later be reused by other spaces. The annotation only applies when `mmap` is +/// called for a chunk for the first time, which reflects which space first attempted the mmap, not +/// which space is currently using the chunk. Use [`crate::policy::space::print_vm_map`] to print a +/// more accurate mapping between address ranges and spaces. +/// +/// On 32-bit architecture, side metadata are allocated in a chunked fasion. One single `mmap` +/// region will contain many different metadata. In that case, we simply annotate the whole region +/// with a `MmapAnno::SideMeta` where `meta` is `"all"`. pub enum MmapAnno<'a> { Space { name: &'a str }, SideMeta { space: &'a str, meta: &'a str }, From de848c41490d524210b8ada2c5f34c59752c838f Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Mon, 25 Nov 2024 20:16:01 +0800 Subject: [PATCH 10/24] Rename MmapAnno to MmapAnnotation --- src/mmtk.rs | 2 +- src/policy/lockfreeimmortalspace.rs | 4 +-- src/policy/space.rs | 2 +- src/util/heap/layout/byte_map_mmapper.rs | 6 ++-- src/util/heap/layout/fragmented_mapper.rs | 6 ++-- src/util/heap/layout/mmapper.rs | 10 +++--- src/util/memory.rs | 32 +++++++++---------- src/util/metadata/side_metadata/global.rs | 10 +++--- src/util/metadata/side_metadata/helpers.rs | 4 +-- src/util/metadata/side_metadata/helpers_32.rs | 6 ++-- src/util/options.rs | 2 +- src/util/raw_memory_freelist.rs | 4 +-- 12 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/mmtk.rs b/src/mmtk.rs index 2523c18be0..6b48969290 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -136,7 +136,7 @@ impl MMTK { /// Create an MMTK instance. This is not public. Bindings should use [`MMTKBuilder::build`]. pub(crate) fn new(options: Arc) -> Self { // Set the static variable for mmap annotation. - crate::util::memory::MMAP_ANNO.store(*options.mmap_anno, Ordering::SeqCst); + crate::util::memory::MMAP_ANNOTATION.store(*options.mmap_annotation, Ordering::SeqCst); // Initialize SFT first in case we need to use this in the constructor. // The first call will initialize SFT map. Other calls will be blocked until SFT map is initialized. diff --git a/src/policy/lockfreeimmortalspace.rs b/src/policy/lockfreeimmortalspace.rs index a8dbe86747..6c6b842425 100644 --- a/src/policy/lockfreeimmortalspace.rs +++ b/src/policy/lockfreeimmortalspace.rs @@ -13,7 +13,7 @@ use crate::util::heap::gc_trigger::GCTrigger; use crate::util::heap::layout::vm_layout::vm_layout; use crate::util::heap::PageResource; use crate::util::heap::VMRequest; -use crate::util::memory::MmapAnno; +use crate::util::memory::MmapAnnotation; use crate::util::memory::MmapStrategy; use crate::util::metadata::side_metadata::SideMetadataContext; use crate::util::metadata::side_metadata::SideMetadataSanity; @@ -246,7 +246,7 @@ impl LockFreeImmortalSpace { start, aligned_total_bytes, strategy, - &MmapAnno::Space { + &MmapAnnotation::Space { name: space.get_name(), }, ) diff --git a/src/policy/space.rs b/src/policy/space.rs index e7aae53d19..1b5b5026c1 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -148,7 +148,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { res.start, res.pages, self.common().mmap_strategy(), - &memory::MmapAnno::Space { + &memory::MmapAnnotation::Space { name: self.get_name(), }, ) diff --git a/src/util/heap/layout/byte_map_mmapper.rs b/src/util/heap/layout/byte_map_mmapper.rs index 08205882d6..412e53ddcb 100644 --- a/src/util/heap/layout/byte_map_mmapper.rs +++ b/src/util/heap/layout/byte_map_mmapper.rs @@ -1,6 +1,6 @@ use super::mmapper::MapState; use super::Mmapper; -use crate::util::memory::MmapAnno; +use crate::util::memory::MmapAnnotation; use crate::util::Address; use crate::util::constants::*; @@ -50,7 +50,7 @@ impl Mmapper for ByteMapMmapper { start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { let start_chunk = Self::address_to_mmap_chunks_down(start); let end_chunk = Self::address_to_mmap_chunks_up(start + pages_to_bytes(pages)); @@ -81,7 +81,7 @@ impl Mmapper for ByteMapMmapper { start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { let start_chunk = Self::address_to_mmap_chunks_down(start); let end_chunk = Self::address_to_mmap_chunks_up(start + pages_to_bytes(pages)); diff --git a/src/util/heap/layout/fragmented_mapper.rs b/src/util/heap/layout/fragmented_mapper.rs index 29cdeedee1..0ebec07b83 100644 --- a/src/util/heap/layout/fragmented_mapper.rs +++ b/src/util/heap/layout/fragmented_mapper.rs @@ -3,7 +3,7 @@ use super::Mmapper; use crate::util::constants::BYTES_IN_PAGE; use crate::util::conversions; use crate::util::heap::layout::vm_layout::*; -use crate::util::memory::{MmapAnno, MmapStrategy}; +use crate::util::memory::{MmapAnnotation, MmapStrategy}; use crate::util::Address; use atomic::{Atomic, Ordering}; use std::cell::UnsafeCell; @@ -94,7 +94,7 @@ impl Mmapper for FragmentedMapper { mut start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { debug_assert!(start.is_aligned_to(BYTES_IN_PAGE)); @@ -153,7 +153,7 @@ impl Mmapper for FragmentedMapper { mut start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { let end = start + conversions::pages_to_bytes(pages); // Iterate over the slabs covered diff --git a/src/util/heap/layout/mmapper.rs b/src/util/heap/layout/mmapper.rs index 4a528a3298..7548f701fa 100644 --- a/src/util/heap/layout/mmapper.rs +++ b/src/util/heap/layout/mmapper.rs @@ -37,7 +37,7 @@ pub trait Mmapper: Sync { start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()>; /// Ensure that a range of pages is mmapped (or equivalent). If the @@ -55,7 +55,7 @@ pub trait Mmapper: Sync { start: Address, pages: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()>; /// Is the page pointed to by this address mapped? Returns true if @@ -95,7 +95,7 @@ impl MapState { state: &Atomic, mmap_start: Address, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { trace!( "Trying to map {} - {}", @@ -123,7 +123,7 @@ impl MapState { state: &Atomic, mmap_start: Address, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { trace!( "Trying to quarantine {} - {}", @@ -168,7 +168,7 @@ impl MapState { state_slices: &[&[Atomic]], mmap_start: Address, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { trace!( "Trying to bulk-quarantine {} - {}", diff --git a/src/util/memory.rs b/src/util/memory.rs index fdf09aff7f..bc2e6b8003 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -17,12 +17,12 @@ const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_F const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; /// This static variable controls whether we annotate mmapped memory region using `prctl`. It can be -/// set via `Options::mmap_anno` or the `MMTK_MMAP_ANNO` environment variable. +/// set via `Options::mmap_annotation` or the `MMTK_MMAP_ANNOTATION` environment variable. /// /// FIXME: Since it is set via `Options`, it is in theory a decision per MMTk instance. However, we /// currently don't have a good design for multiple MMTk instances, so we use static variable for /// now. -pub(crate) static MMAP_ANNO: AtomicBool = AtomicBool::new(true); +pub(crate) static MMAP_ANNOTATION: AtomicBool = AtomicBool::new(true); /// Strategy for performing mmap #[derive(Debug, Copy, Clone)] @@ -110,8 +110,8 @@ pub enum HugePageSupport { /// /// On 32-bit architecture, side metadata are allocated in a chunked fasion. One single `mmap` /// region will contain many different metadata. In that case, we simply annotate the whole region -/// with a `MmapAnno::SideMeta` where `meta` is `"all"`. -pub enum MmapAnno<'a> { +/// with a `MmapAnnotation::SideMeta` where `meta` is `"all"`. +pub enum MmapAnnotation<'a> { Space { name: &'a str }, SideMeta { space: &'a str, meta: &'a str }, Test { file: &'a str, line: u32 }, @@ -121,20 +121,20 @@ pub enum MmapAnno<'a> { #[macro_export] macro_rules! mmap_anno_test { () => { - &$crate::util::memory::MmapAnno::Test { + &$crate::util::memory::MmapAnnotation::Test { file: file!(), line: line!(), } }; } -impl<'a> std::fmt::Display for MmapAnno<'a> { +impl<'a> std::fmt::Display for MmapAnnotation<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - MmapAnno::Space { name } => write!(f, "mmtk:space:{name}"), - MmapAnno::SideMeta { space, meta } => write!(f, "mmtk:sidemeta:{space}:{meta}"), - MmapAnno::Test { file, line } => write!(f, "mmtk:test:{file}:{line}"), - MmapAnno::Misc { name } => write!(f, "mmtk:misc:{name}"), + MmapAnnotation::Space { name } => write!(f, "mmtk:space:{name}"), + MmapAnnotation::SideMeta { space, meta } => write!(f, "mmtk:sidemeta:{space}:{meta}"), + MmapAnnotation::Test { file, line } => write!(f, "mmtk:test:{file}:{line}"), + MmapAnnotation::Misc { name } => write!(f, "mmtk:misc:{name}"), } } } @@ -174,7 +174,7 @@ pub unsafe fn dzmmap( start: Address, size: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { let flags = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; let ret = mmap_fixed(start, size, flags, strategy, anno); @@ -193,7 +193,7 @@ pub fn dzmmap_noreplace( start: Address, size: usize, strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { let flags = MMAP_FLAGS; let ret = mmap_fixed(start, size, flags, strategy, anno); @@ -213,7 +213,7 @@ pub fn mmap_noreserve( start: Address, size: usize, mut strategy: MmapStrategy, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { strategy.prot = MmapProtection::NoAccess; let flags = MMAP_FLAGS | libc::MAP_NORESERVE; @@ -225,7 +225,7 @@ fn mmap_fixed( size: usize, flags: libc::c_int, strategy: MmapStrategy, - _anno: &MmapAnno, + _anno: &MmapAnnotation, ) -> Result<()> { let ptr = start.to_mut_ptr(); let prot = strategy.prot.into_native_flags(); @@ -235,7 +235,7 @@ fn mmap_fixed( )?; #[cfg(target_os = "linux")] - if MMAP_ANNO.load(std::sync::atomic::Ordering::SeqCst) { + if MMAP_ANNOTATION.load(std::sync::atomic::Ordering::SeqCst) { // `PR_SET_VMA` is new in Linux 5.17. We compile against a version of the `libc` crate that // has the `PR_SET_VMA_ANON_NAME` constant. When runnning on an older kernel, it will not // recognize this attribute and will return `EINVAL`. However, `prctl` may return `EINVAL` @@ -334,7 +334,7 @@ pub fn handle_mmap_error( /// /// This function is currently left empty for non-linux, and should be implemented in the future. /// As the function is only used for assertions, MMTk will still run even if we never panic. -pub(crate) fn panic_if_unmapped(_start: Address, _size: usize, _anno: &MmapAnno) { +pub(crate) fn panic_if_unmapped(_start: Address, _size: usize, _anno: &MmapAnnotation) { #[cfg(target_os = "linux")] { let flags = MMAP_FLAGS; diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index e0704bb4f1..f5aa9c6e49 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -2,7 +2,7 @@ use super::*; use crate::util::constants::{BYTES_IN_PAGE, BYTES_IN_WORD, LOG_BITS_IN_BYTE}; use crate::util::conversions::raw_align_up; use crate::util::heap::layout::vm_layout::BYTES_IN_CHUNK; -use crate::util::memory::{self, MmapAnno}; +use crate::util::memory::{self, MmapAnnotation}; use crate::util::metadata::metadata_val_traits::*; #[cfg(feature = "vo_bit")] use crate::util::metadata::vo_bit::VO_BIT_SIDE_METADATA_SPEC; @@ -125,7 +125,7 @@ impl SideMetadataSpec { memory::panic_if_unmapped( meta_start, BYTES_IN_PAGE, - &MmapAnno::Misc { + &MmapAnnotation::Misc { name: "assert_metadata_mapped", }, ); @@ -1433,7 +1433,7 @@ impl SideMetadataContext { space_name: &str, ) -> Result<()> { for spec in self.global.iter() { - let anno = MmapAnno::SideMeta { + let anno = MmapAnnotation::SideMeta { space: space_name, meta: spec.name, }; @@ -1460,7 +1460,7 @@ impl SideMetadataContext { // address space size as the current not-chunked approach. #[cfg(target_pointer_width = "64")] { - let anno = MmapAnno::SideMeta { + let anno = MmapAnnotation::SideMeta { space: space_name, meta: spec.name, }; @@ -1484,7 +1484,7 @@ impl SideMetadataContext { lsize, max ); - let anno = MmapAnno::SideMeta { + let anno = MmapAnnotation::SideMeta { space: space_name, meta: "all", }; diff --git a/src/util/metadata/side_metadata/helpers.rs b/src/util/metadata/side_metadata/helpers.rs index b187af38ab..c8dc979764 100644 --- a/src/util/metadata/side_metadata/helpers.rs +++ b/src/util/metadata/side_metadata/helpers.rs @@ -3,7 +3,7 @@ use super::SideMetadataSpec; use crate::util::constants::LOG_BYTES_IN_PAGE; use crate::util::constants::{BITS_IN_WORD, BYTES_IN_PAGE, LOG_BITS_IN_BYTE}; use crate::util::heap::layout::vm_layout::VMLayout; -use crate::util::memory::{MmapAnno, MmapStrategy}; +use crate::util::memory::{MmapAnnotation, MmapStrategy}; #[cfg(target_pointer_width = "32")] use crate::util::metadata::side_metadata::address_to_chunked_meta_address; use crate::util::Address; @@ -126,7 +126,7 @@ pub(super) fn try_mmap_contiguous_metadata_space( size: usize, spec: &SideMetadataSpec, no_reserve: bool, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result { debug_assert!(start.is_aligned_to(BYTES_IN_PAGE)); debug_assert!(size % BYTES_IN_PAGE == 0); diff --git a/src/util/metadata/side_metadata/helpers_32.rs b/src/util/metadata/side_metadata/helpers_32.rs index 060ed11001..1372d5916c 100644 --- a/src/util/metadata/side_metadata/helpers_32.rs +++ b/src/util/metadata/side_metadata/helpers_32.rs @@ -2,7 +2,7 @@ use super::SideMetadataSpec; use crate::util::{ constants::{self, LOG_BITS_IN_BYTE}, heap::layout::vm_layout::{BYTES_IN_CHUNK, CHUNK_MASK, LOG_BYTES_IN_CHUNK}, - memory::{self, MmapAnno}, + memory::{self, MmapAnnotation}, Address, }; use std::io::Result; @@ -112,7 +112,7 @@ pub(super) fn try_map_per_chunk_metadata_space( size: usize, local_per_chunk: usize, no_reserve: bool, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result { let mut aligned_start = start.align_down(BYTES_IN_CHUNK); let aligned_end = (start + size).align_up(BYTES_IN_CHUNK); @@ -176,7 +176,7 @@ pub(super) fn try_mmap_metadata_chunk( start: Address, local_per_chunk: usize, no_reserve: bool, - anno: &MmapAnno, + anno: &MmapAnnotation, ) -> Result<()> { debug_assert!(start.is_aligned_to(BYTES_IN_CHUNK)); diff --git a/src/util/options.rs b/src/util/options.rs index 23c188c43e..5a606accbd 100644 --- a/src/util/options.rs +++ b/src/util/options.rs @@ -868,7 +868,7 @@ options! { /// Enable mmap annotation. By default, every time MMTk core calls `mmap`, it will annotate the /// mapped memory range with human-readable names, which can be seen in `/proc/pid/maps`. /// Setting this option to `false` will disable this annotation. - mmap_anno: bool [env_var: true, command_line: true] [always_valid] = true + mmap_annotation: bool [env_var: true, command_line: true] [always_valid] = true } #[cfg(test)] diff --git a/src/util/raw_memory_freelist.rs b/src/util/raw_memory_freelist.rs index d7ff144dcd..23f6844ed7 100644 --- a/src/util/raw_memory_freelist.rs +++ b/src/util/raw_memory_freelist.rs @@ -3,7 +3,7 @@ use super::memory::MmapStrategy; use crate::util::address::Address; use crate::util::constants::*; use crate::util::conversions; -use crate::util::memory::MmapAnno; +use crate::util::memory::MmapAnnotation; /** log2 of the number of bits used by a free list entry (two entries per unit) */ const LOG_ENTRY_BITS: usize = LOG_BITS_IN_INT as _; @@ -203,7 +203,7 @@ impl RawMemoryFreeList { start, bytes, self.strategy, - &MmapAnno::Misc { + &MmapAnnotation::Misc { name: "RawMemoryFreeList", }, ); From 4a5e2561d8cdde80bb8aa57f139baec5d1f57bda Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 26 Nov 2024 12:11:22 +0800 Subject: [PATCH 11/24] Fix mock tests. --- src/util/memory.rs | 4 ++++ .../mock_tests/mock_test_handle_mmap_conflict.rs | 16 ++++++++++++---- .../mock_tests/mock_test_handle_mmap_oom.rs | 8 ++++++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index bc2e6b8003..0f7f68e76d 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -118,6 +118,7 @@ pub enum MmapAnnotation<'a> { Misc { name: &'a str }, } +/// Construct an `MmapAnnotation::Test` with the current file name and line number. #[macro_export] macro_rules! mmap_anno_test { () => { @@ -128,6 +129,9 @@ macro_rules! mmap_anno_test { }; } +// Export this to external crates +pub use mmap_anno_test; + impl<'a> std::fmt::Display for MmapAnnotation<'a> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/src/vm/tests/mock_tests/mock_test_handle_mmap_conflict.rs b/src/vm/tests/mock_tests/mock_test_handle_mmap_conflict.rs index b4cc7be194..1dc6a5759e 100644 --- a/src/vm/tests/mock_tests/mock_test_handle_mmap_conflict.rs +++ b/src/vm/tests/mock_tests/mock_test_handle_mmap_conflict.rs @@ -11,13 +11,21 @@ pub fn test_handle_mmap_conflict() { || { let start = unsafe { Address::from_usize(0x100_0000) }; let one_megabyte = 1000000; - let mmap1_res = - memory::dzmmap_noreplace(start, one_megabyte, memory::MmapStrategy::TEST); + let mmap1_res = memory::dzmmap_noreplace( + start, + one_megabyte, + memory::MmapStrategy::TEST, + memory::mmap_anno_test!(), + ); assert!(mmap1_res.is_ok()); let panic_res = std::panic::catch_unwind(|| { - let mmap2_res = - memory::dzmmap_noreplace(start, one_megabyte, memory::MmapStrategy::TEST); + let mmap2_res = memory::dzmmap_noreplace( + start, + one_megabyte, + memory::MmapStrategy::TEST, + memory::mmap_anno_test!(), + ); assert!(mmap2_res.is_err()); memory::handle_mmap_error::( mmap2_res.err().unwrap(), diff --git a/src/vm/tests/mock_tests/mock_test_handle_mmap_oom.rs b/src/vm/tests/mock_tests/mock_test_handle_mmap_oom.rs index 3e23db0fcb..b810c47a43 100644 --- a/src/vm/tests/mock_tests/mock_test_handle_mmap_oom.rs +++ b/src/vm/tests/mock_tests/mock_test_handle_mmap_oom.rs @@ -18,8 +18,12 @@ pub fn test_handle_mmap_oom() { let start = unsafe { Address::from_usize(0x100_0000) }; // mmap 1 terabyte memory - we expect this will fail due to out of memory. // If that's not the case, increase the size we mmap. - let mmap_res = - memory::dzmmap_noreplace(start, LARGE_SIZE, memory::MmapStrategy::TEST); + let mmap_res = memory::dzmmap_noreplace( + start, + LARGE_SIZE, + memory::MmapStrategy::TEST, + memory::mmap_anno_test!(), + ); memory::handle_mmap_error::( mmap_res.err().unwrap(), From 5344a0ba15305f809cb958ec6499ad00c078ce85 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 26 Nov 2024 12:28:04 +0800 Subject: [PATCH 12/24] Fix documentation --- src/util/memory.rs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index 0f7f68e76d..4387f98a59 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -95,7 +95,7 @@ pub enum HugePageSupport { /// Annotation for an mmap entry. /// -/// Invocations of [`mmap_fixed`] and other functions that may transitively call [`mmap_fixed`] +/// Invocations of `mmap_fixed` and other functions that may transitively call `mmap_fixed` /// require an annotation that indicates the purpose of the memory mapping. /// /// This is for debugging. On Linux, mmtk-core will use `prctl` with `PR_SET_VMA` to set the @@ -105,17 +105,37 @@ pub enum HugePageSupport { /// memory range is shared between different spaces. Spaces may use `mmap` to map new chunks, but /// the same chunk may later be reused by other spaces. The annotation only applies when `mmap` is /// called for a chunk for the first time, which reflects which space first attempted the mmap, not -/// which space is currently using the chunk. Use [`crate::policy::space::print_vm_map`] to print a +/// which space is currently using the chunk. Use `crate::policy::space::print_vm_map` to print a /// more accurate mapping between address ranges and spaces. /// /// On 32-bit architecture, side metadata are allocated in a chunked fasion. One single `mmap` /// region will contain many different metadata. In that case, we simply annotate the whole region /// with a `MmapAnnotation::SideMeta` where `meta` is `"all"`. pub enum MmapAnnotation<'a> { - Space { name: &'a str }, - SideMeta { space: &'a str, meta: &'a str }, - Test { file: &'a str, line: u32 }, - Misc { name: &'a str }, + /// The mmap is for a space. + Space { + /// The name of the space. + name: &'a str, + }, + /// The mmap is for a side metadata. + SideMeta { + /// The name of the space. + space: &'a str, + /// The name of the side metadata. + meta: &'a str, + }, + /// The mmap is for a test case. Usually constructed using the [`mmap_anno_test!`] macro. + Test { + /// The source file. + file: &'a str, + /// The line number. + line: u32, + }, + /// For all other use cases. + Misc { + /// A human-readable descriptive name. + name: &'a str, + }, } /// Construct an `MmapAnnotation::Test` with the current file name and line number. From 491a196cff164d50e5b8ff6aa0fda66c0a4ee632 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Tue, 26 Nov 2024 15:31:22 +0800 Subject: [PATCH 13/24] Workaround sysinfo build on MacOS --- Cargo.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 20486c341c..8cec69b50d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,10 @@ is-terminal = "0.4.7" itertools = "0.12.0" jemalloc-sys = { version = "0.5.3", features = ["disable_initial_exec_tls"], optional = true } lazy_static = "1.1" -libc = "0.2" +# libc 0.2.165 introduced an API-breaking change that breaks the `sysinfo` crate on MacOS. +# We temporarily pin the version of libc until the issue is resolved. +# See: https://github.com/GuillaumeGomez/sysinfo/issues/1392 +libc = "=0.2.164" log = { version = "0.4", features = ["max_level_trace", "release_max_level_off"] } memoffset = "0.9" mimalloc-sys = { version = "0.1.6", optional = true } From ff31481f4de5ac966770f1a2691a114aafe32cd7 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:39:26 +0800 Subject: [PATCH 14/24] Revert "Workaround sysinfo build on MacOS" This reverts commit 491a196cff164d50e5b8ff6aa0fda66c0a4ee632. --- Cargo.toml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8cec69b50d..20486c341c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,10 +33,7 @@ is-terminal = "0.4.7" itertools = "0.12.0" jemalloc-sys = { version = "0.5.3", features = ["disable_initial_exec_tls"], optional = true } lazy_static = "1.1" -# libc 0.2.165 introduced an API-breaking change that breaks the `sysinfo` crate on MacOS. -# We temporarily pin the version of libc until the issue is resolved. -# See: https://github.com/GuillaumeGomez/sysinfo/issues/1392 -libc = "=0.2.164" +libc = "0.2" log = { version = "0.4", features = ["max_level_trace", "release_max_level_off"] } memoffset = "0.9" mimalloc-sys = { version = "0.1.6", optional = true } From 4089d83703e9cd6d494ebdf4a080ab05f29f0edd Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:41:21 +0800 Subject: [PATCH 15/24] Use Space::get_name consistently --- src/policy/space.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/policy/space.rs b/src/policy/space.rs index 1b5b5026c1..d1dd36cede 100644 --- a/src/policy/space.rs +++ b/src/policy/space.rs @@ -155,7 +155,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { .and(self.common().metadata.try_map_metadata_space( res.start, bytes, - self.name(), + self.get_name(), )) { memory::handle_mmap_error::(mmap_error, tls, res.start, bytes); @@ -302,7 +302,7 @@ pub trait Space: 'static + SFT + Sync + Downcast { fn ensure_mapped(&self) { self.common() .metadata - .try_map_metadata_space(self.common().start, self.common().extent, self.name()) + .try_map_metadata_space(self.common().start, self.common().extent, self.get_name()) .unwrap_or_else(|e| { // TODO(Javad): handle meta space allocation failure panic!("failed to mmap meta memory: {e}"); From bbfdec0213668f652e319d63943747e384414846 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:45:57 +0800 Subject: [PATCH 16/24] Include android, too. --- src/util/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index 4387f98a59..e166ba90c1 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -258,7 +258,7 @@ fn mmap_fixed( ptr, )?; - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "linux", target_os = "android"))] if MMAP_ANNOTATION.load(std::sync::atomic::Ordering::SeqCst) { // `PR_SET_VMA` is new in Linux 5.17. We compile against a version of the `libc` crate that // has the `PR_SET_VMA_ANON_NAME` constant. When runnning on an older kernel, it will not From 9d860d331323e38041aa0a3ded41e39b25f31901 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:48:22 +0800 Subject: [PATCH 17/24] Use Relaxed order --- src/mmtk.rs | 4 +++- src/util/memory.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mmtk.rs b/src/mmtk.rs index 6b48969290..24e852e4d6 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -136,7 +136,9 @@ impl MMTK { /// Create an MMTK instance. This is not public. Bindings should use [`MMTKBuilder::build`]. pub(crate) fn new(options: Arc) -> Self { // Set the static variable for mmap annotation. - crate::util::memory::MMAP_ANNOTATION.store(*options.mmap_annotation, Ordering::SeqCst); + // `Ordering::Relaxed`: This happens when MMTk is initialized, therefore happens before all + // invocations of `mmap`. + crate::util::memory::MMAP_ANNOTATION.store(*options.mmap_annotation, Ordering::Relaxed); // Initialize SFT first in case we need to use this in the constructor. // The first call will initialize SFT map. Other calls will be blocked until SFT map is initialized. diff --git a/src/util/memory.rs b/src/util/memory.rs index e166ba90c1..d2c48cbc3e 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -259,7 +259,9 @@ fn mmap_fixed( )?; #[cfg(any(target_os = "linux", target_os = "android"))] - if MMAP_ANNOTATION.load(std::sync::atomic::Ordering::SeqCst) { + // `Ordering::Relaxed`: The only store happens when `MMTK` was initialized, which happens before + // everything else. + if MMAP_ANNOTATION.load(std::sync::atomic::Ordering::Relaxed) { // `PR_SET_VMA` is new in Linux 5.17. We compile against a version of the `libc` crate that // has the `PR_SET_VMA_ANON_NAME` constant. When runnning on an older kernel, it will not // recognize this attribute and will return `EINVAL`. However, `prctl` may return `EINVAL` From 18f3f6363ed62fc740cecf35dc5b9493468917fe Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:50:01 +0800 Subject: [PATCH 18/24] Update the doc for map_metadata_internal --- src/util/metadata/side_metadata/global.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index f5aa9c6e49..d24d6e512a 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1425,6 +1425,7 @@ impl SideMetadataContext { /// * `start` - The starting address of the source data. /// * `size` - The size of the source data (in bytes). /// * `no_reserve` - whether to invoke mmap with a noreserve flag (we use this flag to quarantine address range) + /// * `space_name`: The name of the space, used for annotating the mmap. fn map_metadata_internal( &self, start: Address, From c5bc4f9fd556913dc560f46bd357b62816ad746a Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 09:51:42 +0800 Subject: [PATCH 19/24] Comment the "all" metadata name. --- src/util/metadata/side_metadata/global.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/metadata/side_metadata/global.rs b/src/util/metadata/side_metadata/global.rs index d24d6e512a..b36a1332ad 100644 --- a/src/util/metadata/side_metadata/global.rs +++ b/src/util/metadata/side_metadata/global.rs @@ -1485,6 +1485,8 @@ impl SideMetadataContext { lsize, max ); + // We are creating a mmap for all side metadata instead of one specific metadata. We + // just annotate it as "all" here. let anno = MmapAnnotation::SideMeta { space: space_name, meta: "all", From e1071afa880890c692e24563f5f7751a9d637521 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 12:39:16 +0800 Subject: [PATCH 20/24] Doc for strategy and anno --- src/util/heap/layout/mmapper.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/heap/layout/mmapper.rs b/src/util/heap/layout/mmapper.rs index 7548f701fa..b2956a5dba 100644 --- a/src/util/heap/layout/mmapper.rs +++ b/src/util/heap/layout/mmapper.rs @@ -32,6 +32,9 @@ pub trait Mmapper: Sync { /// Arguments: /// * `start`: Address of the first page to be quarantined /// * `bytes`: Number of bytes to quarantine from the start + /// * `strategy`: The mmap strategy. The `prot` field is ignored because we always use + /// `PROT_NONE`. + /// * `anno`: Human-readable annotation to apply to newly mapped memory ranges. fn quarantine_address_range( &self, start: Address, @@ -47,6 +50,8 @@ pub trait Mmapper: Sync { /// Arguments: /// * `start`: The start of the range to be mapped. /// * `pages`: The size of the range to be mapped, in pages + /// * `strategy`: The mmap strategy. + /// * `anno`: Human-readable annotation to apply to newly mapped memory ranges. // NOTE: There is a monotonicity assumption so that only updates require lock // acquisition. // TODO: Fix the above to support unmapping. From 13c10b550a07411ede223c9dcb71e132eda6109e Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 14:57:33 +0800 Subject: [PATCH 21/24] Control annotation with Cargo feature instead. --- Cargo.toml | 7 +++++++ src/mmtk.rs | 5 ----- src/util/memory.rs | 9 +++++---- src/util/options.rs | 6 +----- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 20486c341c..f5c90ce936 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -179,6 +179,13 @@ count_live_bytes_in_gc = [] # capture the type names of work packets. bpftrace_workaround = [] +# Disable mmap annotations. +# All invocations of `mmap` in mmtk-core are accompanied by calls of `prctl` with +# `PR_SET_VMA_ANON_NAME` to annotate the mmap ranges with human-readable names. It is enabled by +# default and should work fine even with large heap sizes. However, if this is causing problems, +# users can disable such annotations by enabling this Cargo feature. +no_mmap_annotation = [] + # Do not modify the following line - ci-common.sh matches it # -- Mutally exclusive features -- # Only one feature from each group can be provided. Otherwise build will fail. diff --git a/src/mmtk.rs b/src/mmtk.rs index 24e852e4d6..d636e2472a 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -135,11 +135,6 @@ unsafe impl Send for MMTK {} impl MMTK { /// Create an MMTK instance. This is not public. Bindings should use [`MMTKBuilder::build`]. pub(crate) fn new(options: Arc) -> Self { - // Set the static variable for mmap annotation. - // `Ordering::Relaxed`: This happens when MMTk is initialized, therefore happens before all - // invocations of `mmap`. - crate::util::memory::MMAP_ANNOTATION.store(*options.mmap_annotation, Ordering::Relaxed); - // Initialize SFT first in case we need to use this in the constructor. // The first call will initialize SFT map. Other calls will be blocked until SFT map is initialized. crate::policy::sft_map::SFTRefStorage::pre_use_check(); diff --git a/src/util/memory.rs b/src/util/memory.rs index d2c48cbc3e..31517d235c 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -258,10 +258,11 @@ fn mmap_fixed( ptr, )?; - #[cfg(any(target_os = "linux", target_os = "android"))] - // `Ordering::Relaxed`: The only store happens when `MMTK` was initialized, which happens before - // everything else. - if MMAP_ANNOTATION.load(std::sync::atomic::Ordering::Relaxed) { + #[cfg(all( + any(target_os = "linux", target_os = "android"), + not(feature = "no_mmap_annotation") + ))] + { // `PR_SET_VMA` is new in Linux 5.17. We compile against a version of the `libc` crate that // has the `PR_SET_VMA_ANON_NAME` constant. When runnning on an older kernel, it will not // recognize this attribute and will return `EINVAL`. However, `prctl` may return `EINVAL` diff --git a/src/util/options.rs b/src/util/options.rs index 5a606accbd..49eab795e0 100644 --- a/src/util/options.rs +++ b/src/util/options.rs @@ -864,11 +864,7 @@ options! { gc_trigger: GCTriggerSelector [env_var: true, command_line: true] [|v: &GCTriggerSelector| v.validate()] = GCTriggerSelector::FixedHeapSize((crate::util::memory::get_system_total_memory() as f64 * 0.5f64) as usize), /// Enable transparent hugepage support for MMTk spaces via madvise (only Linux is supported) /// This only affects the memory for MMTk spaces. - transparent_hugepages: bool [env_var: true, command_line: true] [|v: &bool| !v || cfg!(target_os = "linux")] = false, - /// Enable mmap annotation. By default, every time MMTk core calls `mmap`, it will annotate the - /// mapped memory range with human-readable names, which can be seen in `/proc/pid/maps`. - /// Setting this option to `false` will disable this annotation. - mmap_annotation: bool [env_var: true, command_line: true] [always_valid] = true + transparent_hugepages: bool [env_var: true, command_line: true] [|v: &bool| !v || cfg!(target_os = "linux")] = false } #[cfg(test)] From df35f42db9bacc24011b0e1694022a6023d25615 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 15:21:47 +0800 Subject: [PATCH 22/24] A missing doc comment --- src/util/heap/layout/mmapper.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/heap/layout/mmapper.rs b/src/util/heap/layout/mmapper.rs index b2956a5dba..364cc0bc0d 100644 --- a/src/util/heap/layout/mmapper.rs +++ b/src/util/heap/layout/mmapper.rs @@ -169,6 +169,8 @@ impl MapState { /// /// * `state_slices`: A slice of slices. Each inner slice is a part of a `Slab`. /// * `mmap_start`: The start of the region to transition. + /// * `strategy`: The mmap strategy. + /// * `anno`: Human-readable annotation to apply to newly mapped memory ranges. pub(super) fn bulk_transition_to_quarantined( state_slices: &[&[Atomic]], mmap_start: Address, From b1e43d5e3b5b2590f67c35e918d87a1c23c04610 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 27 Nov 2024 15:53:23 +0800 Subject: [PATCH 23/24] API migration guide --- docs/userguide/src/migration/prefix.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/userguide/src/migration/prefix.md b/docs/userguide/src/migration/prefix.md index 51e8b0fc17..451fd5657c 100644 --- a/docs/userguide/src/migration/prefix.md +++ b/docs/userguide/src/migration/prefix.md @@ -30,6 +30,22 @@ Notes for the mmtk-core developers: +## 0.30.0 + +### mmap-related functions require annotation + +```admonish tldr +Memory-mapping functions in `mmtk::util::memory` now take an additional `MmapAnnotation` argument. +``` + +API changes: + +- module `util::memory` + + The following functions take an additional `MmapAnnotation` argument. + * `dzmmap` + * `dzmmap_noreplace` + * `mmap_noreserve` + ## 0.28.0 ### `handle_user_collection_request` returns `bool` From b3d8672d4349f2d398fd5dfb49dc42cd0b81c0fd Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Fri, 29 Nov 2024 09:59:51 +0800 Subject: [PATCH 24/24] Remove unused static variable --- src/util/memory.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/util/memory.rs b/src/util/memory.rs index 31517d235c..0e689be6b9 100644 --- a/src/util/memory.rs +++ b/src/util/memory.rs @@ -5,7 +5,6 @@ use crate::vm::{Collection, VMBinding}; use bytemuck::NoUninit; use libc::{PROT_EXEC, PROT_NONE, PROT_READ, PROT_WRITE}; use std::io::{Error, Result}; -use std::sync::atomic::AtomicBool; use sysinfo::MemoryRefreshKind; use sysinfo::{RefreshKind, System}; @@ -16,14 +15,6 @@ const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_F // MAP_FIXED is used instead of MAP_FIXED_NOREPLACE (which is not available on macOS). We are at the risk of overwriting pre-existing mappings. const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED; -/// This static variable controls whether we annotate mmapped memory region using `prctl`. It can be -/// set via `Options::mmap_annotation` or the `MMTK_MMAP_ANNOTATION` environment variable. -/// -/// FIXME: Since it is set via `Options`, it is in theory a decision per MMTk instance. However, we -/// currently don't have a good design for multiple MMTk instances, so we use static variable for -/// now. -pub(crate) static MMAP_ANNOTATION: AtomicBool = AtomicBool::new(true); - /// Strategy for performing mmap #[derive(Debug, Copy, Clone)] pub struct MmapStrategy {