diff --git a/kernel/src/driver/pci/device.rs b/kernel/src/driver/pci/device.rs index e10d3968..fe5e54fb 100644 --- a/kernel/src/driver/pci/device.rs +++ b/kernel/src/driver/pci/device.rs @@ -59,7 +59,7 @@ impl Object for PciDevice { } let index = usize::try_from(offset - 1).ok()?; - let pci = PCI.lock(); + let pci = PCI.auto_lock(); let pci = pci.as_ref().unwrap(); let header = pci.get(self.bus, self.device, 0).unwrap(); let bar = header.base_addresses().get(index)?; diff --git a/kernel/src/driver/pci/table.rs b/kernel/src/driver/pci/table.rs index 30219b41..48f342e3 100644 --- a/kernel/src/driver/pci/table.rs +++ b/kernel/src/driver/pci/table.rs @@ -52,7 +52,7 @@ impl Object for PciTable { Ticket::new_complete( path_to_bdf(path) .and_then(|(bus, dev, func)| { - let pci = super::PCI.lock(); + let pci = super::PCI.auto_lock(); pci.as_ref() .unwrap() .get(bus, dev, func) @@ -99,7 +99,7 @@ impl Iterator for QueryTags { type Item = Ticket; fn next(&mut self) -> Option { - let pci = super::PCI.lock(); + let pci = super::PCI.auto_lock(); let pci = pci.as_ref().unwrap(); while self.index < 0x100 << 8 { let (bus, dev, func) = n_to_bdf(self.index.into()).unwrap(); diff --git a/kernel/src/driver/ps2/keyboard/table.rs b/kernel/src/driver/ps2/keyboard/table.rs index fc1a6f61..51cb348b 100644 --- a/kernel/src/driver/ps2/keyboard/table.rs +++ b/kernel/src/driver/ps2/keyboard/table.rs @@ -29,7 +29,7 @@ impl Object for ScancodeReader { fn read(&self, length: usize) -> Ticket> { if length == 0 { Ticket::new_complete(Ok([].into())) - } else if let Some(s) = super::EVENTS.lock().pop() { + } else if let Some(s) = super::EVENTS.auto_lock().pop() { Ticket::new_complete(Ok(<[u8; 4]>::from(s).into())) } else { let (ticket, waker) = Ticket::new(); diff --git a/kernel/src/memory/frame/mod.rs b/kernel/src/memory/frame/mod.rs index 3d13c68c..5d98eee0 100644 --- a/kernel/src/memory/frame/mod.rs +++ b/kernel/src/memory/frame/mod.rs @@ -203,7 +203,6 @@ where } /// Allocate a physically contiguous range of pages. -#[track_caller] pub fn allocate_contiguous(count: NonZeroUsize) -> Result { dumb_stack::STACK .auto_lock() diff --git a/kernel/src/memory/virtual/address_space.rs b/kernel/src/memory/virtual/address_space.rs index 81fb4fd4..59760268 100644 --- a/kernel/src/memory/virtual/address_space.rs +++ b/kernel/src/memory/virtual/address_space.rs @@ -75,6 +75,8 @@ impl AddressSpace { object: Arc, rwx: RWX, ) -> Result, MapError> { + // FIXME this will deadlock because there is now a circular dependency + // on the heap allocator let mut objects = KERNEL_MAPPED_OBJECTS.auto_lock(); let (range, frames, index) = Self::map_object_common( @@ -145,10 +147,12 @@ impl AddressSpace { count: NonZeroUsize, ) -> Result<(), UnmapError> { let mut objects = KERNEL_MAPPED_OBJECTS.auto_lock(); - unsafe { - Self::unmap_object_common(&mut objects, base, count)?; + let obj = unsafe { r#virtual::AddressSpace::kernel_unmap(base, count).unwrap(); - } + Self::unmap_object_common(&mut objects, base, count)? + }; + drop(objects); // Release now to avoid deadlock + drop(obj); Ok(()) } @@ -156,9 +160,9 @@ impl AddressSpace { objects: &mut Vec<(RangeInclusive>, Arc)>, base: NonNull, count: NonZeroUsize, - ) -> Result<(), UnmapError> { + ) -> Result>, UnmapError> { let i = objects.iter().position(|e| e.0.contains(&base)).unwrap(); - let (range, _obj) = &objects[i]; + let (range, _) = &objects[i]; let end = base .as_ptr() .wrapping_add(count.get()) @@ -167,11 +171,10 @@ impl AddressSpace { .cast(); let unmap_range = base..=NonNull::new(end).unwrap(); if &unmap_range == range { - objects.remove(i); + Ok(Some(objects.remove(i).1)) } else { todo!("partial unmap"); } - Ok(()) } /// Map a virtual address to a physical address. diff --git a/kernel/src/object_table/job.rs b/kernel/src/object_table/job.rs index 07a6a987..cd187be7 100644 --- a/kernel/src/object_table/job.rs +++ b/kernel/src/object_table/job.rs @@ -71,7 +71,7 @@ impl JobTask { impl Drop for JobTask { fn drop(&mut self) { - match mem::replace(&mut *self.shared.lock(), JobInner::Cancelled) { + match mem::replace(&mut *self.shared.auto_lock(), JobInner::Cancelled) { JobInner::Active { job, table, .. } => { job.map(|job| Weak::upgrade(&table).map(|t| t.cancel_job(job.0))); } @@ -84,7 +84,7 @@ impl Future for JobTask { type Output = Result<(JobId, JobRequest), Cancelled>; fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll { - match &mut *self.shared.lock() { + match &mut *self.shared.auto_lock() { JobInner::Active { waker, job, .. } => { if let Some(s) = job.take() { Poll::Ready(Ok(s)) diff --git a/kernel/src/object_table/root.rs b/kernel/src/object_table/root.rs index ad5dca5b..23f4fb7f 100644 --- a/kernel/src/object_table/root.rs +++ b/kernel/src/object_table/root.rs @@ -1,5 +1,5 @@ use super::{Error, Object, Query, QueryResult, StreamingTable, Ticket}; -use crate::sync::Mutex; +use crate::sync::SpinLock; use alloc::{ boxed::Box, collections::BTreeMap, @@ -22,20 +22,20 @@ use alloc::{ /// process/ /// ``` pub struct Root { - objects: Mutex, Weak>>, + objects: SpinLock, Weak>>, } impl Root { /// Create a new root pub fn new() -> Self { Self { - objects: Mutex::new(BTreeMap::new()), + objects: SpinLock::new(BTreeMap::new()), } } /// Add a new object to the root. pub fn add(&self, name: impl Into>, object: Weak) { - self.objects.lock_unchecked().insert(name.into(), object); + self.objects.auto_lock().insert(name.into(), object); } fn find<'a>(&self, path: &'a [u8]) -> Option<(Arc, &'a [u8], &'a [u8])> { @@ -43,7 +43,7 @@ impl Root { .iter() .position(|c| *c == b'/') .map_or((path, &b""[..]), |i| (&path[..i], &path[i + 1..])); - let mut objects = self.objects.lock(); + let mut objects = self.objects.auto_lock(); if let Some(obj) = objects.get(object) { if let Some(obj) = Weak::upgrade(&obj) { Some((obj, object, rest)) @@ -70,7 +70,7 @@ impl Object for Root { impl>> Query for Q {} // Filter any dead objects before querying. - let mut objects = self.objects.lock(); + let mut objects = self.objects.auto_lock(); objects.retain(|_, v| v.strong_count() > 0); Ticket::new_complete(Ok(Box::new(Q(objects .keys() @@ -104,7 +104,7 @@ impl Object for Root { Ticket::new_complete(if path.contains(&b'/') { Err(Error::DoesNotExist) } else { - let mut objects = self.objects.lock(); + let mut objects = self.objects.auto_lock(); let tbl = StreamingTable::new() as Arc; let r = objects.insert(path.into(), Arc::downgrade(&tbl)); assert!(r.is_none()); diff --git a/kernel/src/object_table/streaming.rs b/kernel/src/object_table/streaming.rs index 32729308..79361816 100644 --- a/kernel/src/object_table/streaming.rs +++ b/kernel/src/object_table/streaming.rs @@ -34,7 +34,7 @@ impl StreamingTable { let job_id = self.job_id_counter.fetch_add(1, Ordering::Relaxed); loop { - let j = self.job_handlers.lock().pop(); + let j = self.job_handlers.auto_lock().pop(); if let Some(w) = j { if let Ok(mut w) = w.lock() { self.tickets @@ -44,7 +44,7 @@ impl StreamingTable { break; } } else { - let mut l = self.jobs.lock(); + let mut l = self.jobs.auto_lock(); l.push((StreamJob { job_id, job }, Some(ticket_waker.into()), prefix)); break; } @@ -76,20 +76,20 @@ impl StreamingTable { impl Table for StreamingTable { fn take_job(self: Arc, _timeout: Duration) -> JobTask { - let job = self.jobs.lock().pop().map(|(job, tkt, prefix)| { + let job = self.jobs.auto_lock().pop().map(|(job, tkt, prefix)| { tkt.map(|tkt| self.tickets.auto_lock().push((job.job_id, tkt, prefix))); (job.job_id, job.job) }); let s = Arc::downgrade(&self); let (job, waker) = JobTask::new(s, job); - self.job_handlers.lock().push(waker); + self.job_handlers.auto_lock().push(waker); job } fn finish_job(self: Arc, job: Result, job_id: JobId) -> Result<(), ()> { let (tw, mut prefix); { - let mut c = self.tickets.lock(); + let mut c = self.tickets.auto_lock(); let mut c = c.drain_filter(|e| e.0 == job_id); (_, tw, prefix) = c.next().ok_or(())?; assert!(c.next().is_none()); diff --git a/kernel/src/object_table/ticket.rs b/kernel/src/object_table/ticket.rs index 23688484..4b3705d3 100644 --- a/kernel/src/object_table/ticket.rs +++ b/kernel/src/object_table/ticket.rs @@ -73,7 +73,7 @@ impl Future for Ticket { type Output = Result; fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll { - let mut t = self.inner.lock(); + let mut t = self.inner.auto_lock(); if let Some(s) = t.status.take() { return Poll::Ready(s); } diff --git a/kernel/src/scheduler/process/elf.rs b/kernel/src/scheduler/process/elf.rs index 56f9f2d6..073ecb6c 100644 --- a/kernel/src/scheduler/process/elf.rs +++ b/kernel/src/scheduler/process/elf.rs @@ -102,8 +102,8 @@ impl super::Process { core::slice::from_raw_parts(data[0].as_ptr().cast::(), Page::SIZE * data.len()) }; - let slf = Self::new()?; - *slf.objects.lock_unchecked() = objects; + let slf = Self::new().map_err(ElfError::AllocateError)?; + *slf.objects.auto_lock() = objects; (data.len() >= 16) .then(|| ()) diff --git a/kernel/src/scheduler/process/io.rs b/kernel/src/scheduler/process/io.rs index 1728debe..ee71a383 100644 --- a/kernel/src/scheduler/process/io.rs +++ b/kernel/src/scheduler/process/io.rs @@ -111,8 +111,8 @@ impl super::Process { .find(|q| q.user_ptr == base) .ok_or(ProcessQueueError::InvalidAddress)?; - let mut objects = self.objects.lock(); let mut queries = self.queries.lock(); + let mut objects = self.objects.lock(); // Poll tickets first as it may shrink the ticket Vec. poll_tickets(queue, &mut objects, &mut queries); @@ -382,9 +382,13 @@ impl super::Process { break; } - let polls = poll_tickets(queue, &mut self.objects.lock(), &mut self.queries.lock()); - if polls > 0 { - break; + { + let mut queries = self.queries.lock(); + let mut objects = self.objects.auto_lock(); + let polls = poll_tickets(queue, &mut objects, &mut queries); + if polls > 0 { + break; + } } // Prevent blocking other threads. diff --git a/kernel/src/scheduler/process/mod.rs b/kernel/src/scheduler/process/mod.rs index 273c8c98..48c3300b 100644 --- a/kernel/src/scheduler/process/mod.rs +++ b/kernel/src/scheduler/process/mod.rs @@ -22,7 +22,7 @@ pub struct Process { address_space: SpinLock, hint_color: u8, threads: SpinLock, u8>>, - objects: Mutex, u8>>, + objects: SpinLock, u8>>, queries: Mutex, u8>>, io_queues: Mutex>, } @@ -52,7 +52,7 @@ impl From for TicketOrJob { } impl Process { - fn new() -> Result { + fn new() -> Result { Ok(Self { address_space: SpinLock::new(AddressSpace::new()?), hint_color: 0, diff --git a/kernel/src/sync/mutex.rs b/kernel/src/sync/mutex.rs index ce5d521b..9c714a37 100644 --- a/kernel/src/sync/mutex.rs +++ b/kernel/src/sync/mutex.rs @@ -24,11 +24,6 @@ impl Mutex { crate::arch::interrupts_enabled(), "interrupts are disabled. Is the mutex being locked inside an ISR?" ); - self.lock_unchecked() - } - - #[track_caller] - pub fn lock_unchecked(&self) -> Guard { // TODO detect double locks by same thread loop { match self @@ -81,6 +76,11 @@ impl DerefMut for Guard<'_, T> { impl Drop for Guard<'_, T> { fn drop(&mut self) { + debug_assert_ne!( + self.lock.lock.load(Ordering::Relaxed), + 0, + "lock was released" + ); self.lock.lock.store(0, Ordering::Release); } } diff --git a/kernel/src/sync/spinlock.rs b/kernel/src/sync/spinlock.rs index c7aa30d0..d57969c3 100644 --- a/kernel/src/sync/spinlock.rs +++ b/kernel/src/sync/spinlock.rs @@ -140,6 +140,11 @@ impl DerefMut for Guard<'_, T> { impl Drop for Guard<'_, T> { fn drop(&mut self) { ensure_interrupts_off(); + debug_assert_ne!( + self.lock.lock.load(Ordering::Relaxed), + 0, + "lock was released" + ); self.lock.lock.store(0, Ordering::Release); crate::arch::enable_interrupts(); } @@ -162,6 +167,11 @@ impl DerefMut for IsrGuard<'_, T> { impl Drop for IsrGuard<'_, T> { fn drop(&mut self) { ensure_interrupts_off(); + debug_assert_ne!( + self.lock.lock.load(Ordering::Relaxed), + 0, + "lock was released" + ); self.lock.lock.store(0, Ordering::Release); } }