From ae8e13525f91349fd3baa01993d7480e5b512bf5 Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Fri, 28 Jun 2024 16:42:03 +0200 Subject: [PATCH] Remove is_pending flag, read word directly instead. --- src/context/context.rs | 4 ---- src/context/signal.rs | 42 +++++++++++------------------------------- src/context/switch.rs | 23 +++++++---------------- src/main.rs | 5 +---- src/scheme/proc.rs | 1 - src/syscall/process.rs | 9 ++------- syscall | 2 +- 7 files changed, 22 insertions(+), 64 deletions(-) diff --git a/src/context/context.rs b/src/context/context.rs index 68f3ac6ca2..9105c99775 100644 --- a/src/context/context.rs +++ b/src/context/context.rs @@ -212,10 +212,6 @@ pub struct SignalState { /// Offset within the control pages of respective word-aligned structs. pub threadctl_off: u16, pub procctl_off: u16, - - /// Set whenever the kernel is about to have the context jump to its signal trampoline, but the - /// context is currently running. - pub is_pending: bool, } impl Context { diff --git a/src/context/signal.rs b/src/context/signal.rs index 2bbb54bec8..9986c50746 100644 --- a/src/context/signal.rs +++ b/src/context/signal.rs @@ -15,30 +15,6 @@ use crate::{ use super::ContextId; -pub fn kmain_signal_handler() { - /*if context::context_id() != ContextId::new(1) { - log::warn!("kmain signal didn't target PID 1, ignoring"); - return; - } - - let deliverable = context::current().expect("context::kmain_signal_handler not inside of context"); - let kstop_bit = 1 << (SIGKILL - 1); - let kreset_bit = 1 << (SIGTERM - 1); - let bits = deliverable.read().sig.deliverable(); - - if bits & kstop_bit == kstop_bit { - unsafe { - kstop(); - } - } else if bits & kreset_bit == kreset_bit { - unsafe { - kreset(); - } - } else { - log::warn!("Spurious kmain signal, bitmask {bits:#0x}."); - }*/ -} - pub fn signal_handler() { let context_lock = context::current().expect("running signal handler outside of context"); let mut context = context_lock.write(); @@ -48,11 +24,11 @@ pub fn signal_handler() { crate::syscall::process::exit(SIGKILL << 8); } - let thumbs_down = ptrace::breakpoint_callback( + /*let thumbs_down = ptrace::breakpoint_callback( PTRACE_STOP_SIGNAL, Some(ptrace_event!(PTRACE_STOP_SIGNAL)), ) - .and_then(|_| ptrace::next_breakpoint().map(|f| f.contains(PTRACE_FLAG_IGNORE))); + .and_then(|_| ptrace::next_breakpoint().map(|f| f.contains(PTRACE_FLAG_IGNORE)));*/ // TODO: thumbs_down let Some((thread_ctl, proc_ctl, st)) = context.sigcontrol() else { @@ -60,6 +36,15 @@ pub fn signal_handler() { log::trace!("no sigcontrol, returning"); return; }; + if thread_ctl.currently_pending_unblocked() == 0 { + // The context is currently Runnable. When transitioning into Blocked, it will check for + // signals (with the context lock held, which is required when sending signals). After + // that, any detection of pending unblocked signals by the sender, will result in the + // context being unblocked, and signals sent. + + // TODO: prioritize signals over regular program execution + return; + } let control_flags = SigcontrolFlags::from_bits_retain(thread_ctl.control_flags.load(Ordering::Acquire)); if control_flags.contains(SigcontrolFlags::INHIBIT_DELIVERY) { @@ -69,11 +54,6 @@ pub fn signal_handler() { return; } - if !core::mem::take(&mut st.is_pending) { - log::trace!("Not pending, returning"); - return; - } - let sigh_instr_ptr = st.user_handler.get(); let Some(regs) = context.regs_mut() else { diff --git a/src/context/switch.rs b/src/context/switch.rs index df45909cfc..cdcc3d5f53 100644 --- a/src/context/switch.rs +++ b/src/context/switch.rs @@ -11,7 +11,7 @@ use crate::{ use super::{ContextId, Status}; enum UpdateResult { - CanSwitch { signal: bool }, + CanSwitch, Skip, } @@ -50,7 +50,7 @@ unsafe fn update_runnable(context: &mut Context, cpu_id: LogicalCpuId) -> Update // Switch to context if it needs to run if context.status.is_runnable() { - UpdateResult::CanSwitch { signal: context.sig.as_mut().map_or(false, |s| s.is_pending) } + UpdateResult::CanSwitch } else { UpdateResult::Skip } @@ -69,12 +69,8 @@ pub fn tick() { // Switch after 3 ticks (about 6.75 ms) if new_ticks >= 3 { - match switch() { - SwitchResult::Switched { signal: true } => { - crate::context::signal::signal_handler(); - }, - _ => (), - } + switch(); + crate::context::signal::signal_handler(); } } @@ -91,7 +87,7 @@ pub unsafe extern "C" fn switch_finish_hook() { #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum SwitchResult { - Switched { signal: bool }, + Switched, AllContextsIdle, } @@ -156,10 +152,9 @@ pub fn switch() -> SwitchResult { let mut next_context_guard = next_context_lock.write_arc(); // Update state of next context and check if runnable - if let UpdateResult::CanSwitch { signal } = unsafe { update_runnable(&mut *next_context_guard, cpu_id) } { + if let UpdateResult::CanSwitch = unsafe { update_runnable(&mut *next_context_guard, cpu_id) } { // Store locks for previous and next context switch_context_opt = Some((prev_context_guard, next_context_guard)); - percpu.switch_internals.switch_signal.set(signal); break; } else { continue; @@ -226,10 +221,7 @@ pub fn switch() -> SwitchResult { // contexts will return directly to the function pointer passed to context::spawn, and not // reach this code until the next context switch back. - let new_percpu = PercpuBlock::current(); - // For the same reason, we obviously can't reuse the percpu block - - SwitchResult::Switched { signal: new_percpu.switch_internals.switch_signal.get() } + SwitchResult::Switched } else { // No target was found, unset global lock and return arch::CONTEXT_SWITCH_LOCK.store(false, Ordering::SeqCst); @@ -248,7 +240,6 @@ pub struct ContextSwitchPercpu { // The ID of the idle process idle_id: Cell, - switch_signal: Cell, } impl ContextSwitchPercpu { pub fn context_id(&self) -> ContextId { diff --git a/src/main.rs b/src/main.rs index b32588a9e7..c101fc05ec 100644 --- a/src/main.rs +++ b/src/main.rs @@ -245,10 +245,7 @@ fn run_userspace() -> ! { unsafe { interrupt::disable(); match context::switch() { - SwitchResult::Switched { signal } => { - if signal { - crate::context::signal::kmain_signal_handler(); - } + SwitchResult::Switched => { interrupt::enable_and_nop(); } SwitchResult::AllContextsIdle => { diff --git a/src/scheme/proc.rs b/src/scheme/proc.rs index 5a36d1513f..8df04189e5 100644 --- a/src/scheme/proc.rs +++ b/src/scheme/proc.rs @@ -1165,7 +1165,6 @@ impl KernelScheme for ProcScheme { .borrow_frame_enforce_rw_allocated(Page::containing_address(VirtualAddress::new(data.thread_control_addr)))?, proc_control: addrsp .borrow_frame_enforce_rw_allocated(Page::containing_address(VirtualAddress::new(data.proc_control_addr)))?, - is_pending: false, }) } else { None diff --git a/src/syscall/process.rs b/src/syscall/process.rs index b6eb0be89f..7f9606bae8 100644 --- a/src/syscall/process.rs +++ b/src/syscall/process.rs @@ -171,7 +171,6 @@ pub fn kill(pid: ContextId, sig: usize) -> Result { ctl.word[0].fetch_and(!(sig_bit(SIGTTIN) | sig_bit(SIGTTOU) | sig_bit(SIGTSTP)), Ordering::Relaxed); ctl.word[0].fetch_or(sig_bit(SIGCONT), Ordering::Relaxed); if (ctl.word[0].load(Ordering::Relaxed) >> 32) & sig_bit(SIGCONT) != 0 { - st.is_pending = true; } } } else if sig == SIGSTOP || (matches!(sig, SIGTTIN | SIGTTOU | SIGTSTP) && context.sigcontrol().map_or(false, |(_, proc, _)| proc.signal_will_stop(sig))) { @@ -185,7 +184,6 @@ pub fn kill(pid: ContextId, sig: usize) -> Result { } else if let Some((ctl, _, st)) = context.sigcontrol() { let _was_new = ctl.word[sig_group].fetch_or(sig_bit(sig), Ordering::Relaxed); if (ctl.word[sig_group].load(Ordering::Relaxed) >> 32) & sig_bit(sig) != 0 { - st.is_pending = true; context.unblock(); } } else { @@ -249,12 +247,9 @@ pub fn kill(pid: ContextId, sig: usize) -> Result { } else if sent == 0 { Err(Error::new(EPERM)) } else { - // Switch to ensure delivery to self - if let SwitchResult::Switched { signal: true } = context::switch() { - context::signal::signal_handler(); - } + // Inform userspace it should check its own mask - Ok(0) + Err(Error::new(EINTR)) } } diff --git a/syscall b/syscall index 9609c7bc26..87b8ac6581 160000 --- a/syscall +++ b/syscall @@ -1 +1 @@ -Subproject commit 9609c7bc269bc24a487c0200e11639add4d43642 +Subproject commit 87b8ac6581b5d3ccf9513485cbe97a51534dce78