qiurui/redox-os-kernel
1
0
Fork 0
mirror of https://github.com/redox-os/kernel.git synced 2026-09-09 00:07:34 +08:00
Code Issues Packages Projects Releases Wiki Activity

Avoid Arc::clone in context::current with Ref.

Browse Source
This commit is contained in:
4lDO2
2026-08-30 10:14:43 +02:00
parent 5f640551a6
commit b6fa12412e
10 changed files with 72 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/context/mod.rs
+11 -8
View File
@@ -6,7 +6,7 @@ use alloc::{
collections::{BTreeMap, BTreeSet, VecDeque},
sync::{Arc, Weak},
};
use core::{cmp::Reverse, num::NonZeroUsize, ops::Deref, sync::atomic::AtomicUsize};
use core::{cell::Ref, cmp::Reverse, num::NonZeroUsize, ops::Deref, sync::atomic::AtomicUsize};
use syscall::NumaMemoryPolicy;
use crate::{
@@ -206,21 +206,24 @@ pub fn init(token: &mut CleanLockToken) {
}
}
pub fn current() -> Arc<ContextLock> {
PercpuBlock::current()
.switch_internals
.with_context(Arc::clone)
// TODO: Maybe use lock tokens to forbid holding this reference across context::switch (where the
// RefCell's borrow_mut will fail if the reference is kept?)? If so, maybe even avoid `RefCell`
// entirely, if it can be done sufficiently rigorously and without breaking soundness?
pub fn current() -> Ref<'static, Arc<ContextLock>> {
PercpuBlock::current().switch_internals.current_context()
}
pub fn try_current() -> Option<Arc<ContextLock>> {
pub fn try_current() -> Ref<'static, Option<Arc<ContextLock>>> {
PercpuBlock::current()
.switch_internals
.try_with_context(|context| context.map(Arc::clone))
.current_context_raw()
}
pub fn is_current(context: &Arc<ContextLock>) -> bool {
PercpuBlock::current()
.switch_internals
.with_context(|current| Arc::ptr_eq(context, current))
.current_context_raw()
.as_ref()
.map_or(false, |current| Arc::ptr_eq(current, context))
}
#[derive(Clone)]
src/context/signal.rs
+5 -2
View File
@@ -1,5 +1,7 @@
use core::sync::atomic::Ordering;
use alloc::sync::Arc;
use crate::{context, sync::CleanLockToken, syscall::flag::SigcontrolFlags};
pub fn signal_handler(token: &mut CleanLockToken) {
@@ -82,10 +84,11 @@ pub fn excp_handler(excp: syscall::Exception) {
let Some(eh) = context.sig.as_ref().and_then(|s| s.excp_handler) else {
// TODO: Let procmgr print this?
info!(
"UNHANDLED EXCEPTION, CPU {}, PID {}, NAME {}, CONTEXT {current:p}",
"UNHANDLED EXCEPTION, CPU {}, PID {}, NAME {}, CONTEXT {:p}",
crate::cpu_id(),
context.pid,
context.name
context.name,
Arc::as_ptr(&*current),
);
drop(context);
drop(current);
src/context/switch.rs
+23 -55
View File
@@ -17,7 +17,7 @@ use alloc::{
vec::Vec,
};
use core::{
cell::{Cell, RefCell},
cell::{Cell, Ref, RefCell},
cmp::Reverse,
hint, matches, mem,
option::Option::{None, Some},
@@ -60,9 +60,6 @@ unsafe fn opportunistic_write_arc(lock: &Arc<ContextLock>) -> Option<ArcContextL
/// Determines if a given context is eligible to be scheduled on a given CPU (in
/// principle, the current CPU).
///
/// # Safety
/// This function is unsafe because it modifies the `context`'s state directly without synchronization.
///
/// # Parameters
/// - `context`: The context (process/thread) to be checked.
/// - `cpu_id`: The logical ID of the CPU on which the context is being scheduled.
@@ -70,11 +67,7 @@ unsafe fn opportunistic_write_arc(lock: &Arc<ContextLock>) -> Option<ArcContextL
/// # Returns
/// - `UpdateResult::CanSwitch`: If the context can be switched to.
/// - `UpdateResult::Skip`: If the context should be skipped (e.g., it's running on another CPU).
unsafe fn update_runnable(
context: &mut Context,
cpu_id: LogicalCpuId,
switch_time: u128,
) -> UpdateResult {
fn update_runnable(context: &mut Context, cpu_id: LogicalCpuId, switch_time: u128) -> UpdateResult {
// Ignore contexts that are already running.
if context.running {
return UpdateResult::Skip;
@@ -211,9 +204,11 @@ pub fn switch(token: &mut CleanLockToken) -> SwitchResult {
}
// Lock the previous context.
let prev_context_lock = crate::context::current();
// We are careful not to lock this context twice
let mut prev_context_guard = unsafe { prev_context_lock.write_arc() };
let mut prev_context_guard = {
let prev_context_lock_ref = crate::context::current();
// We are careful not to lock this context twice
unsafe { prev_context_lock_ref.write_arc() }
};
if !prev_context_guard.is_preemptable() {
// Unset global lock
@@ -266,7 +261,8 @@ pub fn switch(token: &mut CleanLockToken) -> SwitchResult {
};
// TODO: can this happen?
if !cfg!(opportunistic_context_locking)
&& Weak::as_ptr(&context_ref.0) == Arc::as_ptr(&prev_context_lock)
&& Weak::as_ptr(&context_ref.0)
== Arc::as_ptr(&ArcRwLockWriteGuard::rwlock(&prev_context_guard))
{
continue;
}
@@ -452,9 +448,6 @@ pub fn switch(token: &mut CleanLockToken) -> SwitchResult {
.being_sigkilled
.set(next_context.being_sigkilled);
// Anything implement Drop must be manually dropped now
drop(prev_context_lock);
unsafe {
percpu.new_addrsp_guard.set(addr_space_guard);
arch::switch_to(prev_context, next_context);
@@ -580,7 +573,7 @@ fn select_next_context(
continue;
};
let sw = unsafe { update_runnable(&mut guard, cpu_id, switch_time) };
let sw = update_runnable(&mut guard, cpu_id, switch_time);
if matches!(sw, UpdateResult::Blocked) {
if guard.is_active {
@@ -994,39 +987,23 @@ impl ContextSwitchPercpu {
}
}
/// Applies a function to the current context, allowing controlled access.
///
/// # Parameters
/// - `f`: A closure that receives a reference to the current context and returns a value.
///
/// # Returns
/// The result of applying `f` to the current context.
pub fn with_context<T>(&self, f: impl FnOnce(&Arc<ContextLock>) -> T) -> T {
f(self
.current_ctxt
.borrow()
.as_ref()
.expect("not inside of context"))
/// Gets a reference to the raw current context slot.
pub fn current_context_raw(&self) -> Ref<'_, Option<Arc<ContextLock>>> {
self.current_ctxt.borrow()
}
/// Applies a function to the current context, allowing controlled access.
///
/// # Parameters
/// - `f`: A closure that receives a reference to the current context and returns a value.
///
/// # Returns
/// The result of applying `f` to the current context if any.
pub fn try_with_context<T>(&self, f: impl FnOnce(Option<&Arc<ContextLock>>) -> T) -> T {
f(self.current_ctxt.borrow().as_ref())
/// Gets a reference to the current context, which will always be populated after the startup
/// code (unless there are kernel bugs).
pub fn current_context(&self) -> Ref<'_, Arc<ContextLock>> {
Ref::map(self.current_ctxt.borrow(), |c| {
c.as_ref().expect("no current context present")
})
}
/// Sets the current context to a new value.
///
/// # Safety
/// This function is unsafe as it modifies the context state directly.
///
/// # Parameters
/// - `new`: The new context to be set as the current context.
pub unsafe fn set_current_context(&self, new: Arc<ContextLock>) {
*self.current_ctxt.borrow_mut() = Some(new);
}
@@ -1035,23 +1012,14 @@ impl ContextSwitchPercpu {
///
/// # Safety
/// This function is unsafe as it modifies the idle context state directly.
///
/// # Parameters
/// - `new`: The new context to be set as the idle context.
pub unsafe fn set_idle_context(&self, new: Arc<ContextLock>) {
*self.idle_ctxt.borrow_mut() = Some(new);
}
/// Retrieves the current idle context.
///
/// # Returns
/// A reference to the idle context.
pub fn idle_context(&self) -> Arc<ContextLock> {
Arc::clone(
self.idle_ctxt
.borrow()
.as_ref()
.expect("no idle context present"),
)
pub fn idle_context(&self) -> Ref<'_, Arc<ContextLock>> {
Ref::map(self.idle_ctxt.borrow(), |opt| {
opt.as_ref().expect("no idle context present")
})
}
}
src/panic.rs
+2 -1
View File
@@ -40,7 +40,8 @@ fn panic_handler_inner(info: &PanicInfo) -> ! {
stack_trace();
}
let Some(context_lock) = context::try_current() else {
let current_ref = context::try_current();
let Some(context_lock) = &*current_ref else {
println!("CPU {}, CID <none>", cpu_id());
println!("HALT");
src/scheme/proc.rs
+2 -2
View File
@@ -326,7 +326,7 @@ impl ProcScheme {
);
return Ok((id.get(), InternalFlags::empty()));
}
"cur-context" => context::current(),
"cur-context" => Arc::clone(&*context::current()),
_ => return Err(Error::new(ENOENT)),
};
@@ -416,7 +416,7 @@ impl KernelScheme for ProcScheme {
id,
Handle {
// TODO: placeholder
context: context::current(),
context: Arc::clone(&*context::current()),
kind: ContextHandle::Authority,
},
);
src/sync/wait_condition.rs
+9 -4
View File
@@ -70,8 +70,8 @@ impl WaitCondition {
reason: &'static str,
token: &'a mut LockToken<'a, L2>,
) -> bool {
let current_context_ref = context::current();
{
let current_context_ptr = {
let current_context_ref = context::current();
// Avoid a context switch between blocking ourselves and adding
// ourselves to the wait list as otherwise we might miss a wakeup.
// We cannot add ourselves to the wait list first as that would lead
@@ -94,12 +94,17 @@ impl WaitCondition {
.push(Arc::downgrade(&current_context_ref));
drop(guard);
}
// It's perfectly valid to use raw pointers in Safe Rust for comparing...
Arc::as_ptr(&*current_context_ref)
};
{
// SAFETY: Guaranteed by caller
let token = unsafe { &mut CleanLockToken::new() };
context::switch(token);
// ... and in order for it to be possible to switch back, the context Arc and thus
// current_context_ptr must continue to be alive (meaningful to check against).
}
let mut waited = true;
@@ -109,7 +114,7 @@ impl WaitCondition {
if let Some(index) = contexts
.iter()
.position(|c| Weak::as_ptr(c) == Arc::as_ptr(&current_context_ref))
.position(|c| Weak::as_ptr(c) == current_context_ptr)
{
contexts.swap_remove(index);
waited = false;
src/syscall/fs.rs
+6 -3
View File
@@ -319,13 +319,13 @@ fn call_normal(
let mut nums = arrayvec::ArrayVec::<_, 2>::new();
let current_lock = context::current();
let current_lock_ref = context::current();
let consume = flags.contains(CallFlags::CONSUME);
let mut fds = fds.iter();
let (target_file, scheme) = {
let fd = FileHandle::from(fds.next().copied().unwrap());
let mut current = current_lock.read(token.token());
let mut current = current_lock_ref.read(token.token());
let (file, mut split_token) = match (current.token_split(), consume) {
((ctxt, mut split_token), true) => {
@@ -346,7 +346,7 @@ fn call_normal(
for &fd in fds {
let fd = FileHandle::from(fd);
let mut current = current_lock.read(token.token());
let mut current = current_lock_ref.read(token.token());
let (file, mut split_token) = match (current.token_split(), consume) {
((ctxt, mut split_token), true) => {
@@ -363,6 +363,9 @@ fn call_normal(
nums.push(desc.number);
}
// must never be held when context::switch is called, or it will (predictably) panic
drop(current_lock_ref);
if flags.contains(CallFlags::STD_FS) {
scheme.translate_std_fs_call(
&nums,
src/syscall/futex.rs
+3 -5
View File
@@ -115,9 +115,9 @@ pub fn futex(
.map(|buf| unsafe { buf.read_exact::<TimeSpec>() })
.transpose()?;
let context_lock = context::current();
{
let context_lock = context::current();
// TODO: Lock ordering violation
let mut token = unsafe { CleanLockToken::new() };
let mut futexes = FUTEXES.lock(token.token());
@@ -196,11 +196,9 @@ pub fn futex(
context::switch(token);
let context = context_lock.read(token.token());
// The scheduler clears `wake` on timeout. Hence if a timeout was
// set and `wake` is now `None`, we timed out.
if context.wake.is_none() && timeout_opt.is_some() {
if context::current().read(token.token()).wake.is_none() && timeout_opt.is_some() {
Err(Error::new(ETIMEDOUT))
} else {
Ok(0)
src/syscall/process.rs
+4 -2
View File
@@ -35,7 +35,7 @@ use super::usercopy::UserSliceWo;
/// SAFETY: Returns Never type, all things that implement `Drop` must be dropped manually before calling this to prevent memory leak.
///
pub fn exit_this_context(excp: Option<syscall::Exception>, token: &mut CleanLockToken) -> ! {
let context_lock = context::current();
let context_lock = Arc::clone(&*context::current());
let (addrspace_opt, mut close_files) = {
let mut context = context_lock.write(token.token());
let (context, token) = context.token_split();
@@ -70,12 +70,14 @@ pub fn exit_this_context(excp: Option<syscall::Exception>, token: &mut CleanLock
);
}
{
if !context::contexts_mut(token.downgrade()).remove(&ContextRef(context_lock)) {
let context_ref = ContextRef(context_lock);
if !context::contexts_mut(token.downgrade()).remove(&context_ref) {
#[cfg(feature = "drop_panic")]
{
panic!("This context is not in the cpu")
}
}
drop(context_ref);
}
drop(close_files);
context::switch(token);
src/syscall/time.rs
+7 -3
View File
@@ -36,9 +36,9 @@ pub fn nanosleep(
let start = time::monotonic(token);
let end = start + req.to_nanos();
let current_context = context::current();
{
let context = current_context.upgradeable_read(token.token());
let context_lock = context::current();
let context = context_lock.upgradeable_read(token.token());
if let Some((tctl, pctl, _)) = context.sigcontrol()
&& tctl.currently_pending_unblocked(pctl) != 0
@@ -54,7 +54,11 @@ pub fn nanosleep(
// reason?
context::switch(token);
let was_interrupted = current_context.write(token.token()).wake.take().is_some();
let was_interrupted = context::current()
.write(token.token())
.wake
.take()
.is_some();
if let Some(rem_buf) = rem_buf_opt {
let current = time::monotonic(token);