From 879706e6e9b6d85db6e530ce52f12c4015f04dae Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sat, 29 Jul 2023 16:54:31 +0300 Subject: [PATCH] Kernel: Signals are not queued anymore Posix doesn't require signal queing if you don't use sigqueue() which we don't support. Process also has its own pending signal mask. --- kernel/include/kernel/Process.h | 1 + kernel/include/kernel/Thread.h | 11 +++--- kernel/kernel/Process.cpp | 23 ++++++++----- kernel/kernel/Scheduler.cpp | 2 +- kernel/kernel/Thread.cpp | 59 ++++++++++++++++++++++----------- 5 files changed, 61 insertions(+), 35 deletions(-) diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 16dd264eb0..d401f040b4 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -160,6 +160,7 @@ namespace Kernel BAN::UniqPtr m_general_allocator; vaddr_t m_signal_handlers[_SIGMAX + 1] { }; + uint64_t m_signal_pending_mask { 0 }; bool m_is_userspace { false }; userspace_info_t m_userspace_info; diff --git a/kernel/include/kernel/Thread.h b/kernel/include/kernel/Thread.h index d0f7a50098..8e23b2d09f 100644 --- a/kernel/include/kernel/Thread.h +++ b/kernel/include/kernel/Thread.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include #include @@ -50,8 +49,8 @@ namespace Kernel bool has_signal_to_execute() const; void set_signal_done(int signal); - void handle_next_signal(); - void queue_signal(int signal); + void handle_signal(int signal = 0); + bool add_signal(int signal); void set_return_rsp(uintptr_t& rsp) { m_return_rsp = &rsp; } void set_return_rip(uintptr_t& rip) { m_return_rip = &rip; } @@ -109,9 +108,9 @@ namespace Kernel uintptr_t* m_return_rsp { nullptr }; uintptr_t* m_return_rip { nullptr }; - BAN::CircularQueue m_signal_queue; - uint64_t m_signal_mask { 0 }; - int m_handling_signal { 0 }; + uint64_t m_signal_pending_mask { 0 }; + uint64_t m_signal_block_mask { 0 }; + int m_handling_signal { 0 }; static_assert(_SIGMAX < 64); uint64_t m_terminate_blockers { 0 }; diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 49efe518dd..a17f037d19 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -845,14 +845,14 @@ namespace Kernel return Process::current().sys_raise(signal); LockGuard process_guard(s_process_lock); - CriticalScope _; - for (auto* process : s_processes) { if (process->pid() == pid) { - if (signal) - process->m_threads.front()->queue_signal(signal); + if (signal == 0) + return 0; + CriticalScope _; + process->m_signal_pending_mask |= 1ull << signal; return 0; } } @@ -864,12 +864,19 @@ namespace Kernel { if (signal < _SIGMIN || signal > _SIGMAX) return BAN::Error::from_errno(EINVAL); - ASSERT(m_threads.size() == 1); + ASSERT(this == &Process::current()); + CriticalScope _; + + // FIXME: support raise with signal blocked Thread& current = Thread::current(); - current.queue_signal(signal); - current.handle_next_signal(); - return 0; + if (current.add_signal(signal)) + { + current.handle_signal(signal); + return 0; + } + + ASSERT_NOT_REACHED(); } BAN::ErrorOr Process::sys_tcsetpgrp(int fd, pid_t pgid) diff --git a/kernel/kernel/Scheduler.cpp b/kernel/kernel/Scheduler.cpp index 291b07f826..58c3aac508 100644 --- a/kernel/kernel/Scheduler.cpp +++ b/kernel/kernel/Scheduler.cpp @@ -248,7 +248,7 @@ namespace Kernel start_thread(current->rsp(), current->rip()); case Thread::State::Executing: while (current->has_signal_to_execute() && current->state() == Thread::State::Executing) - current->handle_next_signal(); + current->handle_signal(); // fall through case Thread::State::Terminating: continue_thread(current->rsp(), current->rip()); diff --git a/kernel/kernel/Thread.cpp b/kernel/kernel/Thread.cpp index 440e9ecba5..5a2307b46f 100644 --- a/kernel/kernel/Thread.cpp +++ b/kernel/kernel/Thread.cpp @@ -183,7 +183,7 @@ namespace Kernel // Setup stack for returning { // FIXME: don't use PageTableScope - PageTableScope _(m_process->page_table()); + PageTableScope _(process().page_table()); write_to_stack(m_rsp, this); write_to_stack(m_rsp, &Thread::on_exit); write_to_stack(m_rsp, nullptr); @@ -204,12 +204,13 @@ namespace Kernel m_rsp = stack_base() + stack_size(); m_rip = (uintptr_t)entry; - m_signal_mask = ~0ull; + m_signal_pending_mask = 0; + m_signal_block_mask = ~0ull; // Setup stack for returning { // FIXME: don't use PageTableScope - PageTableScope _(m_process->page_table()); + PageTableScope _(process().page_table()); write_to_stack(m_rsp, this); write_to_stack(m_rsp, &Thread::on_exit); write_to_stack(m_rsp, m_process); @@ -218,7 +219,10 @@ namespace Kernel bool Thread::has_signal_to_execute() const { - return !m_signal_queue.empty() && !m_handling_signal; + if (!m_process || m_handling_signal) + return false; + uint64_t full_pending_mask = m_signal_pending_mask | m_process->m_signal_pending_mask; + return full_pending_mask & ~m_signal_block_mask; } void Thread::set_signal_done(int signal) @@ -232,26 +236,40 @@ namespace Kernel m_handling_signal = 0; } - void Thread::handle_next_signal() + void Thread::handle_signal(int signal) { ASSERT(!interrupts_enabled()); - ASSERT(!m_signal_queue.empty()); ASSERT(&Thread::current() == this); ASSERT(is_userspace()); - int signal = m_signal_queue.front(); - ASSERT(signal >= _SIGMIN && signal <= _SIGMAX); - m_signal_queue.pop(); + if (signal == 0) + { + uint64_t full_pending_mask = m_signal_pending_mask | process().m_signal_pending_mask; + for (signal = _SIGMIN; signal <= _SIGMAX; signal++) + { + uint64_t mask = 1ull << signal; + if ((full_pending_mask & mask) && !(m_signal_block_mask & mask)) + break; + } + ASSERT(signal <= _SIGMAX); + } + else + { + uint64_t full_pending_mask = m_signal_pending_mask | process().m_signal_pending_mask; + uint64_t mask = 1ull << signal; + ASSERT(full_pending_mask & mask); + ASSERT(!(m_signal_block_mask & mask)); + } uintptr_t& return_rsp = this->return_rsp(); uintptr_t& return_rip = this->return_rip(); vaddr_t signal_handler = process().m_signal_handlers[signal]; - // Skip masked and ignored signals - if (m_signal_mask & (1ull << signal)) - ; - else if (signal_handler == (vaddr_t)SIG_IGN) + m_signal_pending_mask &= ~(1ull << signal); + process().m_signal_pending_mask &= ~(1ull << signal); + + if (signal_handler == (vaddr_t)SIG_IGN) ; else if (signal_handler != (vaddr_t)SIG_DFL) { @@ -321,17 +339,18 @@ namespace Kernel } } - void Thread::queue_signal(int signal) + bool Thread::add_signal(int signal) { ASSERT(!interrupts_enabled()); - if (m_signal_queue.full()) + uint64_t mask = 1ull << signal; + if (!(m_signal_block_mask & mask)) { - dwarnln("Signal queue full"); - return; + m_signal_pending_mask |= mask; + if (this != &Thread::current()) + Scheduler::get().unblock_thread(tid()); + return true; } - m_signal_queue.push(signal); - if (this != &Thread::current()) - Scheduler::get().unblock_thread(tid()); + return false; } void Thread::validate_stack() const