From 0eaa6e637d0c202c09dc52716ac6807f1cd7d2aa Mon Sep 17 00:00:00 2001 From: LI Qing Date: Thu, 9 May 2024 16:03:06 +0800 Subject: [PATCH] Optimize the `SigQueues` to return early without lock --- .../src/process/posix_thread/builder.rs | 2 +- .../aster-nix/src/process/posix_thread/mod.rs | 12 +- .../src/process/signal/sig_queues.rs | 119 +++++++++++------- 3 files changed, 82 insertions(+), 51 deletions(-) diff --git a/kernel/aster-nix/src/process/posix_thread/builder.rs b/kernel/aster-nix/src/process/posix_thread/builder.rs index 558e02475..289e50eec 100644 --- a/kernel/aster-nix/src/process/posix_thread/builder.rs +++ b/kernel/aster-nix/src/process/posix_thread/builder.rs @@ -120,7 +120,7 @@ impl PosixThreadBuilder { credentials, real_timer: Mutex::new(real_timer), sig_mask: Mutex::new(sig_mask), - sig_queues: Mutex::new(sig_queues), + sig_queues, sig_context: Mutex::new(None), sig_stack: Mutex::new(None), robust_list: Mutex::new(None), diff --git a/kernel/aster-nix/src/process/posix_thread/mod.rs b/kernel/aster-nix/src/process/posix_thread/mod.rs index 0fbd7eaf6..80e1c081d 100644 --- a/kernel/aster-nix/src/process/posix_thread/mod.rs +++ b/kernel/aster-nix/src/process/posix_thread/mod.rs @@ -59,7 +59,7 @@ pub struct PosixThread { /// Blocked signals sig_mask: Mutex, /// Thread-directed sigqueue - sig_queues: Mutex, + sig_queues: SigQueues, /// Signal handler ucontext address /// FIXME: This field may be removed. For glibc applications with RESTORER flag set, the sig_context is always equals with rsp. sig_context: Mutex>, @@ -88,7 +88,7 @@ impl PosixThread { } pub fn has_pending_signal(&self) -> bool { - !self.sig_queues.lock().is_empty() + !self.sig_queues.is_empty() } /// Returns whether the signal is blocked by the thread. @@ -151,11 +151,11 @@ impl PosixThread { } pub(in crate::process) fn enqueue_signal(&self, signal: Box) { - self.sig_queues.lock().enqueue(signal); + self.sig_queues.enqueue(signal); } pub fn dequeue_signal(&self, mask: &SigMask) -> Option> { - self.sig_queues.lock().dequeue(mask) + self.sig_queues.dequeue(mask) } pub fn register_sigqueue_observer( @@ -163,11 +163,11 @@ impl PosixThread { observer: Weak>, filter: SigEventsFilter, ) { - self.sig_queues.lock().register_observer(observer, filter); + self.sig_queues.register_observer(observer, filter); } pub fn unregiser_sigqueue_observer(&self, observer: &Weak>) { - self.sig_queues.lock().unregister_observer(observer); + self.sig_queues.unregister_observer(observer); } pub fn sig_context(&self) -> &Mutex> { diff --git a/kernel/aster-nix/src/process/signal/sig_queues.rs b/kernel/aster-nix/src/process/signal/sig_queues.rs index c74c679a5..903ae14f4 100644 --- a/kernel/aster-nix/src/process/signal/sig_queues.rs +++ b/kernel/aster-nix/src/process/signal/sig_queues.rs @@ -1,5 +1,7 @@ // SPDX-License-Identifier: MPL-2.0 +use core::sync::atomic::{AtomicUsize, Ordering}; + use super::{ constants::*, sig_mask::SigMask, sig_num::SigNum, signals::Signal, SigEvents, SigEventsFilter, }; @@ -9,31 +11,87 @@ use crate::{ }; pub struct SigQueues { - count: usize, - std_queues: Vec>>, - rt_queues: Vec>>, + // The number of pending signals. + // Useful for quickly determining if any signals are pending without locking `queues`. + count: AtomicUsize, + queues: Mutex, subject: Subject, } impl SigQueues { pub fn new() -> Self { - let count = 0; - let std_queues = (0..COUNT_STD_SIGS).map(|_| None).collect(); - let rt_queues = (0..COUNT_RT_SIGS).map(|_| Default::default()).collect(); - let subject = Subject::new(); - SigQueues { - count, - std_queues, - rt_queues, - subject, + Self { + count: AtomicUsize::new(0), + queues: Mutex::new(Queues::new()), + subject: Subject::new(), } } pub fn is_empty(&self) -> bool { - self.count == 0 + self.count.load(Ordering::Relaxed) == 0 } - pub fn enqueue(&mut self, signal: Box) { + pub fn enqueue(&self, signal: Box) { + let signum = signal.num(); + + let mut queues = self.queues.lock(); + if queues.enqueue(signal) { + self.count.fetch_add(1, Ordering::Relaxed); + // Avoid holding lock when notifying observers + drop(queues); + self.subject.notify_observers(&SigEvents::new(signum)); + } + } + + pub fn dequeue(&self, blocked: &SigMask) -> Option> { + // Fast path for the common case of no pending signals + if self.is_empty() { + return None; + } + + let mut queues = self.queues.lock(); + let signal = queues.dequeue(blocked); + if signal.is_some() { + self.count.fetch_sub(1, Ordering::Relaxed); + } + signal + } + + pub fn register_observer( + &self, + observer: Weak>, + filter: SigEventsFilter, + ) { + self.subject.register_observer(observer, filter); + } + + pub fn unregister_observer(&self, observer: &Weak>) { + self.subject.unregister_observer(observer); + } +} + +impl Default for SigQueues { + fn default() -> Self { + Self::new() + } +} + +struct Queues { + std_queues: Vec>>, + rt_queues: Vec>>, +} + +impl Queues { + fn new() -> Self { + let std_queues = (0..COUNT_STD_SIGS).map(|_| None).collect(); + let rt_queues = (0..COUNT_RT_SIGS).map(|_| Default::default()).collect(); + Self { + std_queues, + rt_queues, + } + } + + fn enqueue(&mut self, signal: Box) -> bool { let signum = signal.num(); if signum.is_std() { // Standard signals @@ -52,26 +110,19 @@ impl SigQueues { let queue = self.get_std_queue_mut(signum); if queue.is_some() { // If there is already a signal pending, just ignore all subsequent signals - return; + return false; } *queue = Some(signal); - self.count += 1; } else { // Real-time signals let queue = self.get_rt_queue_mut(signum); queue.push_back(signal); - self.count += 1; } - self.subject.notify_observers(&SigEvents::new(signum)); + true } - pub fn dequeue(&mut self, blocked: &SigMask) -> Option> { - // Fast path for the common case of no pending signals - if self.is_empty() { - return None; - } - + fn dequeue(&mut self, blocked: &SigMask) -> Option> { // Deliver standard signals. // // According to signal(7): @@ -100,7 +151,6 @@ impl SigQueues { let queue = self.get_std_queue_mut(signum); let signal = queue.take(); if signal.is_some() { - self.count -= 1; return signal; } } @@ -122,7 +172,6 @@ impl SigQueues { let queue = self.get_rt_queue_mut(signum); let signal = queue.pop_front(); if signal.is_some() { - self.count -= 1; return signal; } } @@ -142,22 +191,4 @@ impl SigQueues { let idx = (signum.as_u8() - MIN_RT_SIG_NUM) as usize; &mut self.rt_queues[idx] } - - pub fn register_observer( - &self, - observer: Weak>, - filter: SigEventsFilter, - ) { - self.subject.register_observer(observer, filter); - } - - pub fn unregister_observer(&self, observer: &Weak>) { - self.subject.unregister_observer(observer); - } -} - -impl Default for SigQueues { - fn default() -> Self { - Self::new() - } }