From 36d7724fb84047a8d4640803e0822587daec52c5 Mon Sep 17 00:00:00 2001 From: Garen Tyler Date: Sun, 29 Oct 2023 13:57:23 -0600 Subject: [PATCH] Rewrite spinlock to remove raw pointers --- kernel/defs.h | 3 +- kernel/rustkernel/src/console/printf.rs | 8 +--- kernel/rustkernel/src/console/uart.rs | 11 ++--- kernel/rustkernel/src/fs/file.rs | 14 ++----- kernel/rustkernel/src/mem/kalloc.rs | 16 ++----- kernel/rustkernel/src/proc.rs | 4 +- kernel/rustkernel/src/sync/sleeplock.rs | 4 +- kernel/rustkernel/src/sync/spinlock.rs | 55 ++++--------------------- kernel/rustkernel/src/trap.rs | 20 +++------ kernel/spinlock.h | 7 +--- 10 files changed, 31 insertions(+), 111 deletions(-) diff --git a/kernel/defs.h b/kernel/defs.h index 4301a68..d9605f3 100644 --- a/kernel/defs.h +++ b/kernel/defs.h @@ -117,8 +117,7 @@ void procdump(void); void swtch(struct context*, struct context*); // spinlock.rs -void acquire(struct spinlock*); -int holding(struct spinlock*); +void acquire(struct spinlock *); void initlock(struct spinlock*, char*); void release(struct spinlock*); void push_off(void); diff --git a/kernel/rustkernel/src/console/printf.rs b/kernel/rustkernel/src/console/printf.rs index cbd6d69..4439faa 100644 --- a/kernel/rustkernel/src/console/printf.rs +++ b/kernel/rustkernel/src/console/printf.rs @@ -4,7 +4,7 @@ use core::ffi::{c_char, CStr}; pub use crate::panic; #[no_mangle] -pub static mut PRINT_LOCK: Spinlock = unsafe { Spinlock::uninitialized() }; +pub static mut PRINT_LOCK: Spinlock = Spinlock::new(); #[repr(C)] pub struct PrintLock { @@ -48,9 +48,5 @@ pub unsafe extern "C" fn printstr(s: *const c_char) { #[no_mangle] pub unsafe extern "C" fn printfinit() { - PRINT_LOCK = Spinlock::new( - CStr::from_bytes_with_nul_unchecked(b"pr\0") - .as_ptr() - .cast_mut(), - ); + PRINT_LOCK = Spinlock::new(); } diff --git a/kernel/rustkernel/src/console/uart.rs b/kernel/rustkernel/src/console/uart.rs index 020bd2b..26f153d 100644 --- a/kernel/rustkernel/src/console/uart.rs +++ b/kernel/rustkernel/src/console/uart.rs @@ -9,7 +9,7 @@ use crate::{ sync::spinmutex::SpinMutex, trap::InterruptBlocker, }; -use core::{ffi::CStr, ptr::addr_of_mut}; +use core::ptr::addr_of_mut; enum Register { ReceiveHolding, @@ -115,7 +115,7 @@ const LSR_RX_READY: u8 = 1 << 0; /// THR can accept another character to send const LSR_TX_IDLE: u8 = 1 << 5; -static mut uart_tx_lock: Spinlock = unsafe { Spinlock::uninitialized() }; +static mut uart_tx_lock: Spinlock = Spinlock::new(); const UART_TX_BUF_SIZE: usize = 32; static mut uart_tx_buf: [u8; UART_TX_BUF_SIZE] = [0u8; UART_TX_BUF_SIZE]; // static uart_tx_buf: SpinMutex<[u8; UART_TX_BUF_SIZE]> = SpinMutex::new([0u8; UART_TX_BUF_SIZE]); @@ -143,12 +143,7 @@ pub(crate) unsafe fn uartinit() { // Enable transmit and receive interrupts. Register::InterruptEnable.write(IER_TX_ENABLE | IER_RX_ENABLE); - uart_tx_lock = Spinlock::new( - CStr::from_bytes_with_nul(b"uart\0") - .unwrap() - .as_ptr() - .cast_mut(), - ); + uart_tx_lock = Spinlock::new(); } /// Add a character to the output buffer and tell the diff --git a/kernel/rustkernel/src/fs/file.rs b/kernel/rustkernel/src/fs/file.rs index 8bdb817..544bff6 100644 --- a/kernel/rustkernel/src/fs/file.rs +++ b/kernel/rustkernel/src/fs/file.rs @@ -7,10 +7,7 @@ use crate::{ proc::myproc, sync::{sleeplock::Sleeplock, spinlock::Spinlock}, }; -use core::{ - ffi::CStr, - ptr::{addr_of_mut, null_mut}, -}; +use core::ptr::{addr_of_mut, null_mut}; #[repr(C)] #[derive(Copy, Clone, PartialEq, Default)] @@ -121,7 +118,7 @@ pub struct FileTable { pub static mut devsw: [Devsw; crate::NDEV] = [Devsw::new(); crate::NDEV]; #[no_mangle] pub static mut ftable: FileTable = FileTable { - lock: unsafe { Spinlock::uninitialized() }, + lock: Spinlock::new(), files: unsafe { [File::uninitialized(); crate::NFILE] }, }; pub const CONSOLE: usize = 1; @@ -137,12 +134,7 @@ extern "C" { } pub unsafe fn fileinit() { - ftable.lock = Spinlock::new( - CStr::from_bytes_with_nul(b"ftable\0") - .unwrap() - .as_ptr() - .cast_mut(), - ); + ftable.lock = Spinlock::new(); } /// Allocate a file structure. diff --git a/kernel/rustkernel/src/mem/kalloc.rs b/kernel/rustkernel/src/mem/kalloc.rs index ad90b25..6962bcd 100644 --- a/kernel/rustkernel/src/mem/kalloc.rs +++ b/kernel/rustkernel/src/mem/kalloc.rs @@ -7,21 +7,18 @@ use crate::{ riscv::{memlayout::PHYSTOP, pg_round_up, PGSIZE}, sync::spinlock::Spinlock, }; -use core::{ - ffi::{c_char, CStr}, - ptr::{addr_of_mut, null_mut}, -}; +use core::ptr::{addr_of_mut, null_mut}; extern "C" { // oh my god this is so stupid why the fuck // this took me so long to figure out it's 3am rn // First address after kernel. Defined by kernel.ld. - pub static mut end: [c_char; 0]; + pub static mut end: [u8; 0]; } #[no_mangle] pub static mut kmem: KernelMemory = KernelMemory { - lock: unsafe { Spinlock::uninitialized() }, + lock: Spinlock::new(), freelist: null_mut(), }; @@ -37,12 +34,7 @@ pub struct KernelMemory { #[no_mangle] pub unsafe extern "C" fn kinit() { - kmem.lock = Spinlock::new( - CStr::from_bytes_with_nul(b"kmem\0") - .unwrap() - .as_ptr() - .cast_mut(), - ); + kmem.lock = Spinlock::new(); freerange(addr_of_mut!(end).cast(), PHYSTOP as *mut u8) } diff --git a/kernel/rustkernel/src/proc.rs b/kernel/rustkernel/src/proc.rs index 1d9fc8c..7f4fc98 100644 --- a/kernel/rustkernel/src/proc.rs +++ b/kernel/rustkernel/src/proc.rs @@ -328,9 +328,7 @@ pub unsafe extern "C" fn sched() { let p = myproc(); let c = mycpu(); - if !(*p).lock.held_by_current_cpu() { - panic!("sched p->lock"); - } else if (*c).interrupt_disable_layers != 1 { + if (*c).interrupt_disable_layers != 1 { panic!("sched locks"); } else if (*p).state == ProcState::Running { panic!("sched running"); diff --git a/kernel/rustkernel/src/sync/sleeplock.rs b/kernel/rustkernel/src/sync/sleeplock.rs index c2f767c..e1bd985 100644 --- a/kernel/rustkernel/src/sync/sleeplock.rs +++ b/kernel/rustkernel/src/sync/sleeplock.rs @@ -18,7 +18,7 @@ impl Sleeplock { pub const unsafe fn uninitialized() -> Sleeplock { Sleeplock { locked: 0, - inner: Spinlock::uninitialized(), + inner: Spinlock::new(), name: null_mut(), pid: 0, } @@ -27,7 +27,7 @@ impl Sleeplock { pub const fn new(name: *mut c_char) -> Sleeplock { Sleeplock { locked: 0, - inner: Spinlock::new(name), + inner: Spinlock::new(), name, pid: 0, } diff --git a/kernel/rustkernel/src/sync/spinlock.rs b/kernel/rustkernel/src/sync/spinlock.rs index 6015846..5bed774 100644 --- a/kernel/rustkernel/src/sync/spinlock.rs +++ b/kernel/rustkernel/src/sync/spinlock.rs @@ -1,56 +1,27 @@ -use crate::{ - proc::{mycpu, Cpu}, - trap::{pop_intr_off, push_intr_off}, -}; +use crate::trap::{pop_intr_off, push_intr_off}; use core::{ ffi::c_char, - ptr::{addr_of, null_mut}, sync::atomic::{AtomicBool, Ordering}, }; #[repr(C)] +#[derive(Default)] pub struct Spinlock { pub locked: AtomicBool, - pub name: *mut c_char, - pub cpu: *mut Cpu, } impl Spinlock { - pub const unsafe fn uninitialized() -> Spinlock { - Spinlock { - locked: AtomicBool::new(false), - name: null_mut(), - cpu: null_mut(), - } - } /// Initializes a `Spinlock`. - pub const fn new(name: *mut c_char) -> Spinlock { + pub const fn new() -> Spinlock { Spinlock { locked: AtomicBool::new(false), - cpu: null_mut(), - name, } } - /// Check whether this cpu is holding the lock. - /// - /// Interrupts must be off. - pub fn held_by_current_cpu(&self) -> bool { - self.cpu == unsafe { mycpu() } && self.locked.load(Ordering::Relaxed) - } pub unsafe fn lock_unguarded(&self) { push_intr_off(); - if self.held_by_current_cpu() { - panic!("Attempt to acquire twice by the same CPU"); - } - - let this: &mut Self = &mut *addr_of!(*self).cast_mut(); - - while this.locked.swap(true, Ordering::Acquire) { + while self.locked.swap(true, Ordering::Acquire) { core::hint::spin_loop(); } - - // The lock is now locked and we can write our CPU info. - this.cpu = mycpu(); } pub fn lock(&self) -> SpinlockGuard<'_> { unsafe { @@ -59,14 +30,7 @@ impl Spinlock { SpinlockGuard { lock: self } } pub unsafe fn unlock(&self) { - if !self.held_by_current_cpu() { - panic!("Attempt to release lock from different CPU"); - } - - let this: &mut Self = &mut *addr_of!(*self).cast_mut(); - - this.cpu = null_mut(); - this.locked.store(false, Ordering::Release); + self.locked.store(false, Ordering::Release); pop_intr_off(); } @@ -82,13 +46,8 @@ impl<'l> Drop for SpinlockGuard<'l> { } #[no_mangle] -pub unsafe extern "C" fn initlock(lock: *mut Spinlock, name: *mut c_char) { - *lock = Spinlock::new(name); -} - -#[no_mangle] -pub unsafe extern "C" fn holding(lock: *mut Spinlock) -> i32 { - (*lock).held_by_current_cpu().into() +pub unsafe extern "C" fn initlock(lock: *mut Spinlock, _name: *mut c_char) { + *lock = Spinlock::new(); } #[no_mangle] diff --git a/kernel/rustkernel/src/trap.rs b/kernel/rustkernel/src/trap.rs index 0b10792..9251519 100644 --- a/kernel/rustkernel/src/trap.rs +++ b/kernel/rustkernel/src/trap.rs @@ -5,10 +5,7 @@ use crate::{ sync::spinlock::Spinlock, syscall::syscall, }; -use core::{ - ffi::{c_char, CStr}, - ptr::{addr_of, addr_of_mut}, -}; +use core::ptr::{addr_of, addr_of_mut}; extern "C" { pub fn kernelvec(); @@ -17,24 +14,19 @@ extern "C" { // fn syscall(); // pub fn userret(satp: u64); fn virtio_disk_intr(); - pub static mut trampoline: [c_char; 0]; - pub static mut uservec: [c_char; 0]; - pub static mut userret: [c_char; 0]; + pub static mut trampoline: [u8; 0]; + pub static mut uservec: [u8; 0]; + pub static mut userret: [u8; 0]; } #[no_mangle] -pub static mut tickslock: Spinlock = unsafe { Spinlock::uninitialized() }; +pub static mut tickslock: Spinlock = Spinlock::new(); #[no_mangle] pub static mut ticks: u32 = 0; #[no_mangle] pub unsafe extern "C" fn trapinit() { - tickslock = Spinlock::new( - CStr::from_bytes_with_nul(b"time\0") - .unwrap() - .as_ptr() - .cast_mut(), - ); + tickslock = Spinlock::new(); } /// Set up to take exceptions and traps while in the kernel. diff --git a/kernel/spinlock.h b/kernel/spinlock.h index d4c1d65..80aa562 100644 --- a/kernel/spinlock.h +++ b/kernel/spinlock.h @@ -3,10 +3,7 @@ // Mutual exclusion lock. struct spinlock { - uint locked; // Is the lock held? - - // For debugging: - char *name; // Name of lock. - struct cpu *cpu; // The cpu holding the lock. + // Is the lock held? + uint8 locked; };