From 7336e524836773027951dda95f1607fbc39429c1 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Sat, 11 Jul 2026 15:38:33 -0400 Subject: [PATCH] memory: preserve first-fit gap selection across ordered regions Merge the address-sorted guest mapping and host reservation streams without materializing a combined list. Normalize fork-restored mappings once at their ingestion boundary so the ordering invariant is explicit. Reference-equivalence tests cover interleaving, equal starts, overlap, exact fits, exhaustion, and systematic relative orderings. The copied munmap regression from the original PR remains owned by #756. This is an unmeasured internal refactor. It makes no throughput or PHP/PHPT coverage claim and changes no intended host or ABI contract. --- crates/kernel/src/memory.rs | 196 ++++++++++++++++++++++++++++++++++-- 1 file changed, 189 insertions(+), 7 deletions(-) diff --git a/crates/kernel/src/memory.rs b/crates/kernel/src/memory.rs index 9eda24f37..b7808b0a5 100644 --- a/crates/kernel/src/memory.rs +++ b/crates/kernel/src/memory.rs @@ -92,7 +92,11 @@ impl MemoryManager { } /// Restore mmap mappings from fork (used by deserialize_fork_state). - pub fn set_mappings(&mut self, mappings: Vec) { + pub fn set_mappings(&mut self, mut mappings: Vec) { + // Ordinary mmap/munmap mutations preserve address order. Restore that + // invariant explicitly at the fork-state boundary so later first-fit + // scans do not depend on serialized input order. + mappings.sort_by_key(|mapping| mapping.addr); self.mappings = mappings; } @@ -173,13 +177,43 @@ impl MemoryManager { /// Find the first gap in [mmap_base, max_addr) that can fit `needed` bytes. fn find_gap(&self, needed: usize) -> Option { let mut cursor = self.mmap_base.max(self.program_break); - let mut occupied: Vec<(usize, usize)> = - Vec::with_capacity(self.mappings.len() + self.reserved_regions.len()); - occupied.extend(self.mappings.iter().map(|m| (m.addr, m.len))); - occupied.extend(self.reserved_regions.iter().map(|r| (r.addr, r.len))); - occupied.sort_by_key(|(addr, _)| *addr); - for (addr, len) in occupied { + // Both collections are maintained in address order. Merge them as two + // sorted streams so the first-fit decision is identical to scanning a + // combined sorted list without materializing that temporary list. + let mut mapping_idx = 0; + let mut reserved_idx = 0; + loop { + let next_mapping = self + .mappings + .get(mapping_idx) + .map(|mapping| (mapping.addr, mapping.len, true)); + let next_reserved = self + .reserved_regions + .get(reserved_idx) + .map(|reserved| (reserved.addr, reserved.len, false)); + let Some((addr, len, is_mapping)) = (match (next_mapping, next_reserved) { + (Some(mapping), Some(reserved)) => { + // The old stable address sort saw mappings before reserved + // regions at an equal start address. + if mapping.0 <= reserved.0 { + Some(mapping) + } else { + Some(reserved) + } + } + (Some(mapping), None) => Some(mapping), + (None, Some(reserved)) => Some(reserved), + (None, None) => None, + }) else { + break; + }; + + if is_mapping { + mapping_idx += 1; + } else { + reserved_idx += 1; + } if addr < cursor { let end = addr.saturating_add(len); if end > cursor { @@ -552,6 +586,42 @@ mod tests { use super::*; use wasm_posix_shared::mmap::*; + fn mapped_region(addr: usize, len: usize) -> MappedRegion { + MappedRegion { + addr, + len, + prot: PROT_READ | PROT_WRITE, + flags: MAP_PRIVATE | MAP_ANONYMOUS, + } + } + + /// Reference the former combined-list implementation so the stream merge + /// is checked against the exact first-fit policy it replaces. + fn reference_find_gap(mm: &MemoryManager, needed: usize) -> Option { + let mut cursor = mm.mmap_base.max(mm.program_break); + let mut occupied = Vec::with_capacity(mm.mappings.len() + mm.reserved_regions.len()); + occupied.extend(mm.mappings.iter().map(|mapping| (mapping.addr, mapping.len))); + occupied.extend( + mm.reserved_regions + .iter() + .map(|reserved| (reserved.addr, reserved.len)), + ); + occupied.sort_by_key(|(addr, _)| *addr); + + for (addr, len) in occupied { + if addr < cursor { + cursor = cursor.max(addr.saturating_add(len)); + continue; + } + if addr - cursor >= needed { + return Some(cursor); + } + cursor = cursor.max(addr.saturating_add(len)); + } + + (cursor.saturating_add(needed) <= mm.max_addr).then_some(cursor) + } + #[test] fn test_mmap_anonymous() { let mut mm = MemoryManager::new(); @@ -668,6 +738,118 @@ mod tests { assert_eq!(addr2, addr + 0x10000); } + #[test] + fn test_mmap_gap_stream_merge_matches_combined_reference() { + let page = 0x10000; + let base = MemoryManager::MMAP_BASE; + let layouts = [ + // Empty address space. + (vec![], vec![], base + 8 * page), + // Guest and host ranges interleave with exact and undersized gaps. + ( + vec![mapped_region(base, page), mapped_region(base + 4 * page, page)], + vec![ReservedRegion { + addr: base + 2 * page, + len: page, + }], + base + 8 * page, + ), + // Equal starts retain the former stable-sort order, while the + // longer overlapping reservation still advances the cursor. + ( + vec![mapped_region(base, page)], + vec![ReservedRegion { + addr: base, + len: 3 * page, + }], + base + 8 * page, + ), + // Occupancy beginning before mmap_base can overlap and extend + // through later entries from the other stream. + ( + vec![mapped_region(base - page, 3 * page)], + vec![ReservedRegion { + addr: base + page, + len: 3 * page, + }], + base + 8 * page, + ), + // No trailing range is large enough. + ( + vec![mapped_region(base, 2 * page)], + vec![ReservedRegion { + addr: base + 2 * page, + len: 2 * page, + }], + base + 4 * page, + ), + ]; + + for (case, (mappings, reserved_regions, max_addr)) in layouts.into_iter().enumerate() { + let mut mm = MemoryManager::new(); + mm.max_addr = max_addr; + mm.set_mappings(mappings); + mm.reserved_regions = reserved_regions; + mm.reserved_regions + .sort_unstable_by_key(|reserved| reserved.addr); + + for needed in [page, 2 * page, 3 * page] { + assert_eq!( + mm.find_gap(needed), + reference_find_gap(&mm, needed), + "layout {case}, needed {needed:#x}", + ); + } + } + + // Exhaust the relative ordering and overlap combinations for one + // range from each stream, including starts below mmap_base and equal + // starts. The table above covers multiple entries within one stream. + for mapping_slot in 0..=5 { + for mapping_pages in 1..=3 { + for reserved_slot in 0..=5 { + for reserved_pages in 1..=3 { + let mut mm = MemoryManager::new(); + mm.max_addr = base + 8 * page; + mm.set_mappings(vec![mapped_region( + base - page + mapping_slot * page, + mapping_pages * page, + )]); + mm.reserved_regions = vec![ReservedRegion { + addr: base - page + reserved_slot * page, + len: reserved_pages * page, + }]; + + for needed in [page, 2 * page, 3 * page] { + assert_eq!(mm.find_gap(needed), reference_find_gap(&mm, needed)); + } + } + } + } + } + } + + #[test] + fn test_set_mappings_restores_address_order() { + let page = 0x10000; + let base = MemoryManager::MMAP_BASE; + let mut mm = MemoryManager::new(); + + mm.set_mappings(vec![ + mapped_region(base + 2 * page, page), + mapped_region(base, page), + ]); + + assert_eq!( + mm.mappings() + .iter() + .map(|mapping| mapping.addr) + .collect::>(), + vec![base, base + 2 * page], + ); + assert_eq!(mm.find_gap(page), Some(base + page)); + } + #[test] fn test_mmap_non_fixed_prefers_free_address_hint() { let mut mm = MemoryManager::new();