From fd94856d3cee411e061d9462b016d8a0394a6ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 27 Sep 2023 11:20:34 +0200 Subject: [PATCH] Revert "fix(core_local): put scheduler into RefCell" This reverts commit 8cbf7f6610009d2f779883120143e98f824ff066. --- src/arch/aarch64/kernel/core_local.rs | 17 ++++++++-------- src/arch/x86_64/kernel/apic.rs | 2 +- src/arch/x86_64/kernel/core_local.rs | 14 ++++++------- src/scheduler/mod.rs | 29 +++++++++++++-------------- src/synch/futex.rs | 18 ++++++++--------- src/synch/recmutex.rs | 4 ++-- src/synch/semaphore.rs | 2 +- src/syscalls/tasks.rs | 4 ++-- 8 files changed, 42 insertions(+), 48 deletions(-) diff --git a/src/arch/aarch64/kernel/core_local.rs b/src/arch/aarch64/kernel/core_local.rs index 44d4dffede..a700972923 100644 --- a/src/arch/aarch64/kernel/core_local.rs +++ b/src/arch/aarch64/kernel/core_local.rs @@ -1,7 +1,7 @@ use alloc::boxed::Box; use alloc::vec::Vec; use core::arch::asm; -use core::cell::{RefCell, RefMut}; +use core::cell::{Cell, RefCell, RefMut}; use core::ptr; use core::sync::atomic::Ordering; @@ -20,7 +20,7 @@ pub(crate) struct CoreLocal { /// ID of the current Core. core_id: CoreId, /// Scheduler of the current Core. - scheduler: RefCell>, + scheduler: Cell<*mut PerCoreScheduler>, /// Interface to the interrupt counters irq_statistics: &'static IrqStatistics, /// Queue of async tasks @@ -44,7 +44,7 @@ impl CoreLocal { let this = Self { this: ptr::null_mut(), core_id, - scheduler: RefCell::new(None), + scheduler: Cell::new(ptr::null_mut()), irq_statistics, async_tasks: RefCell::new(Vec::new()), #[cfg(feature = "smp")] @@ -91,18 +91,17 @@ pub(crate) fn core_id() -> CoreId { } } -pub(crate) fn core_scheduler() -> RefMut<'static, PerCoreScheduler> { - RefMut::map(CoreLocal::get().scheduler.borrow_mut(), |scheduler| { - scheduler.as_mut().unwrap() - }) +#[inline] +pub(crate) fn core_scheduler() -> &'static mut PerCoreScheduler { + unsafe { &mut *CoreLocal::get().scheduler.get() } } pub(crate) fn async_tasks() -> RefMut<'static, Vec> { CoreLocal::get().async_tasks.borrow_mut() } -pub(crate) fn set_core_scheduler(scheduler: PerCoreScheduler) { - *CoreLocal::get().scheduler.borrow_mut() = Some(scheduler); +pub(crate) fn set_core_scheduler(scheduler: *mut PerCoreScheduler) { + CoreLocal::get().scheduler.set(scheduler); } pub(crate) fn increment_irq_counter(irq_no: u8) { diff --git a/src/arch/x86_64/kernel/apic.rs b/src/arch/x86_64/kernel/apic.rs index 3d66c15804..78b0a48198 100644 --- a/src/arch/x86_64/kernel/apic.rs +++ b/src/arch/x86_64/kernel/apic.rs @@ -229,7 +229,7 @@ extern "x86-interrupt" fn wakeup_handler(_stack_frame: interrupts::ExceptionStac debug!("Received Wakeup Interrupt"); increment_irq_counter(WAKEUP_INTERRUPT_NUMBER); - let mut core_scheduler = core_scheduler(); + let core_scheduler = core_scheduler(); core_scheduler.check_input(); eoi(); if core_scheduler.is_scheduling() { diff --git a/src/arch/x86_64/kernel/core_local.rs b/src/arch/x86_64/kernel/core_local.rs index 50509a9e22..bf6210f5fa 100644 --- a/src/arch/x86_64/kernel/core_local.rs +++ b/src/arch/x86_64/kernel/core_local.rs @@ -24,7 +24,7 @@ pub(crate) struct CoreLocal { /// Sequential ID of this CPU Core. core_id: CoreId, /// Scheduler for this CPU Core. - scheduler: RefCell>, + scheduler: Cell<*mut PerCoreScheduler>, /// Task State Segment (TSS) allocated for this CPU Core. pub tss: Cell<*mut TaskStateSegment>, /// start address of the kernel stack @@ -54,7 +54,7 @@ impl CoreLocal { let this = Self { this: ptr::null_mut(), core_id, - scheduler: RefCell::new(None), + scheduler: Cell::new(ptr::null_mut()), tss: Cell::new(ptr::null_mut()), kernel_stack: Cell::new(0), irq_statistics, @@ -101,18 +101,16 @@ pub(crate) fn core_id() -> CoreId { } } -pub(crate) fn core_scheduler() -> RefMut<'static, PerCoreScheduler> { - RefMut::map(CoreLocal::get().scheduler.borrow_mut(), |scheduler| { - scheduler.as_mut().unwrap() - }) +pub(crate) fn core_scheduler() -> &'static mut PerCoreScheduler { + unsafe { &mut *CoreLocal::get().scheduler.get() } } pub(crate) fn async_tasks() -> RefMut<'static, Vec> { CoreLocal::get().async_tasks.borrow_mut() } -pub(crate) fn set_core_scheduler(scheduler: PerCoreScheduler) { - *CoreLocal::get().scheduler.borrow_mut() = Some(scheduler); +pub(crate) fn set_core_scheduler(scheduler: *mut PerCoreScheduler) { + CoreLocal::get().scheduler.set(scheduler); } pub(crate) fn increment_irq_counter(irq_no: u8) { diff --git a/src/scheduler/mod.rs b/src/scheduler/mod.rs index 53b75993e9..b939d08e0e 100644 --- a/src/scheduler/mod.rs +++ b/src/scheduler/mod.rs @@ -1,8 +1,9 @@ +use alloc::boxed::Box; use alloc::collections::{BTreeMap, VecDeque}; use alloc::rc::Rc; #[cfg(feature = "smp")] use alloc::vec::Vec; -use core::cell::{RefCell, RefMut}; +use core::cell::RefCell; use core::sync::atomic::{AtomicU32, Ordering}; use crossbeam_utils::Backoff; @@ -87,9 +88,9 @@ pub trait PerCoreSchedulerExt { fn exit(self, exit_code: i32) -> !; } -impl PerCoreSchedulerExt for RefMut<'_, PerCoreScheduler> { +impl PerCoreSchedulerExt for &mut PerCoreScheduler { #[cfg(target_arch = "x86_64")] - fn reschedule(mut self) { + fn reschedule(self) { without_interrupts(|| { if let Some(last_stack_pointer) = self.scheduler() { let (new_stack_pointer, is_idle) = { @@ -101,12 +102,10 @@ impl PerCoreSchedulerExt for RefMut<'_, PerCoreScheduler> { }; if is_idle || Rc::ptr_eq(&self.current_task, &self.fpu_owner) { - drop(self); unsafe { switch_to_fpu_owner(last_stack_pointer, new_stack_pointer.as_usize()); } } else { - drop(self); unsafe { switch_to_task(last_stack_pointer, new_stack_pointer.as_usize()); } @@ -124,8 +123,6 @@ impl PerCoreSchedulerExt for RefMut<'_, PerCoreScheduler> { use crate::interrupts::SGI_RESCHED; - drop(self); - unsafe { asm!("dsb nsh", "isb", options(nostack, nomem, preserves_flags)); } @@ -145,14 +142,13 @@ impl PerCoreSchedulerExt for RefMut<'_, PerCoreScheduler> { } #[cfg(any(feature = "tcp", feature = "udp"))] - fn add_network_timer(mut self, wakeup_time: Option) { + fn add_network_timer(self, wakeup_time: Option) { without_interrupts(|| { self.blocked_tasks.add_network_timer(wakeup_time); - drop(self); }) } - fn exit(mut self, exit_code: i32) -> ! { + fn exit(self, exit_code: i32) -> ! { without_interrupts(|| { // Get the current task. let mut current_task_borrowed = self.current_task.borrow_mut(); @@ -542,7 +538,7 @@ impl PerCoreScheduler { let backoff = Backoff::new(); loop { - let mut core_scheduler = core_scheduler(); + let core_scheduler = core_scheduler(); interrupts::disable(); // run async tasks @@ -552,7 +548,6 @@ impl PerCoreScheduler { core_scheduler.cleanup_tasks(); if core_scheduler.ready_queue.is_empty() { - drop(core_scheduler); if backoff.is_completed() { interrupts::enable_and_wait(); } else { @@ -703,7 +698,7 @@ pub fn add_current_core() { "Initializing scheduler for core {} with idle task {}", core_id, tid ); - set_core_scheduler(PerCoreScheduler { + let boxed_scheduler = Box::new(PerCoreScheduler { #[cfg(feature = "smp")] core_id, current_task: idle_task.clone(), @@ -714,6 +709,9 @@ pub fn add_current_core() { finished_tasks: VecDeque::new(), blocked_tasks: BlockedTaskQueue::new(), }); + + let scheduler = Box::into_raw(boxed_scheduler); + set_core_scheduler(scheduler); #[cfg(feature = "smp")] { SCHEDULER_INPUTS.lock().insert( @@ -730,9 +728,11 @@ fn get_scheduler_input(core_id: CoreId) -> &'static InterruptTicketMutex Result<(), ()> { + let core_scheduler = core_scheduler(); + debug!( "Task {} is waiting for task {}", - core_scheduler().get_current_task_id(), + core_scheduler.get_current_task_id(), id ); @@ -740,7 +740,6 @@ pub fn join(id: TaskId) -> Result<(), ()> { let mut waiting_tasks_guard = WAITING_TASKS.lock(); if let Some(queue) = waiting_tasks_guard.get_mut(&id) { - let mut core_scheduler = core_scheduler(); queue.push_back(core_scheduler.get_current_task_handle()); core_scheduler.block_current_task(None); diff --git a/src/synch/futex.rs b/src/synch/futex.rs index a22d5a4ebe..ded2428357 100644 --- a/src/synch/futex.rs +++ b/src/synch/futex.rs @@ -52,15 +52,14 @@ pub(crate) fn futex_wait( timeout }; - let mut scheduler = core_scheduler(); + let scheduler = core_scheduler(); scheduler.block_current_task(wakeup_time); let handle = scheduler.get_current_task_handle(); parking_lot.entry(addr(address)).or_default().push(handle); drop(parking_lot); - drop(scheduler); loop { - core_scheduler().reschedule(); + scheduler.reschedule(); let mut parking_lot = PARKING_LOT.lock(); if matches!(wakeup_time, Some(t) if t <= get_timer_ticks()) { @@ -89,7 +88,7 @@ pub(crate) fn futex_wait( } else { // A spurious wakeup occurred, sleep again. // Tasks do not change core, so the handle in the parking lot is still current. - core_scheduler().block_current_task(wakeup_time); + scheduler.block_current_task(wakeup_time); } } drop(parking_lot); @@ -122,15 +121,14 @@ pub(crate) fn futex_wait_and_set( timeout }; - let mut scheduler = core_scheduler(); + let scheduler = core_scheduler(); scheduler.block_current_task(wakeup_time); let handle = scheduler.get_current_task_handle(); parking_lot.entry(addr(address)).or_default().push(handle); drop(parking_lot); - drop(scheduler); loop { - core_scheduler().reschedule(); + scheduler.reschedule(); let mut parking_lot = PARKING_LOT.lock(); if matches!(wakeup_time, Some(t) if t <= get_timer_ticks()) { @@ -159,7 +157,7 @@ pub(crate) fn futex_wait_and_set( } else { // A spurious wakeup occurred, sleep again. // Tasks do not change core, so the handle in the parking lot is still current. - core_scheduler().block_current_task(wakeup_time); + scheduler.block_current_task(wakeup_time); } } drop(parking_lot); @@ -180,7 +178,7 @@ pub(crate) fn futex_wake(address: &AtomicU32, count: i32) -> i32 { Entry::Vacant(_) => return 0, }; - let mut scheduler = core_scheduler(); + let scheduler = core_scheduler(); let mut woken = 0; while woken != count || count == i32::MAX { match queue.get_mut().pop() { @@ -215,7 +213,7 @@ pub(crate) fn futex_wake_or_set(address: &AtomicU32, count: i32, new_value: u32) } }; - let mut scheduler = core_scheduler(); + let scheduler = core_scheduler(); let mut woken = 0; while woken != count || count == i32::MAX { match queue.get_mut().pop() { diff --git a/src/synch/recmutex.rs b/src/synch/recmutex.rs index 03520a97f5..9491124e84 100644 --- a/src/synch/recmutex.rs +++ b/src/synch/recmutex.rs @@ -27,10 +27,10 @@ impl RecursiveMutex { pub fn acquire(&self) { // Get information about the current task. - let tid = core_scheduler().get_current_task_id(); + let core_scheduler = core_scheduler(); + let tid = core_scheduler.get_current_task_id(); loop { - let mut core_scheduler = core_scheduler(); { let mut locked_state = self.state.lock(); diff --git a/src/synch/semaphore.rs b/src/synch/semaphore.rs index b0bb5558d9..1c508fb8a0 100644 --- a/src/synch/semaphore.rs +++ b/src/synch/semaphore.rs @@ -68,12 +68,12 @@ impl Semaphore { pub fn acquire(&self, time: Option) -> bool { #[cfg(feature = "smp")] let backoff = Backoff::new(); + let core_scheduler = core_scheduler(); let wakeup_time = time.map(|ms| crate::arch::processor::get_timer_ticks() + ms * 1000); // Loop until we have acquired the semaphore. loop { - let mut core_scheduler = core_scheduler(); let mut locked_state = self.state.lock(); if locked_state.count > 0 { diff --git a/src/syscalls/tasks.rs b/src/syscalls/tasks.rs index 3edc6a8204..e4962dae15 100644 --- a/src/syscalls/tasks.rs +++ b/src/syscalls/tasks.rs @@ -116,7 +116,7 @@ pub(crate) extern "C" fn __sys_usleep(usecs: u64) { // Enough time to set a wakeup timer and block the current task. debug!("sys_usleep blocking the task for {} microseconds", usecs); let wakeup_time = arch::processor::get_timer_ticks() + usecs; - let mut core_scheduler = core_scheduler(); + let core_scheduler = core_scheduler(); core_scheduler.block_current_task(Some(wakeup_time)); // Switch to the next task. @@ -300,7 +300,7 @@ static BLOCKED_TASKS: InterruptTicketMutex> = extern "C" fn __sys_block_current_task(timeout: &Option) { let wakeup_time = timeout.map(|t| arch::processor::get_timer_ticks() + t * 1000); - let mut core_scheduler = core_scheduler(); + let core_scheduler = core_scheduler(); let handle = core_scheduler.get_current_task_handle(); let tid = core_scheduler.get_current_task_id();