From 613a5fac0fe757105b8b13860848207129f7f59e Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Sun, 7 Nov 2021 23:33:50 +0900 Subject: [PATCH 1/4] Refactor context switching. NFCI --- kernel/arch/x64/interrupt.rs | 8 -------- kernel/process/switch.rs | 27 ++++++++++----------------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/kernel/arch/x64/interrupt.rs b/kernel/arch/x64/interrupt.rs index 5afd316f..5e4b6734 100644 --- a/kernel/arch/x64/interrupt.rs +++ b/kernel/arch/x64/interrupt.rs @@ -195,11 +195,3 @@ unsafe extern "C" fn x64_handle_interrupt(vec: u8, frame: *const InterruptFrame) } } } - -pub unsafe fn enable_interrupt() { - asm!("sti"); -} - -pub fn is_interrupt_enabled() -> bool { - x86::current::rflags::read().contains(RFlags::FLAGS_IF) -} diff --git a/kernel/process/switch.rs b/kernel/process/switch.rs index 592c7938..efe4c723 100644 --- a/kernel/process/switch.rs +++ b/kernel/process/switch.rs @@ -1,7 +1,7 @@ use super::*; use crate::process::PId; use crate::{ - arch::{self, enable_interrupt, is_interrupt_enabled}, + arch::{self}, process::process::PROCESSES, }; @@ -11,10 +11,6 @@ use core::mem::{self}; /// Yields execution to another thread. pub fn switch() { - // Save the current interrupt enable flag to restore it in the next execution - // of the currently running thread. - let interrupt_enabled = is_interrupt_enabled(); - let prev = current_process().clone(); let prev_pid = prev.pid(); let prev_state = prev.state(); @@ -46,10 +42,17 @@ pub fn switch() { lock.page_table().switch(); } - // Drop `next_thread` here because `switch_thread` won't return when the current - // process is being destroyed (e.g. by exit(2)) and it leads to a memory leak. + // Drop `prev` and `next` here because `switch_thread` won't return when the + // current process is being destroyed (e.g. by exit(2)). + // + // Since processes are referenced from at least the following two places, + // we can safely decrement reference counts without immediately drop here: + // + // - prev or next: they holds Arc (not &Arc). + // - Their parent processes's list of children. // // To cheat the borrow checker we do so by `Arc::decrement_strong_count`. + debug_assert!(Arc::strong_count(&prev) > 1); debug_assert!(Arc::strong_count(&next) > 1); unsafe { Arc::decrement_strong_count(Arc::as_ptr(&prev)); @@ -64,14 +67,4 @@ pub fn switch() { // reference count by `Arc::decrement_strong_count` above. mem::forget(prev); mem::forget(next); - - // Now we're in the next thread. Release held locks and continue executing. - - // Retstore the interrupt enable flag manually because lock guards - // (`prev` and `next`) that holds the flag state are `mem::forget`-ed. - if interrupt_enabled { - unsafe { - enable_interrupt(); - } - } } From c2ba74bb710da3f83be2002e09bae6e54f923681 Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Sun, 7 Nov 2021 23:39:52 +0900 Subject: [PATCH 2/4] Fix compile errors --- kernel/arch/x64/interrupt.rs | 2 +- kernel/arch/x64/mod.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/arch/x64/interrupt.rs b/kernel/arch/x64/interrupt.rs index 5e4b6734..638b0991 100644 --- a/kernel/arch/x64/interrupt.rs +++ b/kernel/arch/x64/interrupt.rs @@ -8,7 +8,7 @@ use crate::{ timer::handle_timer_irq, }; -use x86::{controlregs::cr2, current::rflags::RFlags, irq::*}; +use x86::{controlregs::cr2, irq::*}; /// The interrupt stack frame. #[derive(Debug, Copy, Clone)] diff --git a/kernel/arch/x64/mod.rs b/kernel/arch/x64/mod.rs index aafa1d0e..cdea9fdf 100644 --- a/kernel/arch/x64/mod.rs +++ b/kernel/arch/x64/mod.rs @@ -36,7 +36,6 @@ pub use arch_prctl::arch_prctl; pub use backtrace::Backtrace; pub use boot::init; pub use idle::{halt, idle}; -pub use interrupt::{enable_interrupt, is_interrupt_enabled}; pub use ioapic::enable_irq; pub use lock::{SpinLock, SpinLockGuard}; pub use page_table::{PageFaultReason, PageTable}; From 9a7ebc8cb9b8bd807c4385cbb0057f5359646347 Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Mon, 8 Nov 2021 00:55:14 +0900 Subject: [PATCH 3/4] Embed the symbol table into unit testing kernel images --- Makefile | 5 ++++- kernel/main.rs | 1 + kernel/printk.rs | 7 ++++--- kernel/test_runner.rs | 19 +++++++++---------- tools/run-unittests.sh | 7 +++++++ 5 files changed, 25 insertions(+), 14 deletions(-) create mode 100755 tools/run-unittests.sh diff --git a/Makefile b/Makefile index 86cffb18..36fa39fd 100644 --- a/Makefile +++ b/Makefile @@ -49,10 +49,13 @@ CARGOFLAGS += -Z build-std=core,alloc -Z build-std-features=compiler-builtins-me CARGOFLAGS += --target $(target_json) CARGOFLAGS += $(if $(RELEASE),--release,) TESTCARGOFLAGS += --package kerla -Z unstable-options -TESTCARGOFLAGS += --config "target.$(ARCH).runner = '$(PYTHON3) $(topdir)/tools/run-qemu.py --arch $(ARCH)'" +TESTCARGOFLAGS += --config "target.$(ARCH).runner = './tools/run-unittests.sh'" WATCHFLAGS += --clear export CARGO_FROM_MAKE=1 export INITRAMFS_PATH +export ARCH +export PYTHON3 +export NM # # Build Commands diff --git a/kernel/main.rs b/kernel/main.rs index e49af6a0..238ac778 100644 --- a/kernel/main.rs +++ b/kernel/main.rs @@ -9,6 +9,7 @@ #![test_runner(crate::test_runner::run_tests)] #![reexport_test_harness_main = "test_main"] #![allow(clippy::upper_case_acronyms)] +#![allow(clippy::print_with_newline)] // FIXME: #![allow(unaligned_references)] diff --git a/kernel/printk.rs b/kernel/printk.rs index 1e4d556f..af44814a 100644 --- a/kernel/printk.rs +++ b/kernel/printk.rs @@ -2,7 +2,6 @@ use alloc::boxed::Box; use arrayvec::ArrayVec; use kerla_utils::ring_buffer::RingBuffer; -use crate::arch::SpinLock; use crate::arch::{print_str, printchar, Backtrace, VAddr}; use crate::lang_items::PANICKED; use core::mem::size_of; @@ -12,8 +11,10 @@ use core::{fmt, slice}; pub struct Printer; pub const KERNEL_LOG_BUF_SIZE: usize = 8192; -pub static KERNEL_LOG_BUF: SpinLock> = - SpinLock::new(RingBuffer::new()); +// We use spin::Mutex here because SpinLock's debugging features may cause a +// problem (capturing a backtrace requires memory allocation). +pub static KERNEL_LOG_BUF: spin::Mutex> = + spin::Mutex::new(RingBuffer::new()); impl core::fmt::Write for Printer { fn write_char(&mut self, c: char) -> core::fmt::Result { diff --git a/kernel/test_runner.rs b/kernel/test_runner.rs index 1aa7e69e..09ba44d0 100644 --- a/kernel/test_runner.rs +++ b/kernel/test_runner.rs @@ -2,7 +2,7 @@ use crate::arch::*; use core::panic::PanicInfo; -use core::sync::atomic::{AtomicBool, Ordering}; +use core::sync::atomic::Ordering; pub trait Testable { fn run(&self); @@ -15,7 +15,7 @@ where fn run(&self) { print!("{} ... ", core::any::type_name::()); self(); - println!("\x1b[92mok\x1b[0m"); + print!("\x1b[92mok\x1b[0m\n"); } } @@ -24,8 +24,8 @@ pub fn run_tests(tests: &[&dyn Testable]) { for test in tests { test.run(); } - println!(); - println!("\x1b[92mPassed all tests :)\x1b[0m\n"); + print!("\n"); + print!("\x1b[92mPassed all tests :)\x1b[0m\n"); } pub fn end_tests() -> ! { @@ -35,18 +35,17 @@ pub fn end_tests() -> ! { loop {} } -static ALREADY_PANICED: AtomicBool = AtomicBool::new(false); - #[cfg(test)] #[panic_handler] fn panic(info: &PanicInfo) -> ! { - if ALREADY_PANICED.load(Ordering::SeqCst) { + use crate::lang_items::PANICKED; + + if PANICKED.load(Ordering::SeqCst) { loop {} } - ALREADY_PANICED.store(true, Ordering::SeqCst); - - print!("\x1b[1;91mfail\x1b[0m\n{}\n", info); + PANICKED.store(true, Ordering::SeqCst); + print!("\x1b[1;91mfail\npanic: {}\x1b[0m", info); semihosting_halt(ExitStatus::Failure); loop {} } diff --git a/tools/run-unittests.sh b/tools/run-unittests.sh new file mode 100755 index 00000000..139fc1ae --- /dev/null +++ b/tools/run-unittests.sh @@ -0,0 +1,7 @@ +#!/bin/sh +set -ue + +executable="$1" +$NM $executable | rustfilt | awk '{ $2=""; print $0 }' > $executable.symbols +$PYTHON3 ../tools/embed-symbol-table.py $executable.symbols $executable +$PYTHON3 ../tools/run-qemu.py --arch $ARCH $executable From 1794ba494327a780924a4e04d87af5d0cdbe99eb Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Mon, 8 Nov 2021 01:03:24 +0900 Subject: [PATCH 4/4] Allow clippy::print_with_newline --- kernel/main.rs | 1 - kernel/test_runner.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/main.rs b/kernel/main.rs index 238ac778..e49af6a0 100644 --- a/kernel/main.rs +++ b/kernel/main.rs @@ -9,7 +9,6 @@ #![test_runner(crate::test_runner::run_tests)] #![reexport_test_harness_main = "test_main"] #![allow(clippy::upper_case_acronyms)] -#![allow(clippy::print_with_newline)] // FIXME: #![allow(unaligned_references)] diff --git a/kernel/test_runner.rs b/kernel/test_runner.rs index 09ba44d0..0eaf7f94 100644 --- a/kernel/test_runner.rs +++ b/kernel/test_runner.rs @@ -1,4 +1,5 @@ #![cfg(test)] +#![allow(clippy::print_with_newline)] use crate::arch::*; use core::panic::PanicInfo;