From d314aeb4e9980789df2824138cf8f6b125de7936 Mon Sep 17 00:00:00 2001 From: guoweikang Date: Thu, 17 Oct 2024 15:09:34 +0800 Subject: [PATCH] Remove wait_list, directly defined list in wait_queu Signed-off-by: guoweikang --- modules/axtask/src/lib.rs | 1 - modules/axtask/src/run_queue.rs | 3 +- modules/axtask/src/wait_list.rs | 56 ---------------- modules/axtask/src/wait_queue.rs | 108 +++++++++++++++++-------------- 4 files changed, 60 insertions(+), 108 deletions(-) delete mode 100644 modules/axtask/src/wait_list.rs diff --git a/modules/axtask/src/lib.rs b/modules/axtask/src/lib.rs index 4851c53410..489ac0a95a 100644 --- a/modules/axtask/src/lib.rs +++ b/modules/axtask/src/lib.rs @@ -47,7 +47,6 @@ cfg_if::cfg_if! { mod task; mod task_ext; mod api; - mod wait_list; mod wait_queue; #[cfg(feature = "irq")] diff --git a/modules/axtask/src/run_queue.rs b/modules/axtask/src/run_queue.rs index 0bba518056..ce30911c60 100644 --- a/modules/axtask/src/run_queue.rs +++ b/modules/axtask/src/run_queue.rs @@ -10,8 +10,7 @@ use scheduler::BaseScheduler; use axhal::cpu::this_cpu_id; use crate::task::{CurrentTask, TaskState}; -use crate::wait_list::WaitTaskNode; -use crate::wait_queue::WaitQueueGuard; +use crate::wait_queue::{WaitQueueGuard, WaitTaskNode}; use crate::{AxTaskRef, CpuMask, Scheduler, TaskInner, WaitQueue}; macro_rules! percpu_static { diff --git a/modules/axtask/src/wait_list.rs b/modules/axtask/src/wait_list.rs deleted file mode 100644 index a7fb848fd5..0000000000 --- a/modules/axtask/src/wait_list.rs +++ /dev/null @@ -1,56 +0,0 @@ -use alloc::sync::Arc; -use linked_list::{def_node, List}; - -use crate::AxTaskRef; - -def_node!(WaitTaskNode, AxTaskRef); - -type Node = Arc; - -pub struct WaitTaskList { - list: List, -} - -impl WaitTaskList { - /// Creates a new empty [WaitList]. - pub const fn new() -> Self { - Self { list: List::new() } - } - - /// add wait to list back - pub fn push_back(&mut self, node: Node) { - self.list.push_back(node); - } - - /// pop front node from list - pub fn pop_front(&mut self) -> Option { - self.list.pop_front() - } - - /// remove special task from list - pub fn remove_task(&mut self, task: &AxTaskRef) -> Option { - let mut cursor = self.list.cursor_front_mut(); - let find = loop { - match cursor.current() { - Some(node) => { - if Arc::ptr_eq(node.inner(), task) { - break cursor.remove_current(); - } - } - None => break None, - } - cursor.move_next(); - }; - find - } - - /// Removes the given Node - /// - /// # Safety - /// - /// Callers must ensure that `data` is either on this list or in no list. It being on another - /// list leads to memory unsafety. - pub fn remove(&mut self, node: &Node) -> Option { - unsafe { self.list.remove(node) } - } -} diff --git a/modules/axtask/src/wait_queue.rs b/modules/axtask/src/wait_queue.rs index 4b9f155cc2..5f04361cc4 100644 --- a/modules/axtask/src/wait_queue.rs +++ b/modules/axtask/src/wait_queue.rs @@ -2,19 +2,23 @@ use alloc::sync::Arc; use kernel_guard::{NoOp, NoPreemptIrqSave}; use kspin::{SpinNoIrq, SpinNoIrqGuard}; +use linked_list::{def_node, List}; -use crate::wait_list::{WaitTaskList, WaitTaskNode}; use crate::{current_run_queue, select_run_queue, AxTaskRef}; #[cfg(feature = "irq")] use crate::CurrentTask; +def_node!(WaitTaskNode, AxTaskRef); + macro_rules! declare_current_waiter { ($name: ident) => { - let $name = Arc::new(WaitTaskNode::new($crate::current().as_task_ref().clone())); + let $name = Arc::new(WaitTaskNode::new($crate::current().clone())); }; } +type Node = Arc; + /// A queue to store sleeping tasks. /// /// # Examples @@ -38,31 +42,40 @@ macro_rules! declare_current_waiter { /// assert_eq!(VALUE.load(Ordering::Relaxed), 1); /// ``` pub struct WaitQueue { - queue: SpinNoIrq, + list: SpinNoIrq>, } -pub(crate) type WaitQueueGuard<'a> = SpinNoIrqGuard<'a, WaitTaskList>; +pub(crate) type WaitQueueGuard<'a> = SpinNoIrqGuard<'a, List>; impl WaitQueue { /// Creates an empty wait queue. pub const fn new() -> Self { Self { - queue: SpinNoIrq::new(WaitTaskList::new()), + list: SpinNoIrq::new(List::new()), } } - #[cfg(feature = "irq")] - /// Cancel events by removing the task from the wait queue. + /// Cancel events by removing the task from the wait list. /// If `from_timer_list` is true, try to remove the task from the timer list. - fn cancel_timer(&self, curr: CurrentTask) { - // Try to cancel a timer event from timer lists. - // Just mark task's current timer ticket ID as expired. - curr.timer_ticket_expired(); - // Note: - // this task is still not removed from timer list of target CPU, - // which may cause some redundant timer events because it still needs to - // go through the process of expiring an event from the timer list and invoking the callback. - // (it can be considered a lazy-removal strategy, it will be ignored when it is about to take effect.) + fn cancel_events(&self, waiter: &Node, _from_timer_list: bool) { + // SAFETY: + // Waiter is only defined in the local function scope, + // therefore, it is not possible in other List. + unsafe { + self.list.lock().remove(waiter); + } + + #[cfg(feature = "irq")] + if _from_timer_list { + // Try to cancel a timer event from timer lists. + // Just mark task's current timer ticket ID as expired. + waiter.inner().timer_ticket_expired(); + // Note: + // this task is still not removed from timer list of target CPU, + // which may cause some redundant timer events because it still needs to + // go through the process of expiring an event from the timer list and invoking the callback. + // (it can be considered a lazy-removal strategy, it will be ignored when it is about to take effect.) + } } /// Blocks the current task and put it into the wait list, until other task @@ -70,12 +83,11 @@ impl WaitQueue { pub fn wait(&self) { declare_current_waiter!(waiter); let mut rq = current_run_queue::(); - rq.blocked_resched(self.queue.lock(), waiter.clone()); - // It can only be notified, it should not be in the list - assert!(self.queue.lock().remove(&waiter).is_none()); + rq.blocked_resched(self.list.lock(), waiter.clone()); + self.cancel_events(&waiter, false); } - /// Blocks the current task and put it into the wait queue, until the given + /// Blocks the current task and put it into the wait list, until the given /// `condition` becomes true. /// /// Note that even other tasks notify this task, it will not wake up until @@ -87,21 +99,17 @@ impl WaitQueue { declare_current_waiter!(waiter); loop { let mut rq = current_run_queue::(); - let mut wq = self.queue.lock(); + let wq = self.list.lock(); if condition() { break; } - // It can only be notified, it should not be in the list - // FUTURE: Replace it with debug_assert - assert!(wq.remove(&waiter).is_none()); rq.blocked_resched(wq, waiter.clone()); } - // It can only be notified, it should not be in the list - assert!(self.queue.lock().remove(&waiter).is_none()); + self.cancel_events(&waiter, false); } - /// Blocks the current task and put it into the wait queue, until other tasks + /// Blocks the current task and put it into the wait list, until other tasks /// notify it, or the given duration has elapsed. #[cfg(feature = "irq")] pub fn wait_timeout(&self, dur: core::time::Duration) -> bool { @@ -116,18 +124,16 @@ impl WaitQueue { ); crate::timers::set_alarm_wakeup(deadline, curr.clone()); - rq.blocked_resched(self.queue.lock(), waiter.clone()); + rq.blocked_resched(self.list.lock(), waiter.clone()); let timeout = axhal::time::wall_time() >= deadline; - // Always try to remove the task from wait list. - self.queue.lock().remove(&waiter); // Always try to remove the task from the timer list. - self.cancel_timer(curr); + self.cancel_events(&waiter, true); timeout } - /// Blocks the current task and put it into the wait queue, until the given + /// Blocks the current task and put it into the wait list, until the given /// `condition` becomes true, or the given duration has elapsed. /// /// Note that even other tasks notify this task, it will not wake up until @@ -153,29 +159,25 @@ impl WaitQueue { if axhal::time::wall_time() >= deadline { break; } - let mut wq = self.queue.lock(); + let mut wq = self.list.lock(); if condition() { timeout = false; break; } - // It can only be notified, it should not be in the list - // FUTURE: Replace it with debug_assert - assert!(wq.remove(&waiter).is_none()); rq.blocked_resched(wq, waiter.clone()); } - // Always try to remove the task from wait list. - self.queue.lock().remove(&waiter); + // Always try to remove the task from the timer list. - self.cancel_timer(curr); + self.cancel_events(&waiter, true); timeout } - /// Wakes up one task in the wait queue, usually the first one. + /// Wakes up one task in the wait list, usually the first one. /// /// If `resched` is true, the current task will be preempted when the /// preemption is enabled. pub fn notify_one(&self, resched: bool) -> bool { - let mut wq = self.queue.lock(); + let mut wq = self.list.lock(); if let Some(waiter) = wq.pop_front() { unblock_one_task(waiter.inner().clone(), resched); true @@ -184,7 +186,7 @@ impl WaitQueue { } } - /// Wakes all tasks in the wait queue. + /// Wakes all tasks in the wait list. /// /// If `resched` is true, the current task will be preempted when the /// preemption is enabled. @@ -194,17 +196,25 @@ impl WaitQueue { } } - /// Wake up the given task in the wait queue. + /// Wake up the given task in the wait list. /// /// If `resched` is true, the current task will be preempted when the /// preemption is enabled. pub fn notify_task(&mut self, resched: bool, task: &AxTaskRef) -> bool { - let mut wq = self.queue.lock(); - if let Some(waiter) = wq.remove_task(task) { - unblock_one_task(waiter.inner().clone(), resched); - true - } else { - false + let mut wq = self.list.lock(); + let mut cursor = wq.cursor_front_mut(); + loop { + match cursor.current() { + Some(node) => { + if Arc::ptr_eq(node.inner(), task) { + cursor.remove_current(); + unblock_one_task(task.clone(), resched); + break true; + } + } + None => break false, + } + cursor.move_next(); } } }