From 23b3028e155d03ee6a77f0c5bd5719bb8b1642ba Mon Sep 17 00:00:00 2001 From: Bananymous Date: Wed, 8 Mar 2023 03:21:30 +0200 Subject: [PATCH] Kernel: Rename RefCounted -> RefPtr and implement RefCounted --- BAN/include/BAN/Memory.h | 146 +++++++------------ kernel/include/kernel/FS/Ext2.h | 8 +- kernel/include/kernel/FS/FileSystem.h | 2 +- kernel/include/kernel/FS/Inode.h | 6 +- kernel/include/kernel/FS/VirtualFileSystem.h | 6 +- kernel/include/kernel/Scheduler.h | 14 +- kernel/include/kernel/Thread.h | 9 +- kernel/kernel/FS/Ext2.cpp | 14 +- kernel/kernel/FS/VirtualFileSystem.cpp | 2 +- kernel/kernel/Scheduler.cpp | 12 +- kernel/kernel/Shell.cpp | 8 +- kernel/kernel/Thread.cpp | 4 +- 12 files changed, 98 insertions(+), 133 deletions(-) diff --git a/BAN/include/BAN/Memory.h b/BAN/include/BAN/Memory.h index ded79b8b..b9cb44ab 100644 --- a/BAN/include/BAN/Memory.h +++ b/BAN/include/BAN/Memory.h @@ -24,143 +24,107 @@ namespace BAN #endif template - class Unique + class RefCounted { - BAN_NON_COPYABLE(Unique); + BAN_NON_COPYABLE(RefCounted); + BAN_NON_MOVABLE(RefCounted); public: - template - Unique(const Args&... args) + uint32_t ref_count() const { - m_pointer = new T(args...); + return m_ref_count; } - ~Unique() + void ref() const { - delete m_pointer; + ASSERT(m_ref_count > 0); + m_ref_count++; } - operator bool() const + void unref() const { - return m_pointer; + ASSERT(m_ref_count > 0); + m_ref_count--; + if (m_ref_count == 0) + delete (const T*)this; } + protected: + RefCounted() = default; + ~RefCounted() { ASSERT(m_ref_count == 0); } + private: - T* m_pointer = nullptr; + mutable uint32_t m_ref_count = 1; }; template - class RefCounted + class RefPtr { public: - RefCounted() = default; - RefCounted(const RefCounted& other) - { - *this = other; - } - RefCounted(RefCounted&& other) - { - *this = move(other); - } - ~RefCounted() - { - clear(); - } - + RefPtr() = default; + ~RefPtr() { clear(); } + template - static ErrorOr> adopt(U* data) + static RefPtr adopt(U* pointer) { - uint32_t* count = new uint32_t(1); - if (!count) - return Error::from_errno(ENOMEM); - return RefCounted((T*)data, count); + return RefPtr(pointer); } template - static ErrorOr> create(Args... args) + static ErrorOr create(Args&&... args) { - uint32_t* count = new uint32_t(1); - if (!count) + T* pointer = new T(forward(args)...); + if (pointer == nullptr) return Error::from_errno(ENOMEM); - T* data = new T(forward(args)...); - if (!data) - { - delete count; - return Error::from_errno(ENOMEM); - } - return RefCounted(data, count); + return RefPtr(pointer); } - RefCounted& operator=(const RefCounted& other) + RefPtr(const RefPtr& other) { *this = other; } + RefPtr(RefPtr&& other) { *this = move(other); } + + RefPtr& operator=(const RefPtr& other) { clear(); - if (other) - { - m_pointer = other.m_pointer; - m_count = other.m_count; - (*m_count)++; - } - return *this; - } - - RefCounted& operator=(RefCounted&& other) - { - clear(); - if (other) - { - m_pointer = other.m_pointer; - m_count = other.m_count; - other.m_pointer = nullptr; - other.m_count = nullptr; - } + m_pointer = other.m_pointer; + if (m_pointer) + m_pointer->ref(); return *this; } - T* ptr() { return m_pointer; } - const T* ptr() const { return m_pointer; } + RefPtr& operator=(RefPtr&& other) + { + clear(); + m_pointer = other.m_pointer; + other.m_pointer = nullptr; + return *this; + } - T& operator*() { return *ptr();} - const T& operator*() const { return *ptr();} + T* ptr() { ASSERT(!empty()); return m_pointer; } + const T* ptr() const { ASSERT(!empty()); return m_pointer; } + + T& operator*() { return *ptr(); } + const T& operator*() const { return *ptr(); } T* operator->() { return ptr(); } const T* operator->() const { return ptr(); } + bool empty() const { return m_pointer == nullptr; } + operator bool() const { return m_pointer; } + void clear() { - if (!*this) - return; - - (*m_count)--; - if (*m_count == 0) - { - delete m_pointer; - delete m_count; - } - + if (m_pointer) + m_pointer->unref(); m_pointer = nullptr; - m_count = nullptr; } - operator bool() const - { - if (!m_count && !m_pointer) - return false; - ASSERT(m_count && m_pointer); - ASSERT(*m_count > 0); - return true; - } - private: - RefCounted(T* pointer, uint32_t* count) + RefPtr(T* pointer) : m_pointer(pointer) - , m_count(count) - { - ASSERT(!pointer == !count); - } + {} private: - T* m_pointer = nullptr; - uint32_t* m_count = nullptr; + T* m_pointer = nullptr; }; } diff --git a/kernel/include/kernel/FS/Ext2.h b/kernel/include/kernel/FS/Ext2.h index 0931b9bc..88867257 100644 --- a/kernel/include/kernel/FS/Ext2.h +++ b/kernel/include/kernel/FS/Ext2.h @@ -131,8 +131,8 @@ namespace Kernel virtual BAN::StringView name() const override { return m_name; } virtual BAN::ErrorOr> read_all() override; - virtual BAN::ErrorOr>> directory_inodes() override; - virtual BAN::ErrorOr> directory_find(BAN::StringView) override; + virtual BAN::ErrorOr>> directory_inodes() override; + virtual BAN::ErrorOr> directory_find(BAN::StringView) override; private: BAN::ErrorOr for_each_block(BAN::Function(const BAN::Vector&)>&); @@ -158,7 +158,7 @@ namespace Kernel public: static BAN::ErrorOr create(StorageDevice::Partition&); - virtual const BAN::RefCounted root_inode() const override { return m_root_inode; } + virtual const BAN::RefPtr root_inode() const override { return m_root_inode; } private: Ext2FS(StorageDevice::Partition& partition) @@ -179,7 +179,7 @@ namespace Kernel private: StorageDevice::Partition& m_partition; - BAN::RefCounted m_root_inode; + BAN::RefPtr m_root_inode; Ext2::Superblock m_superblock; BAN::Vector m_block_group_descriptors; diff --git a/kernel/include/kernel/FS/FileSystem.h b/kernel/include/kernel/FS/FileSystem.h index 52e5c588..f188d224 100644 --- a/kernel/include/kernel/FS/FileSystem.h +++ b/kernel/include/kernel/FS/FileSystem.h @@ -9,7 +9,7 @@ namespace Kernel class FileSystem { public: - virtual const BAN::RefCounted root_inode() const = 0; + virtual const BAN::RefPtr root_inode() const = 0; }; } \ No newline at end of file diff --git a/kernel/include/kernel/FS/Inode.h b/kernel/include/kernel/FS/Inode.h index d51c569a..68b6d23b 100644 --- a/kernel/include/kernel/FS/Inode.h +++ b/kernel/include/kernel/FS/Inode.h @@ -8,7 +8,7 @@ namespace Kernel { - class Inode + class Inode : public BAN::RefCounted { public: union Mode @@ -48,8 +48,8 @@ namespace Kernel virtual BAN::StringView name() const = 0; virtual BAN::ErrorOr> read_all() = 0; - virtual BAN::ErrorOr>> directory_inodes() = 0; - virtual BAN::ErrorOr> directory_find(BAN::StringView) = 0; + virtual BAN::ErrorOr>> directory_inodes() = 0; + virtual BAN::ErrorOr> directory_find(BAN::StringView) = 0; }; } \ No newline at end of file diff --git a/kernel/include/kernel/FS/VirtualFileSystem.h b/kernel/include/kernel/FS/VirtualFileSystem.h index 6725b45a..bb6d1895 100644 --- a/kernel/include/kernel/FS/VirtualFileSystem.h +++ b/kernel/include/kernel/FS/VirtualFileSystem.h @@ -13,16 +13,16 @@ namespace Kernel static VirtualFileSystem& get(); static bool is_initialized(); - virtual const BAN::RefCounted root_inode() const override { return m_root_inode; } + virtual const BAN::RefPtr root_inode() const override { return m_root_inode; } - BAN::ErrorOr> from_absolute_path(BAN::StringView); + BAN::ErrorOr> from_absolute_path(BAN::StringView); private: VirtualFileSystem() = default; BAN::ErrorOr initialize_impl(); private: - BAN::RefCounted m_root_inode; + BAN::RefPtr m_root_inode; BAN::Vector m_storage_controllers; }; diff --git a/kernel/include/kernel/Scheduler.h b/kernel/include/kernel/Scheduler.h index aedd7e14..64eb4055 100644 --- a/kernel/include/kernel/Scheduler.h +++ b/kernel/include/kernel/Scheduler.h @@ -16,7 +16,7 @@ namespace Kernel void start(); void reschedule(); - BAN::ErrorOr add_thread(BAN::RefCounted); + BAN::ErrorOr add_thread(BAN::RefPtr); void set_current_thread_sleeping(uint64_t); [[noreturn]] void set_current_thread_done(); @@ -24,7 +24,7 @@ namespace Kernel private: Scheduler() = default; - BAN::RefCounted current_thread(); + BAN::RefPtr current_thread(); void wake_threads(); [[nodiscard]] bool save_current_thread(); @@ -34,17 +34,17 @@ namespace Kernel private: struct ActiveThread { - BAN::RefCounted thread; - uint64_t padding; + BAN::RefPtr thread; + uint64_t padding = 0; }; struct SleepingThread { - BAN::RefCounted thread; - uint64_t wake_delta; + BAN::RefPtr thread; + uint64_t wake_time; }; - BAN::RefCounted m_idle_thread; + BAN::RefPtr m_idle_thread; BAN::LinkedList m_active_threads; BAN::LinkedList m_sleeping_threads; diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index 9b6678e3..c720e85d 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -6,13 +6,10 @@ namespace Kernel { - class Thread + class Thread : public BAN::RefCounted { - BAN_NON_COPYABLE(Thread); - BAN_NON_MOVABLE(Thread); - public: - static BAN::ErrorOr> create(const BAN::Function&); + static BAN::ErrorOr> create(const BAN::Function&); ~Thread(); uint32_t tid() const { return m_tid; } @@ -40,7 +37,7 @@ namespace Kernel BAN::Function m_function; - friend class BAN::RefCounted; + friend class BAN::RefPtr; }; } \ No newline at end of file diff --git a/kernel/kernel/FS/Ext2.cpp b/kernel/kernel/FS/Ext2.cpp index 31d0935e..203ef308 100644 --- a/kernel/kernel/FS/Ext2.cpp +++ b/kernel/kernel/FS/Ext2.cpp @@ -253,12 +253,12 @@ namespace Kernel return data_buffer; } - BAN::ErrorOr> Ext2Inode::directory_find(BAN::StringView file_name) + BAN::ErrorOr> Ext2Inode::directory_find(BAN::StringView file_name) { if (!ifdir()) return BAN::Error::from_errno(ENOTDIR); - BAN::RefCounted result; + BAN::RefPtr result; BAN::Function(const BAN::Vector&)> function( [&](const BAN::Vector& block_data) -> BAN::ErrorOr { @@ -273,7 +273,7 @@ namespace Kernel Ext2Inode* inode = new Ext2Inode(m_fs, TRY(m_fs->read_inode(entry->inode)), entry_name); if (inode == nullptr) return BAN::Error::from_errno(ENOMEM); - result = TRY(BAN::RefCounted::adopt(inode)); + result = BAN::RefPtr::adopt(inode); return false; } entry_addr += entry->rec_len; @@ -288,12 +288,12 @@ namespace Kernel return BAN::Error::from_errno(ENOENT); } - BAN::ErrorOr>> Ext2Inode::directory_inodes() + BAN::ErrorOr>> Ext2Inode::directory_inodes() { if (!ifdir()) return BAN::Error::from_errno(ENOTDIR); - BAN::Vector> inodes; + BAN::Vector> inodes; BAN::Function(const BAN::Vector&)> function( [&](const BAN::Vector& block_data) -> BAN::ErrorOr { @@ -310,7 +310,7 @@ namespace Kernel Ext2Inode* inode = new Ext2Inode(m_fs, BAN::move(current_inode), entry_name); if (inode == nullptr) return BAN::Error::from_errno(ENOMEM); - TRY(inodes.push_back(TRY(BAN::RefCounted::adopt(inode)))); + TRY(inodes.push_back(BAN::RefPtr::adopt(inode))); } entry_addr += entry->rec_len; } @@ -431,7 +431,7 @@ namespace Kernel Ext2Inode* root_inode = new Ext2Inode(this, TRY(read_inode(Ext2::Enum::ROOT_INO)), ""); if (root_inode == nullptr) return BAN::Error::from_errno(ENOMEM); - m_root_inode = TRY(BAN::RefCounted::adopt(root_inode)); + m_root_inode = BAN::RefPtr::adopt(root_inode); #if EXT2_DEBUG_PRINT dprintln("root inode:"); diff --git a/kernel/kernel/FS/VirtualFileSystem.cpp b/kernel/kernel/FS/VirtualFileSystem.cpp index e01e4c5b..b94c5e87 100644 --- a/kernel/kernel/FS/VirtualFileSystem.cpp +++ b/kernel/kernel/FS/VirtualFileSystem.cpp @@ -112,7 +112,7 @@ namespace Kernel return BAN::Error::from_string("Could not locate root partition"); } - BAN::ErrorOr> VirtualFileSystem::from_absolute_path(BAN::StringView path) + BAN::ErrorOr> VirtualFileSystem::from_absolute_path(BAN::StringView path) { if (path.front() != '/') return BAN::Error::from_string("Path must be an absolute path"); diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index bc7d5bee..1624181b 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -46,9 +46,9 @@ namespace Kernel ASSERT_NOT_REACHED(); } - Thread& Scheduler::current_thread() + BAN::RefPtr Scheduler::current_thread() { - return m_current_thread ? *m_current_thread->thread : *m_idle_thread; + return m_current_thread ? m_current_thread->thread : m_idle_thread; } void Scheduler::reschedule() @@ -147,9 +147,9 @@ namespace Kernel } read_rsp(rsp); - auto& current = current_thread(); - current.set_rip(rip); - current.set_rsp(rsp); + auto current = current_thread(); + current->set_rip(rip); + current->set_rsp(rsp); return false; } @@ -157,7 +157,7 @@ namespace Kernel { VERIFY_CLI(); - auto& current = current_thread(); + auto& current = *current_thread(); if (current.started()) { diff --git a/kernel/kernel/Shell.cpp b/kernel/kernel/Shell.cpp index 1c3d9e87..d477b2ef 100644 --- a/kernel/kernel/Shell.cpp +++ b/kernel/kernel/Shell.cpp @@ -182,7 +182,7 @@ argument_done: s_thread_spinlock.lock(); - MUST(Scheduler::get().add_thread(MUST(Thread::create( + auto thread_or_error = Thread::create( [this, &arguments] { auto args = arguments; @@ -191,7 +191,11 @@ argument_done: PIT::sleep(5000); process_command(args); } - )))); + ); + if (thread_or_error.is_error()) + return TTY_PRINTLN("{}", thread_or_error.error()); + + MUST(Scheduler::get().add_thread(thread)); while (s_thread_spinlock.is_locked()); } diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index 764e7942..c3a70388 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -22,9 +22,9 @@ namespace Kernel } - BAN::ErrorOr> Thread::create(const BAN::Function& function) + BAN::ErrorOr> Thread::create(const BAN::Function& function) { - return BAN::RefCounted::create(function); + return BAN::RefPtr::create(function); } Thread::Thread(const BAN::Function& function)