diff --git a/kernel/src/vm/vmar/interval_set.rs b/kernel/src/vm/vmar/interval_set.rs index 57df6c7da..389237f71 100644 --- a/kernel/src/vm/vmar/interval_set.rs +++ b/kernel/src/vm/vmar/interval_set.rs @@ -96,6 +96,26 @@ where } } + /// Finds the last interval item before the given point. + /// + /// If no such item exists, returns [`None`]. + pub fn find_prev(&self, point: &K) -> Option<&V> { + self.btree + .upper_bound(core::ops::Bound::Excluded(point)) + .peek_prev() + .map(|(_, v)| v) + } + + /// Finds the first interval item after the given point. + /// + /// If no such item exists, returns [`None`]. + pub fn find_next(&self, point: &K) -> Option<&V> { + self.btree + .lower_bound(core::ops::Bound::Excluded(point)) + .peek_next() + .map(|(_, v)| v) + } + /// Takes an interval item that contains the given point. /// /// If no such item exists, returns [`None`]. Otherwise, returns the item diff --git a/kernel/src/vm/vmar/mod.rs b/kernel/src/vm/vmar/mod.rs index f4ee3e433..687b86d82 100644 --- a/kernel/src/vm/vmar/mod.rs +++ b/kernel/src/vm/vmar/mod.rs @@ -146,6 +146,10 @@ pub(super) struct Vmar_ { struct VmarInner { /// The mapped pages and associated metadata. + /// + /// When inserting a `VmMapping` into this set, use `insert_try_merge` to + /// auto-merge adjacent and compatible mappings, or `insert_without_try_merge` + /// if the mapping is known not mergeable with any neighboring mappings. vm_mappings: IntervalSet, /// The total mapped memory in bytes. total_vm: usize, @@ -193,21 +197,53 @@ impl VmarInner { { Ok(vm_mapping.map_to_addr()) } else { - // FIXME: In Linux, two adjacent mappings created by `mmap` with - // identical properties can be `mremap`ed together. Fix this by - // adding an auto-merge mechanism for adjacent `VmMapping`s. return_errno_with_message!(Errno::EFAULT, "The range must lie in a single mapping"); } } - /// Inserts a `VmMapping` into the `Vmar`. + /// Inserts a `VmMapping` into the `Vmar`, without attempting to merge with + /// neighboring mappings. + /// + /// The caller must ensure that the given `VmMapping` is not mergeable with + /// any neighboring mappings. /// /// Make sure the insertion doesn't exceed address space limit. - fn insert(&mut self, vm_mapping: VmMapping) { + fn insert_without_try_merge(&mut self, vm_mapping: VmMapping) { self.total_vm += vm_mapping.map_size(); self.vm_mappings.insert(vm_mapping); } + /// Inserts a `VmMapping` into the `Vmar`, and attempts to merge it with + /// neighboring mappings. + /// + /// This method will try to merge the `VmMapping` with neighboring mappings + /// that are adjacent and compatible, in order to reduce fragmentation. + /// + /// Make sure the insertion doesn't exceed address space limit. + fn insert_try_merge(&mut self, vm_mapping: VmMapping) { + self.total_vm += vm_mapping.map_size(); + let mut vm_mapping = vm_mapping; + let addr = vm_mapping.map_to_addr(); + + if let Some(prev) = self.vm_mappings.find_prev(&addr) { + let (new_mapping, to_remove) = vm_mapping.try_merge_with(prev); + vm_mapping = new_mapping; + if let Some(addr) = to_remove { + self.vm_mappings.remove(&addr); + } + } + + if let Some(next) = self.vm_mappings.find_next(&addr) { + let (new_mapping, to_remove) = vm_mapping.try_merge_with(next); + vm_mapping = new_mapping; + if let Some(addr) = to_remove { + self.vm_mappings.remove(&addr); + } + } + + self.vm_mappings.insert(vm_mapping); + } + /// Removes a `VmMapping` based on the provided key from the `Vmar`. fn remove(&mut self, key: &Vaddr) -> Option { let vm_mapping = self.vm_mappings.remove(key)?; @@ -272,10 +308,10 @@ impl VmarInner { let (left, taken, right) = vm_mapping.split_range(&intersected_range); if let Some(left) = left { - self.insert(left); + self.insert_without_try_merge(left); } if let Some(right) = right { - self.insert(right); + self.insert_without_try_merge(right); } rss_delta.add(taken.rss_type(), -(taken.unmap(vm_space) as isize)); @@ -378,7 +414,7 @@ impl VmarInner { self.check_extra_size_fits_rlimit(new_map_end - old_map_end)?; let last_mapping = self.remove(&last_mapping_addr).unwrap(); let last_mapping = last_mapping.enlarge(new_map_end - old_map_end); - self.insert(last_mapping); + self.insert_try_merge(last_mapping); Ok(()) } } @@ -462,16 +498,17 @@ impl Vmar_ { // Protects part of the taken `VmMapping`. let (left, taken, right) = vm_mapping.split_range(&intersected_range); - let taken = taken.protect(vm_space.as_ref(), perms); - inner.insert(taken); - - // And put the rest back. + // Puts the rest back. if let Some(left) = left { - inner.insert(left); + inner.insert_without_try_merge(left); } if let Some(right) = right { - inner.insert(right); + inner.insert_without_try_merge(right); } + + // Protects part of the `VmMapping`. + let taken = taken.protect(vm_space.as_ref(), perms); + inner.insert_try_merge(taken); } Ok(()) @@ -592,10 +629,10 @@ impl Vmar_ { let vm_mapping = inner.remove(&old_mapping_addr).unwrap(); let (left, old_mapping, right) = vm_mapping.split_range(&old_range); if let Some(left) = left { - inner.insert(left); + inner.insert_without_try_merge(left); } if let Some(right) = right { - inner.insert(right); + inner.insert_without_try_merge(right); } if new_size < old_size { let (old_mapping, taken) = old_mapping.split(old_range.start + new_size).unwrap(); @@ -609,7 +646,7 @@ impl Vmar_ { }; // Now we can ensure that `new_size >= old_size`. let new_mapping = old_mapping.clone_for_remap_at(new_range.start).unwrap(); - inner.insert(new_mapping.enlarge(new_size - old_size)); + inner.insert_try_merge(new_mapping.enlarge(new_size - old_size)); let preempt_guard = disable_preempt(); let total_range = old_range.start.min(new_range.start)..old_range.end.max(new_range.end); @@ -678,7 +715,7 @@ impl Vmar_ { // Clone the `VmMapping` to the new VMAR. let new_mapping = vm_mapping.new_fork()?; - new_inner.insert(new_mapping); + new_inner.insert_without_try_merge(new_mapping); // Protect the mapping and copy to the new page table for COW. cur_cursor.jump(base).unwrap(); @@ -1017,7 +1054,7 @@ where ); // Add the mapping to the VMAR. - inner.insert(vm_mapping); + inner.insert_try_merge(vm_mapping); Ok(map_to_addr) } diff --git a/kernel/src/vm/vmar/vm_mapping.rs b/kernel/src/vm/vmar/vm_mapping.rs index 7df39270c..d2859f4b4 100644 --- a/kernel/src/vm/vmar/vm_mapping.rs +++ b/kernel/src/vm/vmar/vm_mapping.rs @@ -22,6 +22,7 @@ use crate::{ vm::{ perms::VmPerms, util::duplicate_frame, + vmar::is_intersected, vmo::{CommitFlags, Vmo, VmoCommitError}, }, }; @@ -467,6 +468,40 @@ impl VmMapping { panic!("The mapping does not contain the splitting range"); } } + + /// Attempts to merge `self` with the given `vm_mapping` if they are + /// adjacent and compatible. + /// + /// Two mappings are considered *adjacent* if the end address of `self` + /// equals to the start address of `vm_mapping`, or vice versa. + /// + /// Two mappings are considered *compatible* if all of the following + /// conditions are met: + /// - They have the same access permissions. + /// - They are both anonymous or share the same backing file. + /// - Their file offsets are contiguous if file-backed. + /// - Other attributes (e.g., shared/private flags, whether need to handle + /// page faults around, etc.) must also match. + /// + /// This method returns: + /// - the merged mapping along with the address of the mapping + /// to be removed if successful. + /// - the original `self` and a `None` otherwise. + pub fn try_merge_with(self, vm_mapping: &VmMapping) -> (Self, Option) { + debug_assert!(!is_intersected(&self.range(), &vm_mapping.range())); + + let (left, right) = if self.map_to_addr < vm_mapping.map_to_addr { + (&self, vm_mapping) + } else { + (vm_mapping, &self) + }; + + if let Some(merged) = try_merge(left, right) { + (merged, Some(vm_mapping.map_to_addr)) + } else { + (self, None) + } + } } /************************** VM Space operations ******************************/ @@ -581,3 +616,44 @@ impl MappedVmo { }) } } + +/// Attempts to merge two [`VmMapping`]s into a single mapping if they are +/// adjacent and compatible. +/// +/// - Returns the merged [`VmMapping`] if successful. The caller should +/// remove the original mappings before inserting the merged mapping +/// into the [`Vmar`]. +/// - Returns `None` otherwise. +fn try_merge(left: &VmMapping, right: &VmMapping) -> Option { + let is_adjacent = left.map_end() == right.map_to_addr(); + let is_type_equal = left.is_shared == right.is_shared + && left.handle_page_faults_around == right.handle_page_faults_around + && left.perms == right.perms; + + if !is_adjacent || !is_type_equal { + return None; + } + + let vmo = match (&left.vmo, &right.vmo) { + (None, None) => None, + (Some(l_vmo), Some(r_vmo)) if Arc::ptr_eq(&l_vmo.vmo.0, &r_vmo.vmo.0) => { + let is_offset_contiguous = r_vmo.range.start - l_vmo.range.start == left.map_size() + && l_vmo.range.end - l_vmo.range.start >= left.map_size(); + if !is_offset_contiguous { + return None; + } + let range = l_vmo.range.start..l_vmo.range.end.max(r_vmo.range.end); + Some(MappedVmo::new(l_vmo.vmo.dup().ok()?, range)) + } + _ => return None, + }; + + let map_size = NonZeroUsize::new(left.map_size() + right.map_size()).unwrap(); + + Some(VmMapping { + map_size, + vmo, + inode: left.inode.clone(), + ..*left + }) +} diff --git a/test/apps/mmap/mmap_and_mremap.c b/test/apps/mmap/mmap_and_mremap.c index 5ad0ac428..2f678d2dd 100644 --- a/test/apps/mmap/mmap_and_mremap.c +++ b/test/apps/mmap/mmap_and_mremap.c @@ -5,7 +5,8 @@ #include #include #include - +#include +#include #include "../test.h" #define PAGE_SIZE 4096 @@ -106,3 +107,54 @@ FN_TEST(mmap_and_mremap_fixed) TEST_SUCC(munmap(addr1, PAGE_SIZE)); } END_TEST() + +FN_TEST(mmap_and_mremap_auto_merge_anon) +{ + char *addr = CHECK_MM(mmap(NULL, 6 * PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); + TEST_SUCC(munmap(addr, 6 * PAGE_SIZE)); + + CHECK_MM(mmap(addr, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0)); + strcpy(addr, content); + CHECK_MM(mmap(addr + 2 * PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0)); + CHECK_MM(mmap(addr + PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0)); + + char *new_addr = CHECK_MM(mremap(addr, 3 * PAGE_SIZE, 3 * PAGE_SIZE, + MREMAP_MAYMOVE | MREMAP_FIXED, + addr + 3 * PAGE_SIZE)); + TEST_RES(strcmp(new_addr, content), _ret == 0); + TEST_SUCC(munmap(new_addr, 3 * PAGE_SIZE)); +} +END_TEST() + +FN_TEST(mmap_and_mremap_auto_merge_file) +{ + const char *filename = "mremap_test_file"; + int fd = TEST_SUCC(open(filename, O_CREAT | O_RDWR, 0600)); + TEST_SUCC(ftruncate(fd, 6 * PAGE_SIZE)); + + char *addr = CHECK_MM(mmap(NULL, 6 * PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE, fd, 0)); + TEST_SUCC(munmap(addr, 6 * PAGE_SIZE)); + + CHECK_MM(mmap(addr, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_FIXED, fd, 0)); + strcpy(addr, content); + CHECK_MM(mmap(addr + 2 * PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_FIXED, fd, 2 * PAGE_SIZE)); + CHECK_MM(mmap(addr + PAGE_SIZE, PAGE_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_FIXED, fd, PAGE_SIZE)); + + char *new_addr = CHECK_MM(mremap(addr, 3 * PAGE_SIZE, 3 * PAGE_SIZE, + MREMAP_MAYMOVE | MREMAP_FIXED, + addr + 3 * PAGE_SIZE)); + TEST_RES(strcmp(new_addr, content), _ret == 0); + TEST_SUCC(munmap(new_addr, 3 * PAGE_SIZE)); + + close(fd); + unlink(filename); +} +END_TEST()