Merge pull request #31 from fmckeogh/master

Fix incorrect attempted merging at maximum order

Author: jiegec

Cargo.toml

@@ -20,8 +20,8 @@ version = "0.9.8"
 optional = true
 
 [dev-dependencies]
-criterion = "0.3"
-ctor = "0.1.23"
+criterion = "0.5.1"
+ctor = "0.2.6"
 rand = "0.8.5"
 rand_chacha = "0.3.1"
 

benches/memory_allocator_benchmark.rs

@@ -12,6 +12,7 @@ use alloc::alloc::GlobalAlloc;
 use alloc::alloc::Layout;
 use buddy_system_allocator::LockedHeap;
 use criterion::{black_box, criterion_group, criterion_main, Criterion};
+use rand::{Rng, SeedableRng};
 
 const SMALL_SIZE: usize = 8;
 const LARGE_SIZE: usize = 1024 * 1024; // 1M
@@ -42,10 +43,6 @@ pub fn large_alloc<const ORDER: usize>(heap: &LockedHeap<ORDER>) {
 pub fn mutil_thread_random_size<const ORDER: usize>(heap: &'static LockedHeap<ORDER>) {
     const THREAD_SIZE: usize = 10;
 
-    use rand::prelude::*;
-    use rand::{Rng, SeedableRng};
-    use rand_chacha::ChaCha8Rng;
-
     let mut threads = Vec::with_capacity(THREAD_SIZE);
     let alloc = Arc::new(heap);
     for i in 0..THREAD_SIZE {

src/lib.rs

@@ -157,7 +157,8 @@ impl<const ORDER: usize> Heap<ORDER> {
             // Merge free buddy lists
             let mut current_ptr = ptr.as_ptr() as usize;
             let mut current_class = class;
-            while current_class < self.free_list.len() {
+
+            while current_class < self.free_list.len() - 1 {
                 let buddy = current_ptr ^ (1 << current_class);
                 let mut flag = false;
                 for block in self.free_list[current_class].iter_mut() {
@@ -261,7 +262,7 @@ unsafe impl<const ORDER: usize> GlobalAlloc for LockedHeap<ORDER> {
             .lock()
             .alloc(layout)
             .ok()
-            .map_or(0 as *mut u8, |allocation| allocation.as_ptr())
+            .map_or(core::ptr::null_mut(), |allocation| allocation.as_ptr())
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
@@ -327,7 +328,7 @@ unsafe impl<const ORDER: usize> GlobalAlloc for LockedHeapWithRescue<ORDER> {
                 inner
                     .alloc(layout)
                     .ok()
-                    .map_or(0 as *mut u8, |allocation| allocation.as_ptr())
+                    .map_or(core::ptr::null_mut(), |allocation| allocation.as_ptr())
             }
         }
     }

src/test.rs

@@ -192,3 +192,34 @@ fn test_frame_allocator_aligned() {
         Some(16)
     );
 }
+
+#[test]
+fn test_heap_merge_final_order() {
+    const NUM_ORDERS: usize = 5;
+
+    let backing_size = 1 << NUM_ORDERS;
+    let backing_layout = Layout::from_size_align(backing_size, backing_size).unwrap();
+
+    // create a new heap with 5 orders
+    let mut heap = Heap::<NUM_ORDERS>::new();
+
+    // allocate host memory for use by heap
+    let backing_allocation = unsafe { std::alloc::alloc(backing_layout) };
+
+    let start = backing_allocation as usize;
+    let middle = unsafe { backing_allocation.add(backing_size / 2) } as usize;
+    let end = unsafe { backing_allocation.add(backing_size) } as usize;
+
+    // add two contiguous ranges of memory
+    unsafe { heap.add_to_heap(start, middle) };
+    unsafe { heap.add_to_heap(middle, end) };
+
+    // NUM_ORDERS - 1 is the maximum order of the heap
+    let layout = Layout::from_size_align(1 << (NUM_ORDERS - 1), 1).unwrap();
+
+    // allocation should succeed, using one of the added ranges
+    let alloc = heap.alloc(layout).unwrap();
+
+    // deallocation should not attempt to merge the two contiguous ranges as the next order does not exist
+    heap.dealloc(alloc, layout);
+}