From 9941f4b97fd25d77ab3dead19f5714c90c56be8e Mon Sep 17 00:00:00 2001 From: Seiya Nuta Date: Mon, 8 Nov 2021 08:59:00 +0900 Subject: [PATCH] Refactor context switching. NFCI (#61) * Refactor context switching. NFCI * Fix compile errors * Embed the symbol table into unit testing kernel images * Allow clippy::print_with_newline --- Makefile | 5 ++++- kernel/arch/x64/interrupt.rs | 10 +--------- kernel/arch/x64/mod.rs | 1 - kernel/printk.rs | 7 ++++--- kernel/process/switch.rs | 27 ++++++++++----------------- kernel/test_runner.rs | 20 ++++++++++---------- tools/run-unittests.sh | 7 +++++++ 7 files changed, 36 insertions(+), 41 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/arch/x64/interrupt.rs b/kernel/arch/x64/interrupt.rs index 5afd316f..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)] @@ -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/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}; 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/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(); - } - } } diff --git a/kernel/test_runner.rs b/kernel/test_runner.rs index 1aa7e69e..0eaf7f94 100644 --- a/kernel/test_runner.rs +++ b/kernel/test_runner.rs @@ -1,8 +1,9 @@ #![cfg(test)] +#![allow(clippy::print_with_newline)] 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 +16,7 @@ where fn run(&self) { print!("{} ... ", core::any::type_name::()); self(); - println!("\x1b[92mok\x1b[0m"); + print!("\x1b[92mok\x1b[0m\n"); } } @@ -24,8 +25,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 +36,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