Fix potential deadlocks

This commit is contained in:
Ruihan Li 2025-11-25 17:04:24 +08:00 committed by Tate, Hongliang Tian
parent 42026b3eb9
commit efeaf5fa6e
2 changed files with 70 additions and 63 deletions

View File

@ -76,46 +76,36 @@ define_atomic_version_of_integer_like_type!(EvdevClock, try_from = true, {
struct AtomicEvdevClock(AtomicU8);
});
/// An opened file from an evdev device (`EvdevDevice`).
pub struct EvdevFile {
/// An opened file from an evdev device ([`EvdevDevice`]).
pub(super) struct EvdevFile {
/// Consumer for reading events.
consumer: Mutex<RbConsumer<EvdevEvent>>,
/// Inner data (shared with the device).
inner: Arc<EvdevFileInner>,
/// Weak reference to the evdev device that owns this evdev file.
evdev: Weak<EvdevDevice>,
}
/// An opened evdev file's inner data (shared with its [`EvdevDevice`]).
pub(super) struct EvdevFileInner {
/// Clock ID for this opened evdev file.
clock_id: AtomicEvdevClock,
/// Number of complete event packets available (ended with SYN_REPORT).
/// Number of complete event packets available (ended with `SYN_REPORT`).
packet_count: AtomicUsize,
/// Pollee for event notification.
pollee: Pollee,
/// Weak reference to the evdev device that owns this evdev file.
evdev: Weak<EvdevDevice>,
}
impl Debug for EvdevFile {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
f.debug_struct("EvdevFile")
.field("clock_id", &self.clock_id)
.field("packet_count", &self.packet_count)
.field("clock_id", &self.inner.clock_id)
.field("packet_count", &self.inner.packet_count)
.finish_non_exhaustive()
}
}
impl EvdevFile {
pub(super) fn new(
buffer_size: usize,
evdev: Weak<EvdevDevice>,
) -> (Self, RbProducer<EvdevEvent>) {
let (producer, consumer) = RingBuffer::new(buffer_size).split();
let evdev_file = Self {
consumer: Mutex::new(consumer),
clock_id: AtomicEvdevClock::new(EvdevClock::Monotonic),
packet_count: AtomicUsize::new(0),
pollee: Pollee::new(),
evdev,
};
(evdev_file, producer)
}
impl EvdevFileInner {
pub(super) fn read_clock(&self) -> Duration {
use crate::time::clocks::{BootTimeClock, MonotonicClock, RealTimeClock};
@ -128,22 +118,47 @@ impl EvdevFile {
}
/// Checks if buffer has complete event packets.
pub fn has_complete_packets(&self) -> bool {
pub(self) fn has_complete_packets(&self) -> bool {
self.packet_count.load(Ordering::Relaxed) > 0
}
/// Increments the packet count.
pub fn increment_packet_count(&self) {
pub(super) fn increment_packet_count(&self) {
self.packet_count.fetch_add(1, Ordering::Relaxed);
self.pollee.notify(IoEvents::IN);
}
/// Decrements the packet count.
pub fn decrement_packet_count(&self) {
pub(self) fn decrement_packet_count(&self) {
if self.packet_count.fetch_sub(1, Ordering::Relaxed) == 1 {
self.pollee.invalidate();
}
}
}
impl EvdevFile {
pub(super) fn new(
buffer_size: usize,
evdev: Weak<EvdevDevice>,
) -> (Self, RbProducer<EvdevEvent>) {
let (producer, consumer) = RingBuffer::new(buffer_size).split();
let inner = EvdevFileInner {
clock_id: AtomicEvdevClock::new(EvdevClock::Monotonic),
packet_count: AtomicUsize::new(0),
pollee: Pollee::new(),
};
let evdev_file = Self {
consumer: Mutex::new(consumer),
inner: Arc::new(inner),
evdev,
};
(evdev_file, producer)
}
pub(super) fn inner(&self) -> &Arc<EvdevFileInner> {
&self.inner
}
/// Processes events and writes them to the writer.
///
@ -161,7 +176,7 @@ impl EvdevFile {
// Update the counter since the event has been consumed.
if is_syn_report_event(&event) || is_syn_dropped_event(&event) {
self.decrement_packet_count();
self.inner.decrement_packet_count();
}
// Write the event to the writer.
@ -179,7 +194,7 @@ impl EvdevFile {
fn check_io_events(&self) -> IoEvents {
// TODO: Report `IoEvents::HUP` if the device has been disconnected.
if self.has_complete_packets() {
if self.inner.has_complete_packets() {
IoEvents::IN
} else {
IoEvents::empty()
@ -199,7 +214,8 @@ fn is_syn_dropped_event(event: &EvdevEvent) -> bool {
impl Pollable for EvdevFile {
fn poll(&self, mask: IoEvents, poller: Option<&mut PollHandle>) -> IoEvents {
self.pollee
self.inner
.pollee
.poll_with(mask, poller, || self.check_io_events())
}
}
@ -224,7 +240,7 @@ impl InodeIo for EvdevFile {
// If we're in non-blocking mode, we won't bother the user space with an incomplete packet.
// Note that this aligns to the behavior of `check_io_events`.
if is_nonblocking && !self.has_complete_packets() {
if is_nonblocking && !self.inner.has_complete_packets() {
return_errno_with_message!(Errno::EAGAIN, "the evdev file has no packets");
}
@ -279,7 +295,7 @@ impl FileIo for EvdevFile {
impl Drop for EvdevFile {
fn drop(&mut self) {
if let Some(evdev) = self.evdev.upgrade() {
evdev.detach_closed_files();
evdev.detach_closed_file(&self.inner);
}
}
}

View File

@ -9,12 +9,7 @@
mod file;
use alloc::{
format,
string::String,
sync::{Arc, Weak},
vec::Vec,
};
use alloc::{format, string::String, sync::Arc, vec::Vec};
use core::{
fmt::Debug,
sync::atomic::{AtomicU32, Ordering},
@ -26,7 +21,7 @@ use aster_input::{
input_handler::{ConnectError, InputHandler, InputHandlerClass},
};
use device_id::{DeviceId, MajorId, MinorId};
use file::{EvdevEvent, EvdevFile, EVDEV_BUFFER_SIZE};
use file::{EvdevEvent, EvdevFile, EvdevFileInner, EVDEV_BUFFER_SIZE};
use ostd::sync::SpinLock;
use spin::Once;
@ -55,7 +50,7 @@ pub struct EvdevDevice {
/// This lock is acquired in both the task and interrupt contexts.
/// We must make sure that this lock is taken with the local IRQs disabled.
/// Otherwise, we would be vulnerable to deadlock.
opened_files: SpinLock<Vec<(Weak<EvdevFile>, RbProducer<EvdevEvent>)>>,
opened_files: SpinLock<Vec<(Arc<EvdevFileInner>, RbProducer<EvdevEvent>)>>,
/// Device node name (e.g., "event0").
node_name: String,
/// Device ID.
@ -64,17 +59,14 @@ pub struct EvdevDevice {
impl Debug for EvdevDevice {
fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
let opened_count = self
.opened_files
.lock()
.iter()
.filter(|(file, _)| file.strong_count() > 0)
.count();
let device_name = self.device.name();
let opened_count = self.opened_files.disable_irq().lock().len();
let id_minor = self.id.minor();
f.debug_struct("EvdevDevice")
.field("minor", &self.id.minor())
.field("device_name", &self.device.name())
.field("opened_files", &opened_count)
.finish()
.field("device_name", &device_name)
.field("opened_count", &opened_count)
.field("id_minor", &id_minor)
.finish_non_exhaustive()
}
}
@ -98,15 +90,19 @@ impl EvdevDevice {
}
/// Adds an opened evdev file to this evdev device.
fn attach_file(&self, file: Weak<EvdevFile>, producer: RbProducer<EvdevEvent>) {
fn attach_file(&self, file: Arc<EvdevFileInner>, producer: RbProducer<EvdevEvent>) {
let mut opened_files = self.opened_files.disable_irq().lock();
opened_files.push((file, producer));
}
/// Removes closed evdev files from this evdev device.
pub(self) fn detach_closed_files(&self) {
/// Removes the closed evdev file from this evdev device.
pub(self) fn detach_closed_file(&self, file: &Arc<EvdevFileInner>) {
let mut opened_files = self.opened_files.disable_irq().lock();
opened_files.retain(|(file, _)| file.strong_count() > 0);
let pos = opened_files
.iter()
.position(|(f, _)| Arc::ptr_eq(f, file))
.unwrap();
opened_files.swap_remove(pos);
}
/// Distributes events to all opened evdev files.
@ -114,11 +110,7 @@ impl EvdevDevice {
let mut opened_files = self.opened_files.lock();
// Send events to all opened evdev files using their producers.
for (file_weak, producer) in opened_files.iter_mut() {
let Some(file) = file_weak.upgrade() else {
continue;
};
for (file, producer) in opened_files.iter_mut() {
for event in events {
// Read the current time according to the opened evdev file's clock type.
let time = file.read_clock();
@ -174,13 +166,12 @@ impl EvdevDevice {
pub(self) fn create_file(self: &Arc<Self>, buffer_size: usize) -> Result<Arc<EvdevFile>> {
// Create the evdev file and get the producer.
let (file, producer) = EvdevFile::new(buffer_size, Arc::downgrade(self));
let file = Arc::new(file);
let file_weak = Arc::downgrade(&file);
// Attach the opened evdev file to this device.
self.attach_file(file_weak, producer);
self.attach_file(file.inner().clone(), producer);
Ok(file)
// Note that this can and should be a `Box` after fixing the char device subsystem.
Ok(Arc::new(file))
}
}