From 24e5f94384b463287da26c1590465a38b1207bcc Mon Sep 17 00:00:00 2001 From: Graham MacDonald Date: Sun, 26 May 2024 00:17:20 +0100 Subject: [PATCH] vmem free Signed-off-by: Graham MacDonald --- port/src/boundarytag.rs | 185 ++++++++++++++++++++++++++++------------ 1 file changed, 131 insertions(+), 54 deletions(-) diff --git a/port/src/boundarytag.rs b/port/src/boundarytag.rs index 62f846d..608d3af 100644 --- a/port/src/boundarytag.rs +++ b/port/src/boundarytag.rs @@ -1,4 +1,4 @@ -use core::{fmt, ptr::null_mut, slice}; +use core::{ptr::null_mut, slice}; use crate::mem::VirtRange; @@ -15,12 +15,13 @@ pub enum BoundaryError { #[derive(Debug, PartialEq)] pub enum AllocError { NoSpace, + AllocationNotFound, } #[cfg(test)] type BoundaryResult = core::result::Result; -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] struct Boundary { start: usize, size: usize, @@ -54,14 +55,14 @@ impl Boundary { } } -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] enum TagType { Allocated, Free, Span, } -#[derive(Copy, Clone, Debug, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] struct Tag { tag_type: TagType, boundary: Boundary, @@ -213,6 +214,19 @@ impl TagList { } } + /// Remove tag_item from the list. Placing tag_item onto the free list is + /// the callers responsibility. + fn unlink(tag_item: &mut TagItem) { + if let Some(prev) = unsafe { tag_item.prev.as_mut() } { + prev.next = tag_item.next; + } + if let Some(next) = unsafe { tag_item.next.as_mut() } { + next.prev = tag_item.prev; + } + tag_item.next = null_mut(); + tag_item.prev = null_mut(); + } + fn len(&self) -> usize { let mut n = 0; let mut curr_tag = self.tags; @@ -223,7 +237,6 @@ impl TagList { n } - #[cfg(test)] fn tags_iter(&self) -> impl Iterator + '_ { let mut curr_tag_item = self.tags; core::iter::from_fn(move || { @@ -348,7 +361,7 @@ impl Arena { pub fn free(&mut self, addr: *mut u8) { // TODO Look up in allocation hashtable - panic!("can't free before allocation hashtable set up"); + let _ = self.free_segment(addr as usize); } /// Allocate a segment, returned as a boundary @@ -369,10 +382,12 @@ impl Arena { let mut curr_item = self.used_tags.tags; while let Some(item) = unsafe { curr_item.as_mut() } { if item.tag.tag_type == TagType::Free && item.tag.boundary.size >= size { - // Mark this tag as allocated, and if there's any left over space, create and insert a new tag + // Mark this tag as allocated, and if there's any left over space, + // create and insert a new tag item.tag.tag_type = TagType::Allocated; if item.tag.boundary.size > size { - // Work out the size of the new free item, and change the size of the current, now allocated, item + // Work out the size of the new free item, and change the size + // of the current, now allocated, item let remainder = item.tag.boundary.size - size; item.tag.boundary.size = size; @@ -397,49 +412,89 @@ impl Arena { Err(AllocError::NoSpace) } - // Free addr. We need to know size in case the previous allocation was - // merged with others around it. + // Free addr. We don't need to know size because we don't merge allocations. + // (We only merge freed segments) // TODO Error on precondition fail // TODO Use hashmap if available - // TODO Return Result - fn free_segment(&mut self, boundary: Boundary) { + fn free_segment(&mut self, addr: usize) -> Result<(), AllocError> { // Need to manually scan the used tags - let start = boundary.start as usize; - let end = boundary.end(); let mut curr_item = self.used_tags.tags; while let Some(item) = unsafe { curr_item.as_mut() } { - if item.tag.boundary.start <= start && item.tag.boundary.end() <= end { + if item.tag.boundary.start == addr && item.tag.tag_type == TagType::Allocated { break; } curr_item = item.next; } if curr_item.is_null() { - // TODO Return error - return; + return Err(AllocError::AllocationNotFound); + } + + let mut curr_tag: &mut TagItem = unsafe { curr_item.as_mut() }.unwrap(); + + // Found tag to free + let prev_type = unsafe { curr_tag.prev.as_ref() }.map(|t| t.tag.tag_type); + let next_type = unsafe { curr_tag.next.as_ref() }.map(|t| t.tag.tag_type); + + match (prev_type, next_type) { + (Some(TagType::Allocated), Some(TagType::Allocated)) + | (Some(TagType::Span), Some(TagType::Span)) + | (Some(TagType::Span), Some(TagType::Allocated)) + | (Some(TagType::Allocated), Some(TagType::Span)) + | (Some(TagType::Span), None) + | (Some(TagType::Allocated), None) => { + // No frees on either side + // -> Change curr_tag to free + curr_tag.tag.tag_type = TagType::Free; + } + (Some(TagType::Span), Some(TagType::Free)) + | (Some(TagType::Allocated), Some(TagType::Free)) => { + // Prev non-free, next free + // Change next tag start to merge with curr_tag, release curr_tag + let next = unsafe { curr_tag.next.as_mut() }.unwrap(); + next.tag.boundary.start = curr_tag.tag.boundary.start; + next.tag.boundary.size += curr_tag.tag.boundary.size; + TagList::unlink(&mut curr_tag); + self.free_tags.push(&mut curr_tag); + } + (Some(TagType::Free), None) + | (Some(TagType::Free), Some(TagType::Span)) + | (Some(TagType::Free), Some(TagType::Allocated)) => { + // Prev free, next non-free + // Change prev tag size to merge with curr_tag, release curr_tag + let prev = unsafe { curr_tag.prev.as_mut() }.unwrap(); + prev.tag.boundary.size += curr_tag.tag.boundary.size; + TagList::unlink(&mut curr_tag); + self.free_tags.push(&mut curr_tag); + } + (Some(TagType::Free), Some(TagType::Free)) => { + // Prev and next both free + // Change prev size to merge with both curr_tag and next, release curr_tag + let prev = unsafe { curr_tag.prev.as_mut() }.unwrap(); + let mut next = unsafe { curr_tag.next.as_mut() }.unwrap(); + prev.tag.boundary.size += curr_tag.tag.boundary.size + next.tag.boundary.size; + TagList::unlink(&mut curr_tag); + TagList::unlink(&mut next); + self.free_tags.push(&mut curr_tag); + self.free_tags.push(&mut next); + } + (None, None) + | (None, Some(TagType::Span)) + | (None, Some(TagType::Allocated)) + | (None, Some(TagType::Free)) => { + self.assert_tags_are_consistent(); + panic!("Unexpected tags when freeing"); + } } - // Found tag to free. Cases: - // Allocated segment encloses segment exactly: - // - If segment to the left is a span and segment to the right is a span or doesn't exist, then change segment to free - // - If segment to the left is free and segment to the right is a span or doesn't exist, then change left segment size - // Allocated segment encloses segment to free with space to the left and right: - // - Split existing segment, create new tag for free segment, new tag for allocated segment to the right - // Allocated segment starts before but ends at same point of segment to free: - // - If the segment to the right is a span, insert a new free segment. - // - If the segment to the right is free, merge by changing start of that segment. - // Allocated segment starts at same point of segment to free but ends after; - // - If the segment to the left is a span, insert a new free segment. - // - If the segment to the left is free, merge by changing end of that segment. + Ok(()) } - #[cfg(test)] fn tags_iter(&self) -> impl Iterator + '_ { self.used_tags.tags_iter() } /// Checks that all invariants are correct. - #[cfg(test)] fn assert_tags_are_consistent(&self) { // There must be at least 2 tags debug_assert!(self.used_tags.len() >= 2); @@ -484,21 +539,12 @@ impl Arena { } last_span = Some(tag); } - TagType::Allocated => { - last_span_total += tag.boundary.size; - // First tag after span should have same start as span - if last_tag.is_some_and(|t| t.tag_type == TagType::Span) { - debug_assert_eq!(tag.boundary.start, last_tag.unwrap().boundary.start); - } - } - TagType::Free => { + TagType::Allocated | TagType::Free => { last_span_total += tag.boundary.size; // First tag after span should have same start as span if last_tag.is_some_and(|t| t.tag_type == TagType::Span) { debug_assert_eq!(tag.boundary.start, last_tag.unwrap().boundary.start); } - // Free tag must be last in span - debug_assert_eq!(last_span_total, last_span.unwrap().boundary.size); } } last_tag = Some(tag); @@ -645,6 +691,7 @@ mod tests { #[test] fn test_arena_create() { let arena = create_arena_with_static_tags("test", 4096, 4096 * 20, 4096); + assert_eq!(arena.free_tags.len(), 100); assert_tags_eq( &arena, @@ -681,13 +728,15 @@ mod tests { #[test] fn test_arena_free() { let mut arena = create_arena_with_static_tags("test", 4096, 4096 * 20, 4096); + assert_eq!(arena.free_tags.len(), 100); // We need to test each case where we're freeing by scanning the tags linearly. // To do this we run through each case (comments from the `free` function) - // Allocated segment encloses segment exactly: - // - If segment to the left is a span and segment to the right is a span or doesn't exist, then change segment to free - let a = arena.alloc(4096); + // Prev and next both non-free + // Change curr_tag to free + let a1 = arena.alloc(4096); + assert_eq!(arena.free_tags.len(), 99); assert_tags_eq( &arena, &[ @@ -696,18 +745,46 @@ mod tests { Tag::new(TagType::Free, Boundary::new(4096 * 2, 4096 * 19).unwrap()), ], ); - arena.free(a); + arena.free(a1); + assert_eq!(arena.free_tags.len(), 100); + assert_tags_eq( + &arena, + &[ + Tag::new(TagType::Span, Boundary::new(4096, 4096 * 20).unwrap()), + Tag::new(TagType::Free, Boundary::new(4096, 4096 * 20).unwrap()), + ], + ); - // - If segment to the left is free and segment to the right is a span, or doesn't exist then change left segment size + // Prev non-free, next free + // Change next tag start to merge with curr_tag, release curr_tag + let a1 = arena.alloc(4096); + let _ = arena.alloc(4096); + assert_eq!(arena.free_tags.len(), 98); + assert_tags_eq( + &arena, + &[ + Tag::new(TagType::Span, Boundary::new(4096, 4096 * 20).unwrap()), + Tag::new(TagType::Allocated, Boundary::new(4096, 4096).unwrap()), + Tag::new(TagType::Allocated, Boundary::new(4096 * 2, 4096).unwrap()), + Tag::new(TagType::Free, Boundary::new(4096 * 3, 4096 * 18).unwrap()), + ], + ); + arena.free(a1); + assert_eq!(arena.free_tags.len(), 98); + assert_tags_eq( + &arena, + &[ + Tag::new(TagType::Span, Boundary::new(4096, 4096 * 20).unwrap()), + Tag::new(TagType::Free, Boundary::new(4096, 4096).unwrap()), + Tag::new(TagType::Allocated, Boundary::new(4096 * 2, 4096).unwrap()), + Tag::new(TagType::Free, Boundary::new(4096 * 3, 4096 * 18).unwrap()), + ], + ); - // Allocated segment encloses segment to free with space to the left and right: - // - Split existing segment, create new tag for free segment, new tag for allocated segment to the right + // Prev free, next non-free + // Change prev tag size to merge with curr_tag, release curr_tag - // Allocated segment starts before but ends at same point of segment to free: - // - If the segment to the right is a span, insert a new free segment. - // - If the segment to the right is free, merge by changing start of that segment. - // Allocated segment starts at same point of segment to free but ends after; - // - If the segment to the left is a span, insert a new free segment. - // - If the segment to the left is free, merge by changing end of that segment. + // Prev and next both free + // Change prev size to merge with both curr_tag and next, release curr_tag } }