From 0f48379e1aa3e0e0593411ba58e0ffff5567fda2 Mon Sep 17 00:00:00 2001 From: Mark Poliakov <mark@alnyan.me> Date: Fri, 12 Nov 2021 19:44:10 +0200 Subject: [PATCH] refactor: better ABI for SignalDestination passing --- kernel/src/proc/mod.rs | 3 +- kernel/src/proc/process.rs | 134 ++++++++----------------------------- kernel/src/proc/wait.rs | 4 +- kernel/src/syscall/mod.rs | 21 +++--- libsys/src/calls.rs | 6 +- libsys/src/lib.rs | 13 ++-- libsys/src/proc.rs | 116 ++++++++++++++++++++++++++++++++ libsys/src/signal.rs | 34 ++++++++++ libusr/src/lib.rs | 2 +- user/src/shell/main.rs | 4 +- 10 files changed, 205 insertions(+), 132 deletions(-) create mode 100644 libsys/src/proc.rs diff --git a/kernel/src/proc/mod.rs b/kernel/src/proc/mod.rs index 42f084f..4ddc628 100644 --- a/kernel/src/proc/mod.rs +++ b/kernel/src/proc/mod.rs @@ -3,10 +3,11 @@ use crate::init; use crate::sync::IrqSafeSpinLock; use alloc::collections::BTreeMap; +use libsys::proc::Pid; pub mod elf; pub mod process; -pub use process::{Pid, Process, ProcessRef, State as ProcessState}; +pub use process::{Process, ProcessRef, State as ProcessState}; pub mod io; pub use io::ProcessIo; diff --git a/kernel/src/proc/process.rs b/kernel/src/proc/process.rs index 23c782a..88d0c04 100644 --- a/kernel/src/proc/process.rs +++ b/kernel/src/proc/process.rs @@ -11,23 +11,13 @@ use alloc::rc::Rc; use core::cell::UnsafeCell; use core::fmt; use core::sync::atomic::{AtomicU32, Ordering}; -use libsys::{error::Errno, signal::Signal}; +use libsys::{error::Errno, signal::Signal, proc::{ExitCode, Pid}}; pub use crate::arch::platform::context::{self, Context}; /// Wrapper type for a process struct reference pub type ProcessRef = Rc<Process>; -/// Wrapper type for process exit code -#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)] -#[repr(transparent)] -pub struct ExitCode(i32); - -/// Wrapper type for process ID -#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] -#[repr(transparent)] -pub struct Pid(u32); - /// List of possible process states #[derive(Clone, Copy, Debug, PartialEq)] pub enum State { @@ -64,93 +54,6 @@ pub struct Process { pub io: IrqSafeSpinLock<ProcessIo>, } -impl From<i32> for ExitCode { - fn from(f: i32) -> Self { - Self(f) - } -} - -impl From<()> for ExitCode { - fn from(_: ()) -> Self { - Self(0) - } -} - -impl From<ExitCode> for i32 { - fn from(f: ExitCode) -> Self { - f.0 - } -} - -impl Pid { - /// Kernel idle process always has PID of zero - pub const IDLE: Self = Self(Self::KERNEL_BIT); - - const KERNEL_BIT: u32 = 1 << 31; - - /// Constructs an instance of user-space PID - pub const fn user(id: u32) -> Self { - assert!(id < 256, "PID is too high"); - Self(id) - } - - /// Allocates a new kernel-space PID - pub fn new_kernel() -> Self { - static LAST: AtomicU32 = AtomicU32::new(0); - let id = LAST.fetch_add(1, Ordering::Relaxed); - assert!(id & Self::KERNEL_BIT == 0, "Out of kernel PIDs"); - Self(id | Self::KERNEL_BIT) - } - - /// Allocates a new user-space PID. - /// - /// First user PID is #1. - pub fn new_user() -> Self { - static LAST: AtomicU32 = AtomicU32::new(1); - let id = LAST.fetch_add(1, Ordering::Relaxed); - assert!(id < 256, "Out of user PIDs"); - Self(id) - } - - /// Returns `true` if this PID belongs to a kernel process - pub fn is_kernel(self) -> bool { - self.0 & Self::KERNEL_BIT != 0 - } - - /// Returns address space ID of a user-space process. - /// - /// Panics if called on kernel process PID. - pub fn asid(self) -> u8 { - assert!(!self.is_kernel()); - self.0 as u8 - } - - /// Returns bit value of this pid - pub const fn value(self) -> u32 { - self.0 - } - - /// Constructs [Pid] from raw [u32] value - /// - /// # Safety - /// - /// Unsafe: does not check `num` - pub const unsafe fn from_raw(num: u32) -> Self { - Self(num) - } -} - -impl fmt::Display for Pid { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "Pid(#{}{})", - if self.is_kernel() { "K" } else { "U" }, - self.0 & !Self::KERNEL_BIT - ) - } -} - impl Process { const USTACK_VIRT_TOP: usize = 0x100000000; const USTACK_PAGES: usize = 4; @@ -341,7 +244,7 @@ impl Process { /// Creates a new kernel process pub fn new_kernel(entry: extern "C" fn(usize) -> !, arg: usize) -> Result<ProcessRef, Errno> { - let id = Pid::new_kernel(); + let id = new_kernel_pid(); let res = Rc::new(Self { ctx: UnsafeCell::new(Context::kernel(entry as usize, arg)), signal_ctx: UnsafeCell::new(Context::empty()), @@ -359,7 +262,7 @@ impl Process { state: State::Ready, }), }); - debugln!("New kernel process: {}", id); + debugln!("New kernel process: {:?}", id); assert!(PROCESSES.lock().insert(id, res.clone()).is_none()); Ok(res) } @@ -370,7 +273,7 @@ impl Process { let src_io = self.io.lock(); let mut src_inner = self.inner.lock(); - let dst_id = Pid::new_user(); + let dst_id = new_user_pid(); let dst_space = src_inner.space.as_mut().unwrap().fork()?; let dst_space_phys = (dst_space as *mut _ as usize) - mem::KERNEL_OFFSET; let dst_ttbr0 = dst_space_phys | ((dst_id.asid() as usize) << 48); @@ -392,7 +295,7 @@ impl Process { wait_flag: false, }), }); - debugln!("Process {} forked into {}", src_inner.id, dst_id); + debugln!("Process {:?} forked into {:?}", src_inner.id, dst_id); assert!(PROCESSES.lock().insert(dst_id, dst).is_none()); SCHED.enqueue(dst_id); @@ -405,7 +308,7 @@ impl Process { let drop = { let mut lock = self.inner.lock(); let drop = lock.state == State::Running; - infoln!("Process {} is exiting: {:?}", lock.id, status); + infoln!("Process {:?} is exiting: {:?}", lock.id, status); assert!(lock.exit.is_none()); lock.exit = Some(status); lock.state = State::Finished; @@ -450,7 +353,7 @@ impl Process { if let Some(r) = proc.collect() { // TODO drop the process struct itself PROCESSES.lock().remove(&proc.id()); - debugln!("pid {} has {} refs", proc.id(), Rc::strong_count(&proc)); + debugln!("pid {:?} has {} refs", proc.id(), Rc::strong_count(&proc)); return Ok(r); } @@ -477,9 +380,9 @@ impl Process { proc_lock.remove(&old_pid).is_some(), "Failed to downgrade kernel process (remove kernel pid)" ); - lock.id = Pid::new_user(); + lock.id = new_user_pid(); debugln!( - "Process downgrades from kernel to user: {} -> {}", + "Process downgrades from kernel to user: {:?} -> {:?}", old_pid, lock.id ); @@ -541,6 +444,23 @@ impl Process { impl Drop for Process { fn drop(&mut self) { - debugln!("Dropping process {}", self.id()); + debugln!("Dropping process {:?}", self.id()); } } + +/// Allocates a new kernel-space PID +pub fn new_kernel_pid() -> Pid { + static LAST: AtomicU32 = AtomicU32::new(0); + let id = LAST.fetch_add(1, Ordering::Relaxed); + Pid::kernel(id) +} + +/// Allocates a new user-space PID. +/// +/// First user PID is #1. +pub fn new_user_pid() -> Pid { + static LAST: AtomicU32 = AtomicU32::new(1); + let id = LAST.fetch_add(1, Ordering::Relaxed); + assert!(id < 256, "Out of user PIDs"); + Pid::user(id) +} diff --git a/kernel/src/proc/wait.rs b/kernel/src/proc/wait.rs index a945b97..59e06b2 100644 --- a/kernel/src/proc/wait.rs +++ b/kernel/src/proc/wait.rs @@ -2,11 +2,11 @@ use crate::arch::machine; use crate::dev::timer::TimestampSource; -use crate::proc::{self, sched::SCHED, Pid, Process, ProcessRef}; +use crate::proc::{self, sched::SCHED, Process, ProcessRef}; use crate::sync::IrqSafeSpinLock; use alloc::collections::LinkedList; use core::time::Duration; -use libsys::{error::Errno, stat::FdSet}; +use libsys::{error::Errno, stat::FdSet, proc::Pid}; /// Wait channel structure. Contains a queue of processes /// waiting for some event to happen. diff --git a/kernel/src/syscall/mod.rs b/kernel/src/syscall/mod.rs index 5e69e4c..b42d196 100644 --- a/kernel/src/syscall/mod.rs +++ b/kernel/src/syscall/mod.rs @@ -2,7 +2,7 @@ use crate::arch::platform::exception::ExceptionFrame; use crate::debug::Level; -use crate::proc::{elf, wait, Pid, Process, ProcessIo}; +use crate::proc::{elf, wait, Process, ProcessIo}; use core::cmp::Ordering; use core::mem::size_of; use core::ops::DerefMut; @@ -11,7 +11,8 @@ use libsys::{ abi, error::Errno, ioctl::IoctlCmd, - signal::Signal, + proc::Pid, + signal::{Signal, SignalDestination}, stat::{FdSet, FileMode, OpenFlags, Stat, AT_EMPTY_PATH, AT_FDCWD}, traits::{Read, Write}, }; @@ -182,16 +183,16 @@ pub fn syscall(num: usize, args: &[usize]) -> Result<usize, Errno> { panic!("This code won't run"); } abi::SYS_EX_KILL => { - let pid = args[0] as i32; + let target = SignalDestination::from(args[0] as isize); let signal = Signal::try_from(args[1] as u32)?; - let proc = match pid.cmp(&0) { - Ordering::Greater => { - Process::get(unsafe { Pid::from_raw(pid as u32) }).ok_or(Errno::DoesNotExist)? - } - Ordering::Equal => Process::current(), - Ordering::Less => todo!(), + + match target { + SignalDestination::This => Process::current().set_signal(signal), + SignalDestination::Process(pid) => Process::get(pid) + .ok_or(Errno::DoesNotExist)? + .set_signal(signal), + _ => todo!(), }; - proc.set_signal(signal); Ok(0) } diff --git a/libsys/src/calls.rs b/libsys/src/calls.rs index a4cd887..8ea597f 100644 --- a/libsys/src/calls.rs +++ b/libsys/src/calls.rs @@ -1,7 +1,7 @@ use crate::abi; use crate::{ ioctl::IoctlCmd, - signal::Signal, + signal::{Signal, SignalDestination}, stat::{FdSet, FileMode, OpenFlags, Stat}, }; @@ -214,8 +214,8 @@ pub unsafe fn sys_ex_sigreturn() -> ! { } #[inline(always)] -pub unsafe fn sys_ex_kill(pid: u32, signum: Signal) -> i32 { - syscall!(abi::SYS_EX_KILL, argn!(pid), argn!(signum as u32)) as i32 +pub unsafe fn sys_ex_kill(pid: SignalDestination, signum: Signal) -> i32 { + syscall!(abi::SYS_EX_KILL, argn!(isize::from(pid)), argn!(signum as u32)) as i32 } #[inline(always)] diff --git a/libsys/src/lib.rs b/libsys/src/lib.rs index c6a4034..71f984e 100644 --- a/libsys/src/lib.rs +++ b/libsys/src/lib.rs @@ -1,17 +1,18 @@ -#![feature(asm)] +#![feature(asm, const_panic)] #![no_std] #[macro_use] extern crate bitflags; pub mod abi; -pub mod stat; -pub mod ioctl; -pub mod termios; -pub mod signal; pub mod error; -pub mod path; +pub mod ioctl; pub mod mem; +pub mod path; +pub mod proc; +pub mod signal; +pub mod stat; +pub mod termios; pub mod traits; #[cfg(feature = "user")] diff --git a/libsys/src/proc.rs b/libsys/src/proc.rs new file mode 100644 index 0000000..e63b6dc --- /dev/null +++ b/libsys/src/proc.rs @@ -0,0 +1,116 @@ +use crate::error::Errno; +use core::convert::TryFrom; +use core::fmt; + +/// Wrapper type for process exit code +#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)] +#[repr(transparent)] +pub struct ExitCode(i32); + +/// Wrapper type for process ID +#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq)] +#[repr(transparent)] +pub struct Pid(u32); + +#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug)] +#[repr(transparent)] +pub struct Pgid(u32); + +impl From<i32> for ExitCode { + fn from(f: i32) -> Self { + Self(f) + } +} + +impl From<()> for ExitCode { + fn from(_: ()) -> Self { + Self(0) + } +} + +impl From<ExitCode> for i32 { + fn from(f: ExitCode) -> Self { + f.0 + } +} + +impl Pid { + /// Kernel idle process always has PID of zero + pub const IDLE: Self = Self(Self::KERNEL_BIT); + + const KERNEL_BIT: u32 = 1 << 31; + + /// Constructs an instance of user-space PID + pub const fn user(id: u32) -> Self { + assert!(id < 256, "PID is too high"); + Self(id) + } + + /// Constructs an instance of kernel-space PID + pub const fn kernel(id: u32) -> Self { + assert!(id & Self::KERNEL_BIT == 0, "PID is too high"); + Self(id | Self::KERNEL_BIT) + } + + /// Returns `true` if this PID belongs to a kernel process + pub fn is_kernel(self) -> bool { + self.0 & Self::KERNEL_BIT != 0 + } + + /// Returns address space ID of a user-space process. + /// + /// Panics if called on kernel process PID. + pub fn asid(self) -> u8 { + assert!(!self.is_kernel()); + self.0 as u8 + } + + /// Returns bit value of this pid + pub const fn value(self) -> u32 { + self.0 + } + + /// Constructs [Pid] from raw [u32] value + /// + /// # Safety + /// + /// Unsafe: does not check `num` + pub const unsafe fn from_raw(num: u32) -> Self { + Self(num) + } +} + +impl fmt::Debug for Pid { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "Pid(#{}{})", + if self.is_kernel() { "K" } else { "U" }, + self.0 & !Self::KERNEL_BIT + ) + } +} + +impl TryFrom<Pid> for Pgid { + type Error = Errno; + + fn try_from(pid: Pid) -> Result<Pgid, Errno> { + if pid.is_kernel() { + Err(Errno::InvalidArgument) + } else { + Ok(Pgid(pid.0)) + } + } +} + +impl From<u32> for Pgid { + fn from(p: u32) -> Pgid { + Self(p) + } +} + +impl From<Pgid> for u32 { + fn from(p: Pgid) -> u32 { + p.0 + } +} diff --git a/libsys/src/signal.rs b/libsys/src/signal.rs index e17d1c1..6d2dc3c 100644 --- a/libsys/src/signal.rs +++ b/libsys/src/signal.rs @@ -1,4 +1,5 @@ use crate::error::Errno; +use crate::proc::{Pid, Pgid}; #[derive(Clone, Copy, PartialEq, Debug)] #[repr(u32)] @@ -11,6 +12,39 @@ pub enum Signal { InvalidSystemCall = 31 } +#[derive(Clone, Copy, PartialEq, Debug)] +pub enum SignalDestination { + Group(Pgid), + Process(Pid), + All, + This +} + +impl From<isize> for SignalDestination { + fn from(num: isize) -> Self { + if num > 0 { + Self::Process(Pid::user(num as u32)) + } else if num == 0 { + Self::This + } else if num == -1 { + Self::All + } else { + Self::Group(Pgid::from((-num) as u32)) + } + } +} + +impl From<SignalDestination> for isize { + fn from(p: SignalDestination) -> isize { + match p { + SignalDestination::Process(pid) => pid.value() as isize, + SignalDestination::Group(pgid) => -(u32::from(pgid) as isize), + SignalDestination::This => 0, + SignalDestination::All => -1 + } + } +} + impl TryFrom<u32> for Signal { type Error = Errno; diff --git a/libusr/src/lib.rs b/libusr/src/lib.rs index 6f78ba2..01c383a 100644 --- a/libusr/src/lib.rs +++ b/libusr/src/lib.rs @@ -7,7 +7,7 @@ pub mod io; pub mod os; pub mod sys { - pub use libsys::signal::Signal; + pub use libsys::signal::{Signal, SignalDestination}; pub use libsys::termios; pub use libsys::calls::*; pub use libsys::stat::{self, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; diff --git a/user/src/shell/main.rs b/user/src/shell/main.rs index 8df7e49..08f1c8e 100644 --- a/user/src/shell/main.rs +++ b/user/src/shell/main.rs @@ -4,7 +4,7 @@ #[macro_use] extern crate libusr; -use libusr::sys::Signal; +use libusr::sys::{Signal, SignalDestination}; use libusr::sys::stat::FdSet; fn readline(fd: i32, buf: &mut [u8]) -> Result<&str, ()> { @@ -48,7 +48,7 @@ fn main() -> i32 { if line == "test" { unsafe { - libusr::sys::sys_ex_kill(0, Signal::Interrupt); + libusr::sys::sys_ex_kill(SignalDestination::This, Signal::Interrupt); } trace!("Returned from signal"); continue;