Skip to content

Commit

Permalink
Replaces RefCell with Cell on td_locks and td_pticks (#1268)
Browse files Browse the repository at this point in the history
  • Loading branch information
ultimaweapon authored Jan 30, 2025
1 parent 49917f5 commit b25bd7d
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 39 deletions.
4 changes: 2 additions & 2 deletions kernel/src/lock/mutex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<T> Mutex<T> {
todo!()
}

*td.active_mutexes_mut() += 1;
td.set_active_mutexes(td.active_mutexes() + 1);

MutexGuard {
data: self.data.get(),
Expand All @@ -62,7 +62,7 @@ impl<T> Mutex<T> {
unsafe fn unlock(lock: &AtomicUsize) {
let td = current_thread();

*td.active_mutexes_mut() -= 1;
td.set_active_mutexes(td.active_mutexes() - 1);

// TODO: There is a check for (m->lock_object).lo_data == 0 on the PS4.
if lock
Expand Down
69 changes: 47 additions & 22 deletions kernel/src/proc/thread/cell.rs
Original file line number Diff line number Diff line change
@@ -1,48 +1,73 @@
use super::Thread;
use crate::context::{current_thread, BorrowedArc};
use core::cell::{RefCell, RefMut};
use core::cell::Cell;

/// Encapsulates a field of [Thread] that can only be accessed by the CPU that currently executing
/// the thread.
pub struct PrivateCell<T>(RefCell<T>);
///
/// # Context safety
/// [`Default`] implementation of this type does not require a CPU context as long as implementation
/// on `T` does not.
#[derive(Default)]
pub struct PrivateCell<T>(T);

impl<T> PrivateCell<T> {
/// # Context safety
/// This function does not require a CPU context.
pub fn new(v: T) -> Self {
Self(RefCell::new(v))
fn validate(&self, owner: &Thread) {
// This check will optimized out for most of the time due to the implementation of
// current_thread() use "pure" + "nomem" on inline assembly.
let current = current_thread();

if !core::ptr::eq(BorrowedArc::as_ptr(&current), owner) {
panic!("accessing a private cell from the other thread is not supported");
}
}
}

/// See [borrow_mut] for a safe wrapper.
impl<T> PrivateCell<Cell<T>> {
/// See [set] for a safe wrapper.
///
/// # Safety
/// `owner` must be an owner of this field.
///
/// # Panics
/// If `owner` is not the current thread.
pub unsafe fn borrow_mut(&self, owner: &Thread) -> RefMut<T> {
pub unsafe fn set(&self, owner: &Thread, v: T) {
self.validate(owner);
self.0.borrow_mut()
self.0.set(v);
}
}

fn validate(&self, owner: &Thread) {
// This check will optimized out for most of the time due to the implementation of
// current_thread() use "pure" + "nomem" on inline assembly.
let current = current_thread();

if !core::ptr::eq(BorrowedArc::as_ptr(&current), owner) {
panic!("accessing a private cell from the other thread is not supported");
}
impl<T: Copy> PrivateCell<Cell<T>> {
/// See [get] for a safe wrapper.
///
/// # Safety
/// `owner` must be an owner of this field.
///
/// # Panics
/// If `owner` is not the current thread.
pub unsafe fn get(&self, owner: &Thread) -> T {
self.validate(owner);
self.0.get()
}
}

unsafe impl<T> Sync for PrivateCell<T> {}
unsafe impl<T: Send> Sync for PrivateCell<T> {}

/// Safe wrapper of [PrivateCell::set()].
macro_rules! set {
($t:ident, $f:ident, $v:expr) => {
// SAFETY: $t is an owner of $f.
unsafe { $t.$f.set($t, $v) }
};
}

/// Safe wrapper of [PrivateCell::borrow_mut()].
macro_rules! borrow_mut {
/// Safe wrapper of [PrivateCell::get()].
macro_rules! get {
($t:ident, $f:ident) => {
unsafe { $t.$f.borrow_mut($t) }
// SAFETY: $t is an owner of $f.
unsafe { $t.$f.get($t) }
};
}

pub(super) use borrow_mut;
pub(super) use get;
pub(super) use set;
34 changes: 20 additions & 14 deletions kernel/src/proc/thread/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use self::cell::{borrow_mut, PrivateCell};
use self::cell::{get, set, PrivateCell};
use super::Proc;
use crate::lock::{Gutex, GutexGroup, GutexWrite};
use alloc::sync::Arc;
use core::cell::RefMut;
use core::cell::Cell;
use core::sync::atomic::{AtomicU8, Ordering};

mod cell;
Expand All @@ -18,12 +18,12 @@ mod cell;
/// Do not try to access any [`PrivateCell`] fields from interrupt handler because it might
/// currently locked, which will can cause a panic.
pub struct Thread {
proc: Arc<Proc>, // td_proc
active_pins: AtomicU8, // td_critnest
active_interrupts: AtomicU8, // td_intr_nesting_level
active_mutexes: PrivateCell<u16>, // td_locks
sleeping: Gutex<usize>, // td_wchan
profiling_ticks: PrivateCell<u32>, // td_pticks
proc: Arc<Proc>, // td_proc
active_pins: AtomicU8, // td_critnest
active_interrupts: AtomicU8, // td_intr_nesting_level
active_mutexes: PrivateCell<Cell<u16>>, // td_locks
sleeping: Gutex<usize>, // td_wchan
profiling_ticks: PrivateCell<Cell<u32>>, // td_pticks
}

impl Thread {
Expand All @@ -42,9 +42,9 @@ impl Thread {
proc,
active_pins: AtomicU8::new(0),
active_interrupts: AtomicU8::new(0),
active_mutexes: PrivateCell::new(0),
active_mutexes: PrivateCell::default(),
sleeping: gg.spawn(0),
profiling_ticks: PrivateCell::new(0),
profiling_ticks: PrivateCell::default(),
}
}

Expand Down Expand Up @@ -79,8 +79,14 @@ impl Thread {

/// # Panics
/// If called from the other thread.
pub fn active_mutexes_mut(&self) -> RefMut<u16> {
borrow_mut!(self, active_mutexes)
pub fn active_mutexes(&self) -> u16 {
get!(self, active_mutexes)
}

/// # Panics
/// If called from the other thread.
pub fn set_active_mutexes(&self, v: u16) {
set!(self, active_mutexes, v)
}

/// Sleeping address. Zero if this thread is not in a sleep queue.
Expand All @@ -90,7 +96,7 @@ impl Thread {

/// # Panics
/// If called from the other thread.
pub fn profiling_ticks_mut(&self) -> RefMut<u32> {
borrow_mut!(self, profiling_ticks)
pub fn set_profiling_ticks(&self, v: u32) {
set!(self, profiling_ticks, v)
}
}
2 changes: 1 addition & 1 deletion kernel/src/trap/x86_64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub extern "C" fn syscall_handler() {
let td = current_thread();
let p = td.proc();

*td.profiling_ticks_mut() = 0;
td.set_profiling_ticks(0);

// We merge sv_fetch_syscall_args and the code to invoke each syscall handler together.
p.abi().syscall_handler();
Expand Down

0 comments on commit b25bd7d

Please sign in to comment.