From 4081186b2791289e68069a9a226e930574112a8c Mon Sep 17 00:00:00 2001 From: apio Date: Fri, 30 Dec 2022 19:02:25 +0100 Subject: [PATCH] Heap: Rewrite kmalloc to use Option instead of nullable pointers to iterate over the heap At some point, this should be done inside LinkedList itself, but we have no such thing as break in for_each(). It's iterate over everything or nothing. This also requires operator= in Option, might be also added to Result in the future. --- kernel/src/memory/Heap.cpp | 64 +++++++++++++++++++--------------- luna/include/luna/LinkedList.h | 4 +-- luna/include/luna/Option.h | 25 +++++++++++++ 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/kernel/src/memory/Heap.cpp b/kernel/src/memory/Heap.cpp index ace511c4..a8ae9c08 100644 --- a/kernel/src/memory/Heap.cpp +++ b/kernel/src/memory/Heap.cpp @@ -199,45 +199,50 @@ Result kmalloc(usize size, bool should_scrub) heap.append(block); } - HeapBlock* block = heap.expect_first(); - while (block) + Option block = heap.first(); + while (block.has_value()) { + HeapBlock* const current = block.value(); // Trying to find a free block... - if (is_block_free(block)) + if (is_block_free(current)) { - if (block->full_size < size) + if (current->full_size < size) { - block = heap.next(block).value_or(nullptr); + block = heap.next(current); continue; } break; // We found a free block that's big enough!! } - auto rc = split(block, size); + auto rc = split(current, size); if (rc.has_value()) { block = rc.value(); // We managed to get a free block from a larger used block!! break; } - block = heap.next(block).value_or(nullptr); + block = heap.next(current); } - if (!block) // No free blocks, let's allocate a new one + if (!block.has_value()) // No free blocks, let's allocate a new one { usize pages = get_pages_for_allocation(size + sizeof(HeapBlock)); - block = TRY(allocate_pages(pages)); + HeapBlock* const current = TRY(allocate_pages(pages)); - block->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock); - block->magic = BLOCK_MAGIC; - block->status = BLOCK_START_MEM | BLOCK_END_MEM; - heap.append(block); + current->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock); + current->magic = BLOCK_MAGIC; + current->status = BLOCK_START_MEM | BLOCK_END_MEM; + heap.append(current); + + block = current; } - block->req_size = size; - block->status |= BLOCK_USED; + HeapBlock* const current = block.value(); - if (should_scrub) { memset(get_pointer_from_heap_block(block), KMALLOC_SCRUB_BYTE, size); } + current->req_size = size; + current->status |= BLOCK_USED; - return get_pointer_from_heap_block(block); + if (should_scrub) { memset(get_pointer_from_heap_block(current), KMALLOC_SCRUB_BYTE, size); } + + return get_pointer_from_heap_block(current); } Result kfree(void* ptr) @@ -361,24 +366,25 @@ void dump_heap_usage() } usize alloc_total = 0; usize alloc_used = 0; - HeapBlock* block = heap.expect_first(); - while (block) + auto block = heap.first(); + while (block.has_value()) { - if (is_block_free(block)) + HeapBlock* current = block.value(); + if (is_block_free(current)) { - kdbgln("- Available block (%p), of size %zu (%s%s)", (void*)block, block->full_size, - block->status & BLOCK_START_MEM ? "b" : "-", block->status & BLOCK_END_MEM ? "e" : "-"); - alloc_total += block->full_size + sizeof(HeapBlock); + kdbgln("- Available block (%p), of size %zu (%s%s)", (void*)current, current->full_size, + current->status & BLOCK_START_MEM ? "b" : "-", current->status & BLOCK_END_MEM ? "e" : "-"); + alloc_total += current->full_size + sizeof(HeapBlock); } else { - kdbgln("- Used block (%p), of size %zu, of which %zu bytes are being used (%s%s)", (void*)block, - block->full_size, block->req_size, block->status & BLOCK_START_MEM ? "b" : "-", - block->status & BLOCK_END_MEM ? "e" : "-"); - alloc_total += block->full_size + sizeof(HeapBlock); - alloc_used += block->req_size; + kdbgln("- Used block (%p), of size %zu, of which %zu bytes are being used (%s%s)", (void*)current, + current->full_size, current->req_size, current->status & BLOCK_START_MEM ? "b" : "-", + current->status & BLOCK_END_MEM ? "e" : "-"); + alloc_total += current->full_size + sizeof(HeapBlock); + alloc_used += current->req_size; } - block = heap.next(block).value_or(nullptr); + block = heap.next(current); } kdbgln("-- Total memory allocated for heap: %zu bytes", alloc_total); diff --git a/luna/include/luna/LinkedList.h b/luna/include/luna/LinkedList.h index c390f986..442e0ecc 100644 --- a/luna/include/luna/LinkedList.h +++ b/luna/include/luna/LinkedList.h @@ -130,7 +130,7 @@ template class LinkedList T* expect_first() { check(m_start_node); - return m_start_node; + return (T*)m_start_node; } Option last() @@ -141,7 +141,7 @@ template class LinkedList T* expect_last() { check(m_end_node); - return m_end_node; + return (T*)m_end_node; } Option next(T* item) diff --git a/luna/include/luna/Option.h b/luna/include/luna/Option.h index 8dcf4521..6bbe8b5c 100644 --- a/luna/include/luna/Option.h +++ b/luna/include/luna/Option.h @@ -30,6 +30,31 @@ template class Option if (m_has_value) { m_storage.store_moved_reference(move(other.m_storage.fetch_reference())); } } + Option& operator=(const Option& other) + { + if (this == &other) return *this; + + if (m_has_value) m_storage.destroy(); + m_has_value = other.m_has_value; + + if (m_has_value) { m_storage.store_reference(other.m_storage.fetch_reference()); } + + return *this; + } + + Option& operator=(Option&& other) + { + if (this == &other) return *this; + + if (m_has_value) m_storage.destroy(); + m_has_value = other.m_has_value; + other.m_has_value = false; + + if (m_has_value) { m_storage.store_moved_reference(move(other.m_storage.fetch_reference())); } + + return *this; + } + Option() : m_has_value(false) { }