From 2f511069eecf521661aa4d8ee3020863be75b543 Mon Sep 17 00:00:00 2001 From: Chen Chengjun Date: Thu, 17 Oct 2024 17:41:24 +0800 Subject: [PATCH] Move SoftIRQ implementations to softirq component --- Cargo.lock | 11 ++++ Cargo.toml | 1 + Components.toml | 1 + Makefile | 1 + kernel/Cargo.toml | 1 + kernel/comps/softirq/Cargo.toml | 14 +++++ .../comps/softirq/src/lib.rs | 24 ++++++--- kernel/{ => comps/softirq}/src/softirq_id.rs | 0 kernel/{ => comps/softirq}/src/taskless.rs | 13 +++-- kernel/src/lib.rs | 3 -- kernel/src/time/softirq.rs | 5 +- ostd/src/cpu/local/cell.rs | 7 ++- ostd/src/cpu/local/mod.rs | 6 +-- ostd/src/cpu/mod.rs | 4 +- ostd/src/lib.rs | 4 -- ostd/src/trap/handler.rs | 54 +++++++++++++++---- ostd/src/trap/mod.rs | 4 +- 17 files changed, 106 insertions(+), 47 deletions(-) create mode 100644 kernel/comps/softirq/Cargo.toml rename ostd/src/trap/softirq.rs => kernel/comps/softirq/src/lib.rs (91%) rename kernel/{ => comps/softirq}/src/softirq_id.rs (100%) rename kernel/{ => comps/softirq}/src/taskless.rs (97%) diff --git a/Cargo.lock b/Cargo.lock index 8b8f226d..437bfe61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -161,6 +161,7 @@ dependencies = [ "aster-network", "aster-rights", "aster-rights-proc", + "aster-softirq", "aster-time", "aster-util", "aster-virtio", @@ -218,6 +219,16 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "aster-softirq" +version = "0.1.0" +dependencies = [ + "component", + "intrusive-collections", + "ostd", + "spin 0.9.8", +] + [[package]] name = "aster-time" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 7e72f858..34420f2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ members = [ "kernel/comps/framebuffer", "kernel/comps/input", "kernel/comps/network", + "kernel/comps/softirq", "kernel/comps/time", "kernel/comps/virtio", "kernel/libs/cpio-decoder", diff --git a/Components.toml b/Components.toml index f442c589..69ef6519 100644 --- a/Components.toml +++ b/Components.toml @@ -5,6 +5,7 @@ virtio = { name = "aster-virtio" } input = { name = "aster-input" } block = { name = "aster-block" } console = { name = "aster-console" } +softirq = { name = "aster-softirq" } time = { name = "aster-time" } framebuffer = { name = "aster-framebuffer" } network = { name = "aster-network" } diff --git a/Makefile b/Makefile index cbb61b45..de801a35 100644 --- a/Makefile +++ b/Makefile @@ -135,6 +135,7 @@ OSDK_CRATES := \ kernel/comps/framebuffer \ kernel/comps/input \ kernel/comps/network \ + kernel/comps/softirq \ kernel/comps/time \ kernel/comps/virtio \ kernel/libs/aster-util \ diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 819400b6..e0d07269 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -12,6 +12,7 @@ aster-block = { path = "comps/block" } aster-network = { path = "comps/network" } aster-console = { path = "comps/console" } aster-framebuffer = { path = "comps/framebuffer" } +aster-softirq = { path = "comps/softirq" } aster-time = { path = "comps/time" } aster-virtio = { path = "comps/virtio" } aster-rights = { path = "libs/aster-rights" } diff --git a/kernel/comps/softirq/Cargo.toml b/kernel/comps/softirq/Cargo.toml new file mode 100644 index 00000000..b21ba72e --- /dev/null +++ b/kernel/comps/softirq/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "aster-softirq" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +ostd = { path = "../../../ostd" } +component = { path = "../../libs/comp-sys/component" } +intrusive-collections = "0.9.5" +spin = "0.9.4" + +[features] \ No newline at end of file diff --git a/ostd/src/trap/softirq.rs b/kernel/comps/softirq/src/lib.rs similarity index 91% rename from ostd/src/trap/softirq.rs rename to kernel/comps/softirq/src/lib.rs index 6efa8ec5..fbf0db10 100644 --- a/ostd/src/trap/softirq.rs +++ b/kernel/comps/softirq/src/lib.rs @@ -1,13 +1,22 @@ // SPDX-License-Identifier: MPL-2.0 //! Software interrupt. +#![no_std] +#![deny(unsafe_code)] + +extern crate alloc; use alloc::boxed::Box; use core::sync::atomic::{AtomicU8, Ordering}; +use component::{init_component, ComponentInitError}; +use ostd::{cpu_local_cell, trap::register_bottom_half_handler}; use spin::Once; -use crate::{cpu_local_cell, task::disable_preempt}; +pub mod softirq_id; +mod taskless; + +pub use taskless::Taskless; /// A representation of a software interrupt (softirq) line. /// @@ -95,15 +104,15 @@ impl SoftIrqLine { /// A slice that stores the [`SoftIrqLine`]s, whose ID is equal to its offset in the slice. static LINES: Once<[SoftIrqLine; SoftIrqLine::NR_LINES as usize]> = Once::new(); -/// Initializes the softirq lines. -/// -/// # Safety -/// -/// This function must be called only once. -pub unsafe fn init() { +#[init_component] +fn init() -> Result<(), ComponentInitError> { let lines: [SoftIrqLine; SoftIrqLine::NR_LINES as usize] = core::array::from_fn(|i| SoftIrqLine::new(i as u8)); LINES.call_once(|| lines); + register_bottom_half_handler(process_pending); + + taskless::init(); + Ok(()) } static ENABLED_MASK: AtomicU8 = AtomicU8::new(0); @@ -139,7 +148,6 @@ pub(crate) fn process_pending() { return; } - let _preempt_guard = disable_preempt(); disable_softirq_local(); for _i in 0..SOFTIRQ_RUN_TIMES { diff --git a/kernel/src/softirq_id.rs b/kernel/comps/softirq/src/softirq_id.rs similarity index 100% rename from kernel/src/softirq_id.rs rename to kernel/comps/softirq/src/softirq_id.rs diff --git a/kernel/src/taskless.rs b/kernel/comps/softirq/src/taskless.rs similarity index 97% rename from kernel/src/taskless.rs rename to kernel/comps/softirq/src/taskless.rs index 54e345f3..dba5a370 100644 --- a/kernel/src/taskless.rs +++ b/kernel/comps/softirq/src/taskless.rs @@ -8,13 +8,12 @@ use core::{ }; use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; -use ostd::{ - cpu::local::CpuLocal, - cpu_local, - trap::{self, SoftIrqLine}, -}; +use ostd::{cpu::local::CpuLocal, cpu_local, trap}; -use crate::softirq_id::{TASKLESS_SOFTIRQ_ID, TASKLESS_URGENT_SOFTIRQ_ID}; +use super::{ + softirq_id::{TASKLESS_SOFTIRQ_ID, TASKLESS_URGENT_SOFTIRQ_ID}, + SoftIrqLine, +}; /// `Taskless` represents a _taskless_ job whose execution is deferred to a later time. /// @@ -211,7 +210,7 @@ mod test { fn init() { static DONE: AtomicBool = AtomicBool::new(false); if !DONE.load(Ordering::SeqCst) { - super::init(); + let _ = super::super::init(); DONE.store(true, Ordering::SeqCst); } } diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index 9264bc8c..25126a22 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -70,9 +70,7 @@ pub mod net; pub mod prelude; mod process; mod sched; -pub mod softirq_id; pub mod syscall; -mod taskless; pub mod thread; pub mod time; mod util; @@ -109,7 +107,6 @@ pub fn init() { fs::rootfs::init(boot::initramfs()).unwrap(); device::init().unwrap(); vdso::init(); - taskless::init(); process::init(); } diff --git a/kernel/src/time/softirq.rs b/kernel/src/time/softirq.rs index 52bcbed8..18ad97cc 100644 --- a/kernel/src/time/softirq.rs +++ b/kernel/src/time/softirq.rs @@ -2,9 +2,8 @@ use alloc::{boxed::Box, vec::Vec}; -use ostd::{sync::RwLock, timer, trap::SoftIrqLine}; - -use crate::softirq_id::TIMER_SOFTIRQ_ID; +use aster_softirq::{softirq_id::TIMER_SOFTIRQ_ID, SoftIrqLine}; +use ostd::{sync::RwLock, timer}; static TIMER_SOFTIRQ_CALLBACKS: RwLock>> = RwLock::new(Vec::new()); diff --git a/ostd/src/cpu/local/cell.rs b/ostd/src/cpu/local/cell.rs index 5eb89611..057ae148 100644 --- a/ostd/src/cpu/local/cell.rs +++ b/ostd/src/cpu/local/cell.rs @@ -39,6 +39,7 @@ use crate::arch; /// println!("2nd FOO VAL: {:?}", FOO.load()); /// } /// ``` +#[macro_export] macro_rules! cpu_local_cell { ($( $(#[$attr:meta])* $vis:vis static $name:ident: $t:ty = $init:expr; )*) => { $( @@ -55,8 +56,6 @@ macro_rules! cpu_local_cell { }; } -pub(crate) use cpu_local_cell; - /// Inner mutable CPU-local objects. /// /// CPU-local cell objects are only accessible from the current CPU. When @@ -71,6 +70,10 @@ pub(crate) use cpu_local_cell; /// You should only create the CPU-local cell object using the macro /// [`cpu_local_cell!`]. /// +/// Please exercise extreme caution when using `CpuLocalCell`. In most cases, +/// it is necessary to disable interrupts or preemption when using it to prevent +/// the operated object from being changed, which can lead to race conditions. +/// /// For the difference between [`super::CpuLocal`] and [`CpuLocalCell`], see /// [`super`]. pub struct CpuLocalCell(UnsafeCell); diff --git a/ostd/src/cpu/local/mod.rs b/ostd/src/cpu/local/mod.rs index 4ee1c1cd..5ff528b8 100644 --- a/ostd/src/cpu/local/mod.rs +++ b/ostd/src/cpu/local/mod.rs @@ -28,10 +28,6 @@ // the CPU-local objects can be shared across CPUs. While through a CPU-local // cell object you can only access the value on the current CPU, therefore // enabling inner mutability without locks. -// -// The cell-variant is currently not a public API because that it is rather -// hard to be used without introducing races. But it is useful for OSTD's -// internal implementation. mod cell; mod cpu_local; @@ -41,7 +37,7 @@ pub(crate) mod single_instr; use alloc::vec::Vec; use align_ext::AlignExt; -pub(crate) use cell::{cpu_local_cell, CpuLocalCell}; +pub use cell::CpuLocalCell; pub use cpu_local::{CpuLocal, CpuLocalDerefGuard}; use spin::Once; diff --git a/ostd/src/cpu/mod.rs b/ostd/src/cpu/mod.rs index 00da818a..6525d4cf 100644 --- a/ostd/src/cpu/mod.rs +++ b/ostd/src/cpu/mod.rs @@ -13,11 +13,11 @@ cfg_if::cfg_if! { } use bitvec::prelude::BitVec; -use local::cpu_local_cell; use spin::Once; use crate::{ - arch::boot::smp::get_num_processors, task::DisabledPreemptGuard, trap::DisabledLocalIrqGuard, + arch::boot::smp::get_num_processors, cpu_local_cell, task::DisabledPreemptGuard, + trap::DisabledLocalIrqGuard, }; /// The number of CPUs. diff --git a/ostd/src/lib.rs b/ostd/src/lib.rs index f605d2e7..61f646c9 100644 --- a/ostd/src/lib.rs +++ b/ostd/src/lib.rs @@ -51,8 +51,6 @@ pub use ostd_macros::main; pub use ostd_pod::Pod; pub use self::{error::Error, prelude::Result}; -// [`CpuLocalCell`] is easy to be misused, so we don't expose it to the users. -pub(crate) use crate::cpu::local::cpu_local_cell; /// Initializes OSTD. /// @@ -87,8 +85,6 @@ pub unsafe fn init() { mm::kspace::init_kernel_page_table(mm::init_page_meta()); mm::dma::init(); - // SAFETY: This function is called only once in the entire system. - unsafe { trap::softirq::init() }; arch::init_on_bsp(); smp::init(); diff --git a/ostd/src/trap/handler.rs b/ostd/src/trap/handler.rs index 70bb5e01..9b3f8a00 100644 --- a/ostd/src/trap/handler.rs +++ b/ostd/src/trap/handler.rs @@ -1,6 +1,47 @@ // SPDX-License-Identifier: MPL-2.0 -use crate::{arch::irq::IRQ_LIST, cpu_local_cell, trap::TrapFrame}; +use alloc::boxed::Box; + +use spin::Once; + +use crate::{arch::irq::IRQ_LIST, cpu_local_cell, task::disable_preempt, trap::TrapFrame}; + +static BOTTOM_HALF_HANDLER: Once> = Once::new(); + +/// Register a function to the interrupt bottom half execution. +/// +/// The handling of an interrupt can be divided into two parts: top half and bottom half. +/// Top half typically performs critical tasks and runs at a high priority. +/// Relatively, bottom half defers less critical tasks to reduce the time spent in +/// hardware interrupt context, thus allowing the interrupts to be handled more quickly. +/// +/// The bottom half handler will be called after the top half with interrupts enabled. +/// +/// This function can only be registered once. Subsequent calls will do nothing. +pub fn register_bottom_half_handler(func: F) +where + F: Fn() + Sync + Send + 'static, +{ + BOTTOM_HALF_HANDLER.call_once(|| Box::new(func)); +} + +fn process_top_half(trap_frame: &TrapFrame, irq_number: usize) { + let irq_line = IRQ_LIST.get().unwrap().get(irq_number).unwrap(); + let callback_functions = irq_line.callback_list(); + for callback_function in callback_functions.iter() { + callback_function.call(trap_frame); + } +} + +fn process_bottom_half() { + // We need to disable preemption when processing bottom half since + // the interrupt is enabled in this context. + let _preempt_guard = disable_preempt(); + + if let Some(handler) = BOTTOM_HALF_HANDLER.get() { + handler() + } +} pub(crate) fn call_irq_callback_functions(trap_frame: &TrapFrame, irq_number: usize) { // For x86 CPUs, interrupts are not re-entrant. Local interrupts will be disabled when @@ -9,17 +50,10 @@ pub(crate) fn call_irq_callback_functions(trap_frame: &TrapFrame, irq_number: us // FIXME: For arch that supports re-entrant interrupts, we may need to record nested level here. IN_INTERRUPT_CONTEXT.store(true); - let irq_line = IRQ_LIST.get().unwrap().get(irq_number).unwrap(); - let callback_functions = irq_line.callback_list(); - for callback_function in callback_functions.iter() { - callback_function.call(trap_frame); - } - drop(callback_functions); - + process_top_half(trap_frame, irq_number); crate::arch::interrupts_ack(irq_number); - crate::arch::irq::enable_local(); - crate::trap::softirq::process_pending(); + process_bottom_half(); IN_INTERRUPT_CONTEXT.store(false); } diff --git a/ostd/src/trap/mod.rs b/ostd/src/trap/mod.rs index 1470a04d..763be6f3 100644 --- a/ostd/src/trap/mod.rs +++ b/ostd/src/trap/mod.rs @@ -4,10 +4,8 @@ mod handler; mod irq; -pub mod softirq; -pub use handler::in_interrupt_context; -pub use softirq::SoftIrqLine; +pub use handler::{in_interrupt_context, register_bottom_half_handler}; pub(crate) use self::handler::call_irq_callback_functions; pub use self::irq::{disable_local, DisabledLocalIrqGuard, IrqCallbackFunction, IrqLine};