Replace the CpuLocal's borrow* APIs with get_with

This commit is contained in:
Zhang Junyang
2024-08-24 14:03:03 +08:00
committed by Tate, Hongliang Tian
parent f7a9510be0
commit 54cbacb2ff
5 changed files with 36 additions and 41 deletions

View File

@ -8,7 +8,11 @@ use core::{
}; };
use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink};
use ostd::{cpu::local::CpuLocal, cpu_local, trap::SoftIrqLine}; use ostd::{
cpu::local::CpuLocal,
cpu_local,
trap::{self, SoftIrqLine},
};
use crate::softirq_id::{TASKLESS_SOFTIRQ_ID, TASKLESS_URGENT_SOFTIRQ_ID}; use crate::softirq_id::{TASKLESS_SOFTIRQ_ID, TASKLESS_URGENT_SOFTIRQ_ID};
@ -132,8 +136,9 @@ fn do_schedule(
{ {
return; return;
} }
let irq_guard = trap::disable_local();
taskless_list taskless_list
.borrow_irq_disabled() .get_with(&irq_guard)
.borrow_mut() .borrow_mut()
.push_front(taskless.clone()); .push_front(taskless.clone());
} }
@ -158,7 +163,8 @@ fn taskless_softirq_handler(
softirq_id: u8, softirq_id: u8,
) { ) {
let mut processing_list = { let mut processing_list = {
let guard = taskless_list.borrow_irq_disabled(); let irq_guard = trap::disable_local();
let guard = taskless_list.get_with(&irq_guard);
let mut list_mut = guard.borrow_mut(); let mut list_mut = guard.borrow_mut();
LinkedList::take(list_mut.deref_mut()) LinkedList::take(list_mut.deref_mut())
}; };
@ -169,8 +175,9 @@ fn taskless_softirq_handler(
.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) .compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed)
.is_err() .is_err()
{ {
let irq_guard = trap::disable_local();
taskless_list taskless_list
.borrow_irq_disabled() .get_with(&irq_guard)
.borrow_mut() .borrow_mut()
.push_front(taskless); .push_front(taskless);
SoftIrqLine::get(softirq_id).raise(); SoftIrqLine::get(softirq_id).raise();

View File

@ -35,7 +35,8 @@ static APIC_TYPE: Once<ApicType> = Once::new();
/// }); /// });
/// ``` /// ```
pub fn borrow<R>(f: impl FnOnce(&mut (dyn Apic + 'static)) -> R) -> R { pub fn borrow<R>(f: impl FnOnce(&mut (dyn Apic + 'static)) -> R) -> R {
let apic_guard = APIC_INSTANCE.borrow_irq_disabled(); let irq_guard = crate::trap::disable_local();
let apic_guard = APIC_INSTANCE.get_with(&irq_guard);
// If it is not initialzed, lazily initialize it. // If it is not initialzed, lazily initialize it.
if !apic_guard.is_completed() { if !apic_guard.is_completed() {

View File

@ -15,7 +15,11 @@ use spin::Once;
use trapframe::TrapFrame; use trapframe::TrapFrame;
use self::apic::APIC_TIMER_CALLBACK; use self::apic::APIC_TIMER_CALLBACK;
use crate::{arch::x86::kernel, cpu_local, trap::IrqLine}; use crate::{
arch::x86::kernel,
cpu_local,
trap::{self, IrqLine},
};
/// The timer frequency (Hz). Here we choose 1000Hz since 1000Hz is easier for unit conversion and /// The timer frequency (Hz). Here we choose 1000Hz since 1000Hz is easier for unit conversion and
/// convenient for timer. What's more, the frequency cannot be set too high or too low, 1000Hz is /// convenient for timer. What's more, the frequency cannot be set too high or too low, 1000Hz is
@ -57,8 +61,9 @@ pub fn register_callback<F>(func: F)
where where
F: Fn() + Sync + Send + 'static, F: Fn() + Sync + Send + 'static,
{ {
let irq_guard = trap::disable_local();
INTERRUPT_CALLBACKS INTERRUPT_CALLBACKS
.borrow_irq_disabled() .get_with(&irq_guard)
.borrow_mut() .borrow_mut()
.push(Box::new(func)); .push(Box::new(func));
} }
@ -66,7 +71,8 @@ where
fn timer_callback(_: &TrapFrame) { fn timer_callback(_: &TrapFrame) {
jiffies::ELAPSED.fetch_add(1, Ordering::SeqCst); jiffies::ELAPSED.fetch_add(1, Ordering::SeqCst);
let callbacks_guard = INTERRUPT_CALLBACKS.borrow_irq_disabled(); let irq_guard = trap::disable_local();
let callbacks_guard = INTERRUPT_CALLBACKS.get_with(&irq_guard);
for callback in callbacks_guard.borrow().iter() { for callback in callbacks_guard.borrow().iter() {
(callback)(); (callback)();
} }

View File

@ -5,10 +5,7 @@
use core::{marker::Sync, ops::Deref}; use core::{marker::Sync, ops::Deref};
use super::{__cpu_local_end, __cpu_local_start}; use super::{__cpu_local_end, __cpu_local_start};
use crate::{ use crate::{arch, trap::DisabledLocalIrqGuard};
arch,
trap::{self, DisabledLocalIrqGuard},
};
/// Defines a CPU-local variable. /// Defines a CPU-local variable.
/// ///
@ -16,14 +13,13 @@ use crate::{
/// ///
/// You can get the reference to the inner object on one CPU by calling /// You can get the reference to the inner object on one CPU by calling
/// [`CpuLocal::get_on_cpu`]. Also if you intend to access the inner object /// [`CpuLocal::get_on_cpu`]. Also if you intend to access the inner object
/// on the current CPU, you can use [`CpuLocal::borrow_irq_disabled`] or /// on the current CPU, you can use [`CpuLocal::get_with`]. The latter
/// [`CpuLocal::borrow_with`]. The latter two accessors can be used even if /// accessors can be used even if the inner object is not `Sync`.
/// the inner object is not `Sync`.
/// ///
/// # Example /// # Example
/// ///
/// ```rust /// ```rust
/// use ostd::{cpu_local, cpu::PinCurrentCpu, task::disable_preempt}; /// use ostd::{cpu_local, cpu::PinCurrentCpu, task::disable_preempt, trap};
/// use core::{sync::atomic::{AtomicU32, Ordering}, cell::Cell}; /// use core::{sync::atomic::{AtomicU32, Ordering}, cell::Cell};
/// ///
/// cpu_local! { /// cpu_local! {
@ -37,7 +33,8 @@ use crate::{
/// let val_of_foo = ref_of_foo.load(Ordering::Relaxed); /// let val_of_foo = ref_of_foo.load(Ordering::Relaxed);
/// println!("FOO VAL: {}", val_of_foo); /// println!("FOO VAL: {}", val_of_foo);
/// ///
/// let bar_guard = BAR.borrow_irq_disabled(); /// let irq_guard = trap::disable_local();
/// let bar_guard = BAR.get_with(&irq_guard);
/// let val_of_bar = bar_guard.get(); /// let val_of_bar = bar_guard.get();
/// println!("BAR VAL: {}", val_of_bar); /// println!("BAR VAL: {}", val_of_bar);
/// } /// }
@ -87,29 +84,19 @@ impl<T: 'static> CpuLocal<T> {
Self(val) Self(val)
} }
/// Get access to the underlying value with IRQs disabled. /// Get access to the underlying value on the current CPU with a
/// provided IRQ guard.
/// ///
/// By this method, you can borrow a reference to the underlying value /// By this method, you can borrow a reference to the underlying value
/// even if `T` is not `Sync`. Because that it is per-CPU and IRQs are /// even if `T` is not `Sync`. Because that it is per-CPU and IRQs are
/// disabled, no other running tasks can access it. /// disabled, no other running tasks can access it.
pub fn borrow_irq_disabled(&'static self) -> CpuLocalDerefGuard<'_, T> { pub fn get_with<'a>(
CpuLocalDerefGuard {
cpu_local: self,
_guard: InnerGuard::Created(trap::disable_local()),
}
}
/// Get access to the underlying value with a provided guard.
///
/// Similar to [`CpuLocal::borrow_irq_disabled`], but you can provide
/// a guard to disable IRQs if you already have one.
pub fn borrow_with<'a>(
&'static self, &'static self,
guard: &'a DisabledLocalIrqGuard, guard: &'a DisabledLocalIrqGuard,
) -> CpuLocalDerefGuard<'a, T> { ) -> CpuLocalDerefGuard<'a, T> {
CpuLocalDerefGuard { CpuLocalDerefGuard {
cpu_local: self, cpu_local: self,
_guard: InnerGuard::Provided(guard), guard,
} }
} }
@ -199,19 +186,12 @@ impl<T: 'static> !Send for CpuLocal<T> {}
/// A guard for accessing the CPU-local object. /// A guard for accessing the CPU-local object.
/// ///
/// It ensures that the CPU-local object is accessed with IRQs disabled. /// It ensures that the CPU-local object is accessed with IRQs disabled.
/// It is created by [`CpuLocal::borrow_irq_disabled`] or /// It is created by [`CpuLocal::borrow_with`].
/// [`CpuLocal::borrow_with`]. Do not hold this guard for a longtime.
#[must_use] #[must_use]
pub struct CpuLocalDerefGuard<'a, T: 'static> { pub struct CpuLocalDerefGuard<'a, T: 'static> {
cpu_local: &'static CpuLocal<T>, cpu_local: &'static CpuLocal<T>,
_guard: InnerGuard<'a>,
}
enum InnerGuard<'a> {
#[allow(dead_code)] #[allow(dead_code)]
Created(DisabledLocalIrqGuard), guard: &'a DisabledLocalIrqGuard,
#[allow(dead_code)]
Provided(&'a DisabledLocalIrqGuard),
} }
impl<T: 'static> Deref for CpuLocalDerefGuard<'_, T> { impl<T: 'static> Deref for CpuLocalDerefGuard<'_, T> {

View File

@ -189,7 +189,8 @@ mod test {
crate::cpu_local! { crate::cpu_local! {
static FOO: RefCell<usize> = RefCell::new(1); static FOO: RefCell<usize> = RefCell::new(1);
} }
let foo_guard = FOO.borrow_irq_disabled(); let irq_guard = crate::trap::disable_local();
let foo_guard = FOO.get_with(&irq_guard);
assert_eq!(*foo_guard.borrow(), 1); assert_eq!(*foo_guard.borrow(), 1);
*foo_guard.borrow_mut() = 2; *foo_guard.borrow_mut() = 2;
assert_eq!(*foo_guard.borrow(), 2); assert_eq!(*foo_guard.borrow(), 2);