From de7a7c1636c5e392fbae7ee4cf0ff517ae8b6567 Mon Sep 17 00:00:00 2001 From: ChUrl Date: Sat, 16 Jul 2022 03:31:12 +0200 Subject: [PATCH] switch logger + unfinished hack --- c_os/kernel/allocator/TreeAllocator.cc | 69 ++++++++++++++------------ c_os/kernel/allocator/TreeAllocator.h | 6 ++- 2 files changed, 41 insertions(+), 34 deletions(-) diff --git a/c_os/kernel/allocator/TreeAllocator.cc b/c_os/kernel/allocator/TreeAllocator.cc index 8c7cc73..bcf6356 100755 --- a/c_os/kernel/allocator/TreeAllocator.cc +++ b/c_os/kernel/allocator/TreeAllocator.cc @@ -10,11 +10,11 @@ void TreeAllocator::init() { this->free_start->left = NULL; this->free_start->right = NULL; this->free_start->parent = NULL; - this->free_start->red = false; // The root is always black + this->free_start->red = false; // The root is always black this->free_start->next = (list_block_t*)this->free_start; this->free_start->previous = (list_block_t*)this->free_start; - if constexpr (DEBUG) { kout << "Initialized Tree Allocator" << endl; } + log << INFO << "Initialized Tree Allocator" << endl; } void TreeAllocator::dump_free_memory() { @@ -29,7 +29,7 @@ void TreeAllocator::dump_free_memory() { } void* TreeAllocator::alloc(unsigned int req_size) { - if constexpr (DEBUG) { kout << "Requested " << dec << req_size << " Bytes" << endl; } + log << DEBUG << "Requested " << dec << req_size << " Bytes" << endl; // Round to word borders + tree_block size unsigned int rreq_size = req_size; @@ -37,33 +37,42 @@ void* TreeAllocator::alloc(unsigned int req_size) { // the list_block_t is part of every block, but when freeing // memory we need enough space to store the rbt metadata rreq_size = sizeof(tree_block_t) - sizeof(list_block_t); - if constexpr (DEBUG) { kout << " - Increased block size for rbt metadata" << endl; } + log << TRACE << " - Increased block size for rbt metadata" << endl; } unsigned int req_size_diff = (BASIC_ALIGN - rreq_size % BASIC_ALIGN) % BASIC_ALIGN; rreq_size = rreq_size + req_size_diff; if (req_size_diff > 0) { - if constexpr (DEBUG) { kout << " - Rounded to word border (+" << dec << req_size_diff << " bytes)" << endl; } + log << TRACE << " - Rounded to word border (+" << dec << req_size_diff << " bytes)" << endl; } // Finds smallest block that is large enough tree_block_t* best_fit = this->rbt_search_bestfit(rreq_size); if (best_fit == NULL) { - if constexpr (DEBUG) { kout << " - No block found" << endl; } + log << ERROR << " - No block found" << endl; return NULL; } if (best_fit->allocated) { // Something went really wrong - if constexpr (DEBUG) { kout << " - Block already allocated :(" << endl; } + log << ERROR << " - Block already allocated :(" << endl; return NULL; } best_fit->allocated = true; unsigned int size = this->get_size(best_fit); - if constexpr (DEBUG) { kout << " - Found best-fit: " << hex << (unsigned int)best_fit << endl; } + log << TRACE << " - Found best-fit: " << hex << (unsigned int)best_fit << endl; - this->rbt_remove(best_fit); // BUG: On first allocation this crashes as the tree root is being removed + // HACK: I didn't want to handle situations with only one block (where the tree root would + // get removed), so I make sure there are always at least 2 blocks by inserting a dummy + // block. This is not fast at all but it was fast to code... + // I should change this so it only happens when only one block exists in the freelist + tree_block_t dummy; + dummy.allocated = false; + this->rbt_insert(&dummy); // I use the address of the stack allocated struct because it is + // removed before exiting the function + + this->rbt_remove(best_fit); // BUG: Can trigger bluescreen if (size > HEAP_MIN_FREE_BLOCK_SIZE + rreq_size + sizeof(list_block_t)) { // Block can be cut - if constexpr (DEBUG) { kout << " - Allocating " << dec << rreq_size << " Bytes with cutting" << endl; } + log << TRACE << " - Allocating " << dec << rreq_size << " Bytes with cutting" << endl; // [best_fit_start | sizeof(list_block_t) | rreq_size | new_block_start] tree_block_t* new_block = (tree_block_t*)((char*)best_fit + sizeof(list_block_t) + rreq_size); @@ -74,34 +83,25 @@ void* TreeAllocator::alloc(unsigned int req_size) { // Don't cut block // The block is already correctly positioned in the linked list so we only // need to remove it from the freelist, which is done for both cases - if constexpr (DEBUG) { kout << " - Allocating " << dec << rreq_size << " Bytes without cutting" << endl; } + log << TRACE << " - Allocating " << dec << rreq_size << " Bytes without cutting" << endl; } - // Remove the old block from the freelist - // NOTE: Originally this was placed before the leading if/else, but the red black tree code crashes - // when removing the tree root (NullPointerException bluescreen) so I moved the removal down. - // This could influence how the above rbt_insertion places the new_block, resulting in a broken - // freelist (unbalanced red black tree, not completely broken), but I didn't have time to fix this - // correctly - // BUG: If the first allocation allocates the whole heap, this removal will crash the paging - // as the freelist will no longer contain any nodes, gives NullPointerException as it's not handled - // in the red black tree code (I think) - // kout << " - Removing block from freelist" << endl; - // this->rbt_remove(best_fit); // HACK + // HACK: Remove the dummy element + this->rbt_remove(&dummy); - if constexpr (DEBUG) { kout << " - Returned address " << hex << (unsigned int)((char*)best_fit + sizeof(list_block_t)) << endl; } + log << TRACE << " - Returned address " << hex << (unsigned int)((char*)best_fit + sizeof(list_block_t)) << endl; return (void*)((char*)best_fit + sizeof(list_block_t)); } void TreeAllocator::free(void* ptr) { - if constexpr (DEBUG) { kout << "Freeing " << hex << (unsigned int)ptr << endl; } + log << INFO << "Freeing " << hex << (unsigned int)ptr << endl; list_block_t* block = (list_block_t*)((char*)ptr - sizeof(list_block_t)); if (!block->allocated) { // Block already free return; } - block->allocated = false; // If the block is merged backwards afterwards this is unnecessary + block->allocated = false; // If the block is merged backwards afterwards this is unnecessary list_block_t* previous = block->previous; list_block_t* next = block->next; @@ -109,28 +109,30 @@ void TreeAllocator::free(void* ptr) { if (next->allocated && previous->allocated) { // No merge this->rbt_insert((tree_block_t*)block); + return; } + // HACK: Same as when allocating + tree_block_t dummy; + dummy.allocated = false; + this->rbt_insert(&dummy); // I use the address of the stack allocated struct because it is + if (!next->allocated) { // Merge forward - if constexpr (DEBUG) { kout << " - Merging forward" << endl; } + log << TRACE << " - Merging forward" << endl; // Remove the next block from all lists as it is now part of our freed block this->dll_remove(next); - this->rbt_remove((tree_block_t*)next); // BUG: Bluescreen if next is the only block in the freelist + this->rbt_remove((tree_block_t*)next); // BUG: Bluescreen if next is the only block in the freelist if (previous->allocated) { // Don't insert if removed later because of backward merge this->rbt_insert((tree_block_t*)block); } - // NOTE: Originally this was placed before the leading if (after this->dll_remove(next)), - // but the red black tree code crashes when removing the tree node (like on insertion). - // Placing this here could lead to an unbalanced freelist. - // this->rbt_remove((tree_block_t*)next); // HACK } if (!previous->allocated) { // Merge backward - if constexpr (DEBUG) { kout << " - Merging backward" << endl; } + log << TRACE << " - Merging backward" << endl; // Remove the current block from all lists as it is now part of the previous block // It doesn't have to be removed from rbt as it wasn't in there as it was allocated before @@ -138,6 +140,9 @@ void TreeAllocator::free(void* ptr) { this->rbt_remove((tree_block_t*)previous); this->rbt_insert((tree_block_t*)previous); // Reinsert with new size } + + // HACK: Same as when allocating + this->rbt_remove(&dummy); } unsigned int TreeAllocator::get_size(list_block_t* block) const { diff --git a/c_os/kernel/allocator/TreeAllocator.h b/c_os/kernel/allocator/TreeAllocator.h index c2d46b4..fa03a0e 100755 --- a/c_os/kernel/allocator/TreeAllocator.h +++ b/c_os/kernel/allocator/TreeAllocator.h @@ -2,6 +2,7 @@ #define __TreeAllocator_include__ #include "kernel/Allocator.h" +#include "user/lib/Logger.h" // NOTE: I added this file // NOTE: I can't imagine that this is very fast with all the tree logic? @@ -32,11 +33,12 @@ typedef struct tree_block { } tree_block_t; class TreeAllocator : Allocator { - private: // Root of the rbt tree_block_t* free_start; + Logger log; + TreeAllocator(Allocator& copy) = delete; // Verhindere Kopieren // Returns the size of the usable memory of a block @@ -66,7 +68,7 @@ private: void dll_remove(list_block_t* node); public: - TreeAllocator() {}; + TreeAllocator() : log("RBT-Alloc") {}; void init() override; void dump_free_memory() override;