From fdf46d804343c3c1441038254ee860a25644832a Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Sat, 18 Apr 2020 08:23:40 -0600 Subject: [PATCH] Fix multi_core livelocks and add livelock debugging --- Cargo.toml | 2 +- src/context/context.rs | 6 +++++- src/event.rs | 2 +- src/ptrace.rs | 10 +++++++--- src/scheme/debug.rs | 2 +- src/scheme/pipe.rs | 28 +++++++++++++--------------- src/scheme/sys/block.rs | 36 ++++++++++++++++++++++++++++++++++++ src/scheme/sys/mod.rs | 2 ++ src/scheme/user.rs | 4 ++-- src/sync/wait_condition.rs | 10 ++++++---- src/sync/wait_map.rs | 18 +++++++++++------- src/sync/wait_queue.rs | 11 ++++++----- src/syscall/debug.rs | 2 +- src/syscall/futex.rs | 2 +- src/syscall/process.rs | 10 +++++----- src/syscall/time.rs | 2 +- 16 files changed, 99 insertions(+), 48 deletions(-) create mode 100644 src/scheme/sys/block.rs diff --git a/Cargo.toml b/Cargo.toml index d750a8c..56c27a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ version = "0.29.0" default-features = false [features] -default = ["serial_debug"] +default = ["acpi", "multi_core", "serial_debug"] acpi = [] doc = [] graphical_debug = [] diff --git a/src/context/context.rs b/src/context/context.rs index aca7da5..cb2eee0 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -118,6 +118,7 @@ pub struct Context { pub umask: usize, /// Status of context pub status: Status, + pub status_reason: &'static str, /// Context running or not pub running: bool, /// CPU ID, if locked @@ -197,6 +198,7 @@ impl Context { sigmask: [0; 2], umask: 0o022, status: Status::Blocked, + status_reason: "", running: false, cpu_id: None, ticks: 0, @@ -304,9 +306,10 @@ impl Context { } /// Block the context, and return true if it was runnable before being blocked - pub fn block(&mut self) -> bool { + pub fn block(&mut self, reason: &'static str) -> bool { if self.status == Status::Runnable { self.status = Status::Blocked; + self.status_reason = reason; true } else { false @@ -317,6 +320,7 @@ impl Context { pub fn unblock(&mut self) -> bool { if self.status == Status::Blocked { self.status = Status::Runnable; + self.status_reason = ""; if let Some(cpu_id) = self.cpu_id { if cpu_id != crate::cpu_id() { diff --git a/src/event.rs b/src/event.rs index 51f2b9d..f221bc2 100644 --- a/src/event.rs +++ b/src/event.rs @@ -26,7 +26,7 @@ impl EventQueue { } pub fn read(&self, events: &mut [Event]) -> Result { - self.queue.receive_into(events, true).ok_or(Error::new(EINTR)) + self.queue.receive_into(events, true, "EventQueue::read").ok_or(Error::new(EINTR)) } pub fn write(&self, events: &[Event]) -> Result { diff --git a/src/ptrace.rs b/src/ptrace.rs index 193c702..625cc6a 100644 --- a/src/ptrace.rs +++ b/src/ptrace.rs @@ -39,7 +39,7 @@ use core::{ cmp, sync::atomic::Ordering }; -use spin::{Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; +use spin::{Mutex, Once, RwLock, RwLockReadGuard, RwLockWriteGuard}; // ____ _ // / ___| ___ ___ ___(_) ___ _ __ ___ @@ -225,7 +225,9 @@ pub fn wait(pid: ContextId) -> Result<()> { } }; - while !tracer.wait() {} + //TODO: proper wait_condition mutex + let m = Mutex::new(()); + while !tracer.wait(m.lock(), "ptrace::wait") {} let contexts = context::contexts(); let context = contexts.get(pid).ok_or(Error::new(ESRCH))?; @@ -269,7 +271,9 @@ pub fn breakpoint_callback(match_flags: PtraceFlags, event: Option) ) }; - while !tracee.wait() {} + //TODO: proper wait_condition mutex + let m = Mutex::new(()); + while !tracee.wait(m.lock(), "ptrace::breakpoint_callback") {} Some(flags) } diff --git a/src/scheme/debug.rs b/src/scheme/debug.rs index f16860f..8c39c94 100644 --- a/src/scheme/debug.rs +++ b/src/scheme/debug.rs @@ -69,7 +69,7 @@ impl Scheme for DebugScheme { }; INPUT.call_once(init_input) - .receive_into(buf, flags & O_NONBLOCK != O_NONBLOCK) + .receive_into(buf, flags & O_NONBLOCK != O_NONBLOCK, "DebugScheme::read") .ok_or(Error::new(EINTR)) } diff --git a/src/scheme/pipe.rs b/src/scheme/pipe.rs index 3d6a0b3..46dd2ac 100644 --- a/src/scheme/pipe.rs +++ b/src/scheme/pipe.rs @@ -180,31 +180,29 @@ impl PipeRead { fn read(&self, buf: &mut [u8]) -> Result { loop { - { - let mut vec = self.vec.lock(); + let mut vec = self.vec.lock(); - let mut i = 0; - while i < buf.len() { - if let Some(b) = vec.pop_front() { - buf[i] = b; - i += 1; - } else { - break; - } + let mut i = 0; + while i < buf.len() { + if let Some(b) = vec.pop_front() { + buf[i] = b; + i += 1; + } else { + break; } + } - if i > 0 { - event::trigger(self.scheme_id, self.write_id, EVENT_WRITE); + if i > 0 { + event::trigger(self.scheme_id, self.write_id, EVENT_WRITE); - return Ok(i); - } + return Ok(i); } if Arc::weak_count(&self.vec) == 0 { return Ok(0); } else if self.flags.load(Ordering::SeqCst) & O_NONBLOCK == O_NONBLOCK { return Err(Error::new(EAGAIN)); - } else if ! self.condition.wait() { + } else if ! self.condition.wait(vec, "PipeRead::read") { return Err(Error::new(EINTR)); } } diff --git a/src/scheme/sys/block.rs b/src/scheme/sys/block.rs new file mode 100644 index 0000000..d751f3e --- /dev/null +++ b/src/scheme/sys/block.rs @@ -0,0 +1,36 @@ +use alloc::string::String; +use alloc::vec::Vec; +use core::fmt::Write; +use core::str; + +use crate::context; +use crate::syscall; +use crate::syscall::error::Result; + +pub fn resource() -> Result> { + let mut string = String::new(); + + { + let mut rows = Vec::new(); + { + let contexts = context::contexts(); + for (id, context_lock) in contexts.iter() { + let context = context_lock.read(); + rows.push((*id, context.name.lock().clone(), context.status_reason)); + } + } + + for row in rows.iter() { + let id: usize = row.0.into(); + let name = str::from_utf8(&row.1).unwrap_or("."); + + let _ = writeln!(string, "{}: {}", id, name); + + if ! row.2.is_empty() { + let _ = writeln!(string, " {}", row.2); + } + } + } + + Ok(string.into_bytes()) +} diff --git a/src/scheme/sys/mod.rs b/src/scheme/sys/mod.rs index 3cc4d59..d128b57 100644 --- a/src/scheme/sys/mod.rs +++ b/src/scheme/sys/mod.rs @@ -10,6 +10,7 @@ use crate::syscall::error::{Error, EBADF, EINVAL, ENOENT, Result}; use crate::syscall::flag::{MODE_DIR, MODE_FILE, SEEK_CUR, SEEK_END, SEEK_SET}; use crate::syscall::scheme::Scheme; +mod block; mod context; mod cpu; mod exe; @@ -40,6 +41,7 @@ impl SysScheme { pub fn new() -> SysScheme { let mut files: BTreeMap<&'static [u8], Box> = BTreeMap::new(); + files.insert(b"block", Box::new(block::resource)); files.insert(b"context", Box::new(context::resource)); files.insert(b"cpu", Box::new(cpu::resource)); files.insert(b"exe", Box::new(exe::resource)); diff --git a/src/scheme/user.rs b/src/scheme/user.rs index 32766d9..24278f5 100644 --- a/src/scheme/user.rs +++ b/src/scheme/user.rs @@ -97,7 +97,7 @@ impl UserInner { self.todo.send(packet); event::trigger(self.root_id, self.handle_id, EVENT_READ); - Error::demux(self.done.receive(&id)) + Error::demux(self.done.receive(&id, "UserInner::call_inner")) } pub fn capture(&self, buf: &[u8]) -> Result { @@ -202,7 +202,7 @@ impl UserInner { // If unmounting, do not block so that EOF can be returned immediately let unmounting = self.unmounting.load(Ordering::SeqCst); let block = !(nonblock || unmounting); - if let Some(count) = self.todo.receive_into(packet_buf, block) { + if let Some(count) = self.todo.receive_into(packet_buf, block, "UserInner::read") { if count > 0 { // If we received requests, return them to the scheme handler Ok(count * mem::size_of::()) diff --git a/src/sync/wait_condition.rs b/src/sync/wait_condition.rs index 2b68f9d..2865bd3 100644 --- a/src/sync/wait_condition.rs +++ b/src/sync/wait_condition.rs @@ -1,6 +1,6 @@ use alloc::sync::Arc; use alloc::vec::Vec; -use spin::{Mutex, RwLock}; +use spin::{Mutex, MutexGuard, RwLock}; use crate::context::{self, Context}; @@ -36,8 +36,8 @@ impl WaitCondition { len } - // Wait until notified. Returns false if resumed by a signal or the notify_signal function - pub fn wait(&self) -> bool { + // Wait until notified. Unlocks guard when blocking is ready. Returns false if resumed by a signal or the notify_signal function + pub fn wait(&self, guard: MutexGuard, reason: &'static str) -> bool { let id; { let context_lock = { @@ -49,10 +49,12 @@ impl WaitCondition { { let mut context = context_lock.write(); id = context.id; - context.block(); + context.block(reason); } self.contexts.lock().push(context_lock); + + drop(guard); } unsafe { context::switch(); } diff --git a/src/sync/wait_map.rs b/src/sync/wait_map.rs index 8f68337..8033758 100644 --- a/src/sync/wait_map.rs +++ b/src/sync/wait_map.rs @@ -22,13 +22,14 @@ impl WaitMap where K: Clone + Ord { self.inner.lock().remove(key) } - pub fn receive(&self, key: &K) -> V { + pub fn receive(&self, key: &K, reason: &'static str) -> V { loop { - if let Some(value) = self.receive_nonblock(key) { + let mut inner = self.inner.lock(); + if let Some(value) = inner.remove(key) { return value; } //TODO: use false from wait condition to indicate EINTR - let _ = self.condition.wait(); + let _ = self.condition.wait(inner, reason); } } @@ -41,12 +42,15 @@ impl WaitMap where K: Clone + Ord { } } - pub fn receive_any(&self) -> (K, V) { + pub fn receive_any(&self, reason: &'static str) -> (K, V) { loop { - if let Some(entry) = self.receive_any_nonblock() { - return entry; + let mut inner = self.inner.lock(); + if let Some(key) = inner.keys().next().cloned() { + if let Some(entry) = inner.remove(&key).map(|value| (key, value)) { + return entry; + } } - let _ = self.condition.wait(); + let _ = self.condition.wait(inner, reason); } } diff --git a/src/sync/wait_queue.rs b/src/sync/wait_queue.rs index 7ec622a..d20d581 100644 --- a/src/sync/wait_queue.rs +++ b/src/sync/wait_queue.rs @@ -28,22 +28,23 @@ impl WaitQueue { self.inner.lock().is_empty() } - pub fn receive(&self) -> Option { + pub fn receive(&self, reason: &'static str) -> Option { loop { - if let Some(value) = self.inner.lock().pop_front() { + let mut inner = self.inner.lock(); + if let Some(value) = inner.pop_front() { return Some(value); } - if ! self.condition.wait() { + if ! self.condition.wait(inner, reason) { return None; } } } - pub fn receive_into(&self, buf: &mut [T], block: bool) -> Option { + pub fn receive_into(&self, buf: &mut [T], block: bool, reason: &'static str) -> Option { let mut i = 0; if i < buf.len() && block { - buf[i] = self.receive()?; + buf[i] = self.receive(reason)?; i += 1; } diff --git a/src/syscall/debug.rs b/src/syscall/debug.rs index 270453c..e62d034 100644 --- a/src/syscall/debug.rs +++ b/src/syscall/debug.rs @@ -22,7 +22,7 @@ impl<'a> ::core::fmt::Debug for ByteStr<'a> { } } - +//TODO: calling format_call with arguments from another process space will not work pub fn format_call(a: usize, b: usize, c: usize, d: usize, e: usize, f: usize) -> String { match a { SYS_OPEN => format!( diff --git a/src/syscall/futex.rs b/src/syscall/futex.rs index 95da253..ee50c43 100644 --- a/src/syscall/futex.rs +++ b/src/syscall/futex.rs @@ -66,7 +66,7 @@ pub fn futex(addr: &mut i32, op: usize, val: i32, val2: usize, addr2: *mut i32) context.wake = Some(end); } - context.block(); + context.block("futex"); } futexes.push_back((addr as *mut i32 as usize, context_lock)); diff --git a/src/syscall/process.rs b/src/syscall/process.rs index 25c37fe..571fd17 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -355,7 +355,7 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result { let contexts = context::contexts(); let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let mut context = context_lock.write(); - context.block(); + context.block("vfork"); vfork = true; } else { vfork = false; @@ -1429,7 +1429,7 @@ pub fn sigreturn() -> Result { let context_lock = contexts.current().ok_or(Error::new(ESRCH))?; let mut context = context_lock.write(); context.ksig_restore = true; - context.block(); + context.block("sigreturn"); } let _ = unsafe { context::switch() }; @@ -1538,7 +1538,7 @@ pub fn waitpid(pid: ContextId, status_ptr: usize, flags: WaitFlags) -> Result Result Result) -> Result