From 09e447d9d2e0fd519a932f14d4aa800b83b7c88d Mon Sep 17 00:00:00 2001 From: apio Date: Tue, 6 Dec 2022 18:28:04 +0100 Subject: [PATCH] Heap: Use LinkedList instead of doing things manually --- kernel/src/memory/Heap.cpp | 73 +++++++++++++++----------------------- 1 file changed, 28 insertions(+), 45 deletions(-) diff --git a/kernel/src/memory/Heap.cpp b/kernel/src/memory/Heap.cpp index b77a0646..d0f65e5d 100644 --- a/kernel/src/memory/Heap.cpp +++ b/kernel/src/memory/Heap.cpp @@ -5,6 +5,7 @@ #include "memory/KernelVM.h" #include "memory/MemoryManager.h" #include +#include #include #include #include @@ -18,20 +19,17 @@ static constexpr usize BLOCK_DEAD = 0xdeaddeaddeaddead; static constexpr usize MINIMUM_PAGES_PER_ALLOCATION = 4; -struct HeapBlock +struct HeapBlock : DoublyLinkedListNode { usize req_size; usize full_size; int status; - HeapBlock* next; - HeapBlock* last; usize magic; }; static_assert(sizeof(HeapBlock) == 48UL); -static HeapBlock* heap_start = nullptr; -static HeapBlock* heap_end = nullptr; +static DoublyLinkedList heap; static Result allocate_pages(usize count) { @@ -96,7 +94,8 @@ static Result split(HeapBlock* block, usize size) const usize old_size = block->full_size; // Save the old value of this variable since we are going to use it after modifying it - if (available < (size + sizeof(HeapBlock))) return err(0); // This error is not propagated. + if (available < (size + sizeof(HeapBlock))) + return err(ENONE); // This block hasn't got enough free space to hold the requested size. const usize offset = get_fair_offset_to_split_at(block, size + sizeof(HeapBlock)); block->full_size = offset; // shrink the old block to fit this offset @@ -106,24 +105,20 @@ static Result split(HeapBlock* block, usize size) new_block->magic = BLOCK_MAGIC; new_block->status = (block->status & BLOCK_END_MEM) ? BLOCK_END_MEM : 0; new_block->full_size = old_size - (offset + sizeof(HeapBlock)); - new_block->next = block->next; - new_block->last = block; + heap.append_after(block, new_block); - block->status &= ~BLOCK_END_MEM; // this block is no longer the last block in this memory range - block->next = new_block; + block->status &= ~BLOCK_END_MEM; // this block is no longer the last block in its memory range return new_block; } static Result combine_forward(HeapBlock* block) { - HeapBlock* const next = block->next; - if (next == heap_end) heap_end = block; + // The caller needs to ensure there is a next block. + HeapBlock* const next = heap.next(block).value(); + heap.remove(next); next->magic = BLOCK_DEAD; - block->next = block->next->next; - if (block->next) block->next->last = block; - if (next->status & BLOCK_END_MEM) { if (next->status & BLOCK_START_MEM) @@ -142,13 +137,11 @@ static Result combine_forward(HeapBlock* block) static Result combine_backward(HeapBlock* block) { - HeapBlock* const last = block->last; - if (block == heap_end) heap_end = last; + // The caller needs to ensure there is a last block. + HeapBlock* const last = heap.previous(block).value(); + heap.remove(block); block->magic = BLOCK_DEAD; - last->next = block->next; - if (last->next) last->next->last = last; - if (block->status & BLOCK_END_MEM) { if (block->status & BLOCK_START_MEM) @@ -171,7 +164,7 @@ Result kmalloc(usize size) size = align_up(size, 16UL); - if (!heap_start) + if (!heap.first().has_value()) { const usize pages = get_pages_for_allocation(size + sizeof(HeapBlock)); HeapBlock* const block = TRY(allocate_pages(pages)); @@ -179,15 +172,10 @@ Result kmalloc(usize size) block->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock); block->magic = BLOCK_MAGIC; block->status = BLOCK_START_MEM | BLOCK_END_MEM; - block->next = block->last = nullptr; - heap_start = block; - - check(!heap_end); - - heap_end = heap_start; + heap.append(block); } - HeapBlock* block = heap_start; + HeapBlock* block = heap.first().value(); while (block) { // Trying to find a free block... @@ -195,7 +183,7 @@ Result kmalloc(usize size) { if (block->full_size < size) { - block = block->next; // Let's not try to split this block, it's not big enough + block = heap.next(block).value_or(nullptr); continue; } break; // We found a free block that's big enough!! @@ -203,10 +191,10 @@ Result kmalloc(usize size) auto rc = split(block, size); if (rc.has_value()) { - block = rc.release_value(); // We managed to get a free block from a larger used block!! + block = rc.value(); // We managed to get a free block from a larger used block!! break; } - block = block->next; + block = heap.next(block).value_or(nullptr); } if (!block) // No free blocks, let's allocate a new one @@ -217,11 +205,7 @@ Result kmalloc(usize size) block->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock); block->magic = BLOCK_MAGIC; block->status = BLOCK_START_MEM | BLOCK_END_MEM; - block->next = nullptr; - block->last = heap_end; - heap_end->next = block; - - heap_end = block; + heap.append(block); } block->req_size = size; @@ -257,13 +241,15 @@ Result kfree(void* ptr) else block->status &= ~BLOCK_USED; - if (block->next && is_block_free(block->next)) + auto maybe_next = heap.next(block); + if (maybe_next.has_value() && is_block_free(maybe_next.value())) { // The next block is also free, thus we can merge! TRY(combine_forward(block)); } - if (block->last && is_block_free(block->last)) + auto maybe_last = heap.previous(block); + if (maybe_last.has_value() && is_block_free(maybe_last.value())) { // The last block is also free, thus we can merge! block = TRY(combine_backward(block)); @@ -271,10 +257,7 @@ Result kfree(void* ptr) if ((block->status & BLOCK_START_MEM) && (block->status & BLOCK_END_MEM)) { - if (block == heap_start) heap_start = block->next; - if (block == heap_end) heap_end = block->last; - if (block->last) block->last->next = block->next; - if (block->next) block->next->last = block->last; + heap.remove(block); TRY(release_pages(block, get_blocks_from_size(block->full_size + sizeof(HeapBlock), ARCH_PAGE_SIZE))); } @@ -337,14 +320,14 @@ Result kcalloc(usize nmemb, usize size) void dump_heap_usage() { kdbgln("-- Dumping usage stats for kernel heap:"); - if (!heap_start) + if (!heap.count()) { kdbgln("- Heap is not currently being used"); return; } usize alloc_total = 0; usize alloc_used = 0; - HeapBlock* block = heap_start; + HeapBlock* block = heap.first().value(); while (block) { if (is_block_free(block)) @@ -358,7 +341,7 @@ void dump_heap_usage() alloc_total += block->full_size + sizeof(HeapBlock); alloc_used += block->req_size; } - block = block->next; + block = heap.next(block).value_or(nullptr); } kdbgln("-- Total memory allocated for heap: %zu bytes", alloc_total);