Kernel: Create CriticalScope and fix kmalloc

This disables interrupts for the current scope and restores them
after the scope. This is used in kmalloc, since scheduler might
call into kmalloc/kfree, but deadlock if some thread is currently
trying to allocate. This allows us to use kmalloc in Scheduler.
This commit is contained in:
Bananymous 2023-03-08 13:51:09 +02:00
parent a068d828fe
commit d90aba0963
8 changed files with 42 additions and 40 deletions

View File

@ -0,0 +1,30 @@
#pragma once
#include <BAN/NoCopyMove.h>
#include <stddef.h>
namespace Kernel
{
class CriticalScope
{
BAN_NON_COPYABLE(CriticalScope);
BAN_NON_MOVABLE(CriticalScope);
public:
CriticalScope()
{
asm volatile("pushf; cli; pop %0" : "=r"(m_flags) :: "memory");
}
~CriticalScope()
{
asm volatile("push %0; popf" :: "rm"(m_flags) : "memory", "cc");
}
private:
size_t m_flags;
};
}

View File

@ -18,6 +18,4 @@ public:
static InterruptController& get(); static InterruptController& get();
}; };
uintptr_t disable_interrupts_and_get_flags();
void restore_flags(uintptr_t);
bool interrupts_enabled(); bool interrupts_enabled();

View File

@ -26,18 +26,6 @@ void InterruptController::initialize(bool force_pic)
s_instance = PIC::create(); s_instance = PIC::create();
} }
uintptr_t disable_interrupts_and_get_flags()
{
uintptr_t flags;
asm volatile("pushf; cli; pop %0" : "=r"(flags) :: "memory");
return flags;
}
void restore_flags(uintptr_t flags)
{
asm volatile("push %0; popf" :: "rm"(flags) : "memory", "cc");
}
bool interrupts_enabled() bool interrupts_enabled()
{ {
uintptr_t flags; uintptr_t flags;

View File

@ -1,10 +1,9 @@
#include <kernel/Arch.h> #include <kernel/Arch.h>
#include <kernel/Attributes.h> #include <kernel/Attributes.h>
#include <kernel/CriticalScope.h>
#include <kernel/InterruptController.h> #include <kernel/InterruptController.h>
#include <kernel/Scheduler.h> #include <kernel/Scheduler.h>
#include <kernel/PCI.h>
#if 1 #if 1
#define VERIFY_STI() ASSERT(interrupts_enabled()) #define VERIFY_STI() ASSERT(interrupts_enabled())
#define VERIFY_CLI() ASSERT(!interrupts_enabled()) #define VERIFY_CLI() ASSERT(!interrupts_enabled())
@ -89,10 +88,9 @@ namespace Kernel
BAN::ErrorOr<void> Scheduler::add_thread(BAN::RefPtr<Thread> thread) BAN::ErrorOr<void> Scheduler::add_thread(BAN::RefPtr<Thread> thread)
{ {
auto flags = disable_interrupts_and_get_flags(); Kernel::CriticalScope critical;
BAN::ErrorOr<void> result = m_active_threads.emplace_back(thread); TRY(m_active_threads.emplace_back(thread));
restore_flags(flags); return {};
return result;
} }
void Scheduler::advance_current_thread() void Scheduler::advance_current_thread()

View File

@ -195,7 +195,7 @@ argument_done:
if (thread_or_error.is_error()) if (thread_or_error.is_error())
return TTY_PRINTLN("{}", thread_or_error.error()); return TTY_PRINTLN("{}", thread_or_error.error());
MUST(Scheduler::get().add_thread(thread)); MUST(Scheduler::get().add_thread(thread_or_error.release_value()));
while (s_thread_spinlock.is_locked()); while (s_thread_spinlock.is_locked());
} }

View File

@ -47,6 +47,7 @@ namespace Kernel
Thread::~Thread() Thread::~Thread()
{ {
dprintln("thread {} destruct", tid());
kfree(m_stack_base); kfree(m_stack_base);
} }

View File

@ -103,7 +103,6 @@ extern "C" void kernel_main()
MUST(Scheduler::initialize()); MUST(Scheduler::initialize());
Scheduler& scheduler = Scheduler::get(); Scheduler& scheduler = Scheduler::get();
#if 1
MUST(scheduler.add_thread(MUST(Thread::create( MUST(scheduler.add_thread(MUST(Thread::create(
[terminal_driver] [terminal_driver]
{ {
@ -120,7 +119,6 @@ extern "C" void kernel_main()
terminal_driver->set_font(font_or_error.release_value()); terminal_driver->set_font(font_or_error.release_value());
} }
)))); ))));
#endif
MUST(scheduler.add_thread(MUST(Thread::create( MUST(scheduler.add_thread(MUST(Thread::create(
[tty1] [tty1]
{ {

View File

@ -1,21 +1,13 @@
#include <BAN/Errors.h> #include <BAN/Errors.h>
#include <BAN/Math.h> #include <kernel/CriticalScope.h>
#include <kernel/kmalloc.h> #include <kernel/kmalloc.h>
#include <kernel/kprint.h> #include <kernel/kprint.h>
#include <kernel/multiboot.h> #include <kernel/multiboot.h>
#include <kernel/SpinLock.h>
#include <kernel/LockGuard.h>
#include <stdint.h>
#define MB (1 << 20) #define MB (1 << 20)
static constexpr size_t s_kmalloc_min_align = alignof(max_align_t); static constexpr size_t s_kmalloc_min_align = alignof(max_align_t);
static Kernel::SpinLock s_general_lock;
static Kernel::SpinLock s_fixed_lock;
struct kmalloc_node struct kmalloc_node
{ {
void set_align(ptrdiff_t align) { m_align = align; } void set_align(ptrdiff_t align) { m_align = align; }
@ -192,8 +184,6 @@ void kmalloc_dump_info()
static void* kmalloc_fixed() static void* kmalloc_fixed()
{ {
Kernel::LockGuard guard(s_fixed_lock);
auto& info = s_kmalloc_fixed_info; auto& info = s_kmalloc_fixed_info;
if (!info.free_list_head) if (!info.free_list_head)
@ -233,8 +223,6 @@ static void* kmalloc_fixed()
static void* kmalloc_impl(size_t size, size_t align) static void* kmalloc_impl(size_t size, size_t align)
{ {
Kernel::LockGuard guard(s_general_lock);
ASSERT(align % s_kmalloc_min_align == 0); ASSERT(align % s_kmalloc_min_align == 0);
ASSERT(size % s_kmalloc_min_align == 0); ASSERT(size % s_kmalloc_min_align == 0);
@ -309,12 +297,15 @@ static constexpr bool is_power_of_two(size_t value)
void* kmalloc(size_t size, size_t align) void* kmalloc(size_t size, size_t align)
{ {
const kmalloc_info& info = s_kmalloc_info; const kmalloc_info& info = s_kmalloc_info;
if (size == 0 || size >= info.size) if (size == 0 || size >= info.size)
return nullptr; return nullptr;
ASSERT(is_power_of_two(align)); ASSERT(is_power_of_two(align));
Kernel::CriticalScope critical;
// if the size fits into fixed node, we will try to use that since it is faster // if the size fits into fixed node, we will try to use that since it is faster
if (align == s_kmalloc_min_align && size <= sizeof(kmalloc_fixed_info::node::data)) if (align == s_kmalloc_min_align && size <= sizeof(kmalloc_fixed_info::node::data))
@ -334,10 +325,10 @@ void kfree(void* address)
uintptr_t address_uint = (uintptr_t)address; uintptr_t address_uint = (uintptr_t)address;
ASSERT(address_uint % s_kmalloc_min_align == 0); ASSERT(address_uint % s_kmalloc_min_align == 0);
Kernel::CriticalScope critical;
if (s_kmalloc_fixed_info.base <= address_uint && address_uint < s_kmalloc_fixed_info.end) if (s_kmalloc_fixed_info.base <= address_uint && address_uint < s_kmalloc_fixed_info.end)
{ {
Kernel::LockGuard guard(s_fixed_lock);
auto& info = s_kmalloc_fixed_info; auto& info = s_kmalloc_fixed_info;
ASSERT(info.used_list_head); ASSERT(info.used_list_head);
@ -369,8 +360,6 @@ void kfree(void* address)
} }
else if (s_kmalloc_info.base <= address_uint && address_uint < s_kmalloc_info.end) else if (s_kmalloc_info.base <= address_uint && address_uint < s_kmalloc_info.end)
{ {
Kernel::LockGuard guard(s_general_lock);
auto& info = s_kmalloc_info; auto& info = s_kmalloc_info;
auto* node = info.from_address(address); auto* node = info.from_address(address);