From ceb0ef67e110dd6835f838215554c99e6f6473d0 Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Thu, 11 Jul 2024 01:41:24 +0800 Subject: [PATCH] Make `Pauser` work in raw tasks --- kernel/aster-nix/src/prelude.rs | 13 +++++++-- kernel/aster-nix/src/process/process/mod.rs | 2 +- .../src/process/process/timer_manager.rs | 8 ++---- kernel/aster-nix/src/process/signal/pauser.rs | 28 ++++++++++--------- kernel/aster-nix/src/syscall/clock_gettime.rs | 4 +-- kernel/aster-nix/src/syscall/timer_create.rs | 3 +- kernel/aster-nix/src/thread/mod.rs | 15 +++++----- 7 files changed, 41 insertions(+), 32 deletions(-) diff --git a/kernel/aster-nix/src/prelude.rs b/kernel/aster-nix/src/prelude.rs index fa0d9c975..701b8ed91 100644 --- a/kernel/aster-nix/src/prelude.rs +++ b/kernel/aster-nix/src/prelude.rs @@ -30,11 +30,20 @@ macro_rules! current { }; } -/// return current thread +/// Returns the current thread. +/// +/// # Panics +/// +/// This macro will panic if the current task is not associated with a thread. +/// +/// Except for unit tests, all tasks should be associated with threads. To write code that can be +/// called directly in unit tests, consider using [`Thread::current`] instead. +/// +/// [`Thread::current`]: crate::thread::Thread::current #[macro_export] macro_rules! current_thread { () => { - $crate::thread::Thread::current() + $crate::thread::Thread::current().expect("the current task is not associated with a thread") }; } diff --git a/kernel/aster-nix/src/process/process/mod.rs b/kernel/aster-nix/src/process/process/mod.rs index 2f7ccaf36..54b594287 100644 --- a/kernel/aster-nix/src/process/process/mod.rs +++ b/kernel/aster-nix/src/process/process/mod.rs @@ -641,7 +641,7 @@ impl Process { } pub fn current() -> Arc { - let current_thread = Thread::current(); + let current_thread = current_thread!(); if let Some(posix_thread) = current_thread.as_posix_thread() { posix_thread.process() } else { diff --git a/kernel/aster-nix/src/process/process/timer_manager.rs b/kernel/aster-nix/src/process/process/timer_manager.rs index 98a0e39fb..4edacba33 100644 --- a/kernel/aster-nix/src/process/process/timer_manager.rs +++ b/kernel/aster-nix/src/process/process/timer_manager.rs @@ -18,14 +18,12 @@ use ostd::{ use super::Process; use crate::{ + prelude::*, process::{ posix_thread::PosixThreadExt, signal::{constants::SIGALRM, signals::kernel::KernelSignal}, }, - thread::{ - work_queue::{submit_work_item, work_item::WorkItem}, - Thread, - }, + thread::work_queue::{submit_work_item, work_item::WorkItem}, time::{ clocks::{ProfClock, RealTimeClock}, Timer, TimerManager, @@ -38,7 +36,7 @@ use crate::{ /// invoke the callbacks of expired timers which are based on the updated /// CPU clock. fn update_cpu_time() { - let current_thread = Thread::current(); + let current_thread = current_thread!(); if let Some(posix_thread) = current_thread.as_posix_thread() { let process = posix_thread.process(); let timer_manager = process.timer_manager(); diff --git a/kernel/aster-nix/src/process/signal/pauser.rs b/kernel/aster-nix/src/process/signal/pauser.rs index b64fe7db5..015f1e904 100644 --- a/kernel/aster-nix/src/process/signal/pauser.rs +++ b/kernel/aster-nix/src/process/signal/pauser.rs @@ -123,9 +123,9 @@ impl Pauser { where F: FnMut() -> Option, { - let current_thread = current_thread!(); + let current_thread = Thread::current(); let sig_queue_waiter = - SigObserverRegistrar::new(¤t_thread, self.sig_mask, self.clone()); + SigObserverRegistrar::new(current_thread.as_ref(), self.sig_mask, self.clone()); let cond = || { if let Some(res) = cond() { @@ -174,8 +174,12 @@ enum SigObserverRegistrar<'a> { } impl<'a> SigObserverRegistrar<'a> { - fn new(current_thread: &'a Arc, sig_mask: SigMask, pauser: Arc) -> Self { - let Some(thread) = current_thread.as_posix_thread() else { + fn new( + current_thread: Option<&'a Arc>, + sig_mask: SigMask, + pauser: Arc, + ) -> Self { + let Some(thread) = current_thread.and_then(|thread| thread.as_posix_thread()) else { return Self::KernelThread; }; @@ -281,19 +285,17 @@ mod test { let boolean = Arc::new(AtomicBool::new(false)); let boolean_cloned = boolean.clone(); - let thread1 = Thread::spawn_kernel_thread(ThreadOptions::new(move || { - pauser - .pause_until(|| boolean.load(Ordering::Relaxed).then_some(())) - .unwrap(); - })); - - let thread2 = Thread::spawn_kernel_thread(ThreadOptions::new(move || { + let thread = Thread::spawn_kernel_thread(ThreadOptions::new(move || { Thread::yield_now(); + boolean_cloned.store(true, Ordering::Relaxed); pauser_cloned.resume_all(); })); - thread1.join(); - thread2.join(); + pauser + .pause_until(|| boolean.load(Ordering::Relaxed).then_some(())) + .unwrap(); + + thread.join(); } } diff --git a/kernel/aster-nix/src/syscall/clock_gettime.rs b/kernel/aster-nix/src/syscall/clock_gettime.rs index 52bd6947a..536149d06 100644 --- a/kernel/aster-nix/src/syscall/clock_gettime.rs +++ b/kernel/aster-nix/src/syscall/clock_gettime.rs @@ -9,7 +9,7 @@ use super::SyscallReturn; use crate::{ prelude::*, process::{posix_thread::PosixThreadExt, process_table}, - thread::{thread_table, Thread}, + thread::thread_table, time::{ clockid_t, clocks::{ @@ -121,7 +121,7 @@ pub fn read_clock(clockid: clockid_t) -> Result { Ok(process.prof_clock().read_time()) } ClockId::CLOCK_THREAD_CPUTIME_ID => { - let thread = Thread::current(); + let thread = current_thread!(); Ok(thread.as_posix_thread().unwrap().prof_clock().read_time()) } } diff --git a/kernel/aster-nix/src/syscall/timer_create.rs b/kernel/aster-nix/src/syscall/timer_create.rs index bc9bdf0a1..da03146c3 100644 --- a/kernel/aster-nix/src/syscall/timer_create.rs +++ b/kernel/aster-nix/src/syscall/timer_create.rs @@ -20,7 +20,6 @@ use crate::{ thread::{ thread_table, work_queue::{submit_work_item, work_item::WorkItem}, - Thread, }, time::{ clockid_t, @@ -113,7 +112,7 @@ pub fn sys_timer_create( match clock_id { ClockId::CLOCK_PROCESS_CPUTIME_ID => process_timer_manager.create_prof_timer(func), ClockId::CLOCK_THREAD_CPUTIME_ID => { - let current_thread = Thread::current(); + let current_thread = current_thread!(); current_thread .as_posix_thread() .unwrap() diff --git a/kernel/aster-nix/src/thread/mod.rs b/kernel/aster-nix/src/thread/mod.rs index b9cbe0fdd..349c9eb0e 100644 --- a/kernel/aster-nix/src/thread/mod.rs +++ b/kernel/aster-nix/src/thread/mod.rs @@ -50,15 +50,16 @@ impl Thread { } } - pub fn current() -> Arc { - let task = Task::current(); - let thread = task + /// Returns the current thread, or `None` if the current task is not associated with a thread. + /// + /// Except for unit tests, all tasks should be associated with threads. This method is useful + /// when writing code that can be called directly by unit tests. If this isn't the case, + /// consider using [`current_thread!`] instead. + pub fn current() -> Option> { + Task::current() .data() - .downcast_ref::>() - .expect("[Internal Error] task data should points to weak"); - thread + .downcast_ref::>()? .upgrade() - .expect("[Internal Error] current thread cannot be None") } pub(in crate::thread) fn task(&self) -> &Arc {