From a3b70e1c492bfd1d56dd074b9b36d854c47eb4dd Mon Sep 17 00:00:00 2001 From: Paul Pan Date: Thu, 22 Aug 2024 16:26:25 +0800 Subject: [PATCH] feat: lib/utils/linked_list: drop cell --- kernel/src/objects/mod.rs | 19 +----- kernel/src/scheduler.rs | 12 ++-- lib/utils/src/linked_list.rs | 126 ++++++++++++++++++++++------------- 3 files changed, 89 insertions(+), 68 deletions(-) diff --git a/kernel/src/objects/mod.rs b/kernel/src/objects/mod.rs index 7fb9409..2a8d9e6 100644 --- a/kernel/src/objects/mod.rs +++ b/kernel/src/objects/mod.rs @@ -14,7 +14,7 @@ */ -use core::{marker::PhantomData, ptr::NonNull}; +use core::marker::PhantomData; use uapi::{ cap::ObjectType, error::{SysError, SysResult}, @@ -61,22 +61,7 @@ impl<'a, T: KernelObject + ?Sized> TryFrom<&'a CapEntry> for Cap<'a, T> { impl<'a, T: KernelObject + ?Sized> Cap<'a, T> { pub fn append(&mut self, new: &mut CapEntry) { - let next = self.cte.link.next_raw(); - - // update new cap's link - new.link.set_prev(Some(NonNull::from(self.cte))); - new.link.set_next(next); - - // record new cap's addr - let new_addr = Some(NonNull::from(new)); - - // update next cap's link.prev - if let Some(mut next) = next { - unsafe { next.as_mut().link.set_prev(new_addr) }; - } - - // update self's link.next - self.cte.link.next.set(new_addr); + self.cte.link.append(new) } } diff --git a/kernel/src/scheduler.rs b/kernel/src/scheduler.rs index a0fb0d5..18a4ecf 100644 --- a/kernel/src/scheduler.rs +++ b/kernel/src/scheduler.rs @@ -1,8 +1,8 @@ use crate::objects::*; -use core::ptr::NonNull; +use core::sync::atomic::AtomicPtr; use log::{error, trace}; use spin::lazy::Lazy; -use utils::{container_of, linked_list::Link}; +use utils::{container_of_mut, linked_list::Link}; #[thread_local] pub static IDLE_THREAD: Lazy = Lazy::new(|| { @@ -26,9 +26,11 @@ impl Scheduler { } pub fn init(&self) { - let head = unsafe { Some(NonNull::from(&*container_of!(&self.head, TcbObject, link))) }; - self.head.set_next(head); - self.head.set_prev(head); + unsafe { + let head = Some(AtomicPtr::new(container_of_mut!(&self.head, TcbObject, link))); + self.head.set_next(&head); + self.head.set_prev(&head); + } } pub fn add(&self, tcb: &TcbObject) { diff --git a/lib/utils/src/linked_list.rs b/lib/utils/src/linked_list.rs index 7c46ba7..0dc4d0d 100644 --- a/lib/utils/src/linked_list.rs +++ b/lib/utils/src/linked_list.rs @@ -1,5 +1,15 @@ use crate::{container_of_offset, to_field_offset}; -use core::{cell::Cell, ptr::NonNull}; +use core::sync::atomic::{AtomicPtr, Ordering}; + +#[macro_export] +macro_rules! as_mut_ptr { + ($ref:expr => $type:ty) => { + $ref as *const _ as *mut $type + }; + ($ref:expr) => { + $ref as *const _ as *mut _ + }; +} #[macro_export] macro_rules! LinkHelperImpl { @@ -21,8 +31,8 @@ pub trait LinkHelper: Sized { } pub struct Link { - pub prev: Cell>>, - pub next: Cell>>, + pub prev: Option>, + pub next: Option>, } impl Default for Link { @@ -34,18 +44,15 @@ impl Default for Link { impl Clone for Link { fn clone(&self) -> Self { Self { - prev: Cell::new(self.prev.get()), - next: Cell::new(self.next.get()), + prev: copy_ptr(&self.prev), + next: copy_ptr(&self.next), } } } impl Link { pub const fn new() -> Self { - Self { - prev: Cell::new(None), - next: Cell::new(None), - } + Self { prev: None, next: None } } /// # Safety @@ -54,67 +61,75 @@ impl Link { &*(container_of_offset!(self, T, T::LINK_OFFSET)) } - pub fn prev_raw(&self) -> Option> { - self.prev.get() + pub fn prev_raw(&self) -> &Option> { + &self.prev } - pub fn next_raw(&self) -> Option> { - self.next.get() + pub fn next_raw(&self) -> &Option> { + &self.next } pub fn prev(&self) -> Option<&T> { - self.prev_raw().map(|p| unsafe { &*p.as_ptr() }) + self.prev_raw().as_ref().map(|p| unsafe { &*p.load(Ordering::Acquire) }) } pub fn prev_mut(&self) -> Option<&mut T> { - self.prev_raw().map(|p| unsafe { &mut *p.as_ptr() }) + self.prev_raw().as_ref().map(|p| unsafe { &mut *p.load(Ordering::Acquire) }) } pub fn next(&self) -> Option<&T> { - self.next_raw().map(|n| unsafe { &*n.as_ptr() }) + self.next_raw().as_ref().map(|n| unsafe { &*n.load(Ordering::Acquire) }) } pub fn next_mut(&self) -> Option<&mut T> { - self.next_raw().map(|n| unsafe { &mut *n.as_ptr() }) + self.next_raw().as_ref().map(|n| unsafe { &mut *n.load(Ordering::Acquire) }) } - pub fn set_prev(&self, prev: Option>) { - self.prev.set(prev); + /// # Safety + /// Take care of race condition + pub unsafe fn set_prev(&self, prev: &Option>) { + #[allow(invalid_reference_casting)] + let link = &mut *(self as *const _ as *mut Self); + link.prev = copy_ptr(prev); } - pub fn set_next(&self, next: Option>) { - self.next.set(next); + /// # Safety + /// Take care of race condition + pub unsafe fn set_next(&self, next: &Option>) { + #[allow(invalid_reference_casting)] + let link = &mut *(self as *const _ as *mut Self); + link.next = copy_ptr(next); } pub fn prepend(&self, new: &T) { unsafe { // setup new's link new.get_link().set_prev(self.prev_raw()); - new.get_link().set_next(Some(NonNull::from(self.object()))); + new.get_link().set_next(&Some(AtomicPtr::new(as_mut_ptr!(self.object())))); // setup prev's link if let Some(prev) = self.prev_raw() { - prev.as_ref().get_link().set_next(Some(NonNull::from(new))) + load_ptr(prev).get_link().set_next(&Some(AtomicPtr::new(as_mut_ptr!(new)))); } // setup self's link - self.set_prev(Some(NonNull::from(new))); + self.set_prev(&Some(AtomicPtr::new(as_mut_ptr!(new)))); } } pub fn append(&self, new: &T) { unsafe { // setup new's link - new.get_link().set_prev(Some(NonNull::from(self.object()))); + new.get_link().set_prev(&Some(AtomicPtr::new(as_mut_ptr!(self.object())))); new.get_link().set_next(self.next_raw()); // setup next's link if let Some(next) = self.next_raw() { - next.as_ref().get_link().set_prev(Some(NonNull::from(new))) + load_ptr(next).get_link().set_prev(&Some(AtomicPtr::new(as_mut_ptr!(new)))) } // setup self's link - self.set_next(Some(NonNull::from(new))); + self.set_next(&Some(AtomicPtr::new(as_mut_ptr!(new)))); } } @@ -122,21 +137,31 @@ impl Link { unsafe { // setup prev's link if let Some(prev) = self.prev_raw() { - prev.as_ref().get_link().set_next(self.next_raw()) + load_ptr(prev).get_link().set_next(self.next_raw()) } // setup next's link if let Some(next) = self.next_raw() { - next.as_ref().get_link().set_prev(self.prev_raw()) + load_ptr(next).get_link().set_prev(self.prev_raw()) } // setup self's link - self.set_prev(None); - self.set_next(None); + self.set_prev(&None); + self.set_next(&None); } } } +#[inline] +fn copy_ptr(ptr: &Option>) -> Option> { + ptr.as_ref().map(|p| AtomicPtr::new(p.load(Ordering::Acquire))) +} + +#[inline] +unsafe fn load_ptr<'a, T>(ptr: &AtomicPtr) -> &'a mut T { + &mut *ptr.load(Ordering::Acquire) +} + #[cfg(test)] mod tests { use super::*; @@ -148,15 +173,6 @@ mod tests { LinkHelperImpl!(Node: link); - macro_rules! as_mut_ptr { - ($ref:expr => $type:ty) => { - $ref as *const _ as *mut $type - }; - ($ref:expr) => { - $ref as *const _ as *mut _ - }; - } - #[test] fn test_linked_list() { let node1 = Node { @@ -195,9 +211,15 @@ mod tests { { // check next link assert!(node1.link.next_raw().is_some()); - assert_eq!(node1.link.next_raw().unwrap().as_ptr(), as_mut_ptr!(&node2)); + assert_eq!( + node1.link.next_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node2) + ); assert!(node2.link.next_raw().is_some()); - assert_eq!(node2.link.next_raw().unwrap().as_ptr(), as_mut_ptr!(&node3)); + assert_eq!( + node2.link.next_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node3) + ); assert!(node3.link.next_raw().is_none()); } @@ -205,9 +227,15 @@ mod tests { // check prev link assert!(node1.link.prev_raw().is_none()); assert!(node2.link.prev_raw().is_some()); - assert_eq!(node2.link.prev_raw().unwrap().as_ptr(), as_mut_ptr!(&node1)); + assert_eq!( + node2.link.prev_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node1) + ); assert!(node3.link.prev_raw().is_some()); - assert_eq!(node3.link.prev_raw().unwrap().as_ptr(), as_mut_ptr!(&node2)); + assert_eq!( + node3.link.prev_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node2) + ); } { @@ -218,10 +246,16 @@ mod tests { assert!(node2.link.prev_raw().is_none()); assert!(node1.link.next_raw().is_some()); - assert_eq!(node1.link.next_raw().unwrap().as_ptr(), as_mut_ptr!(&node3)); + assert_eq!( + node1.link.next_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node3) + ); assert!(node3.link.prev_raw().is_some()); - assert_eq!(node3.link.prev_raw().unwrap().as_ptr(), as_mut_ptr!(&node1)); + assert_eq!( + node3.link.prev_raw().as_ref().unwrap().load(Ordering::Acquire), + as_mut_ptr!(&node1) + ); } } }