fix(starry-perf): separate control and IRQ output locks

os/StarryOS/kernel/src/axtest_exports.rs

@@ -303,6 +303,10 @@ pub fn dummy_stat_fs_fields_match_expected_defaults() -> bool {
     super::pseudofs::dummy_stat_fs_fields_match_expected_defaults_for_test()
 }
 
+pub fn perf_control_callback_runs_preemptible() -> bool {
+    super::perf::control_callback_runs_preemptible_for_test()
+}
+
 pub fn is_wext_ioctl_validation_rules_hold() -> bool {
     super::file::is_wext_ioctl_validation_rules_hold_for_test()
 }

os/StarryOS/kernel/src/perf/bpf.rs

@@ -19,6 +19,7 @@ use core::{
 use ax_alloc::GlobalPage;
 use ax_errno::{AxError, AxResult};
 use ax_hal::mem::virt_to_phys;
+use ax_kspin::SpinNoIrq;
 use ax_memory_addr::{PAGE_SIZE_4K, PhysAddr};
 use ax_task::IrqNotify;
 use axpoll::{IoEvents, PollSet, Pollable};
@@ -43,14 +44,56 @@ use crate::{
 #[cfg(target_arch = "x86_64")]
 const BPF_JIT_MEM_PAGES: usize = 4;
 
-/// Wraps `kbpf_basic::perf::bpf::BpfPerfEvent` with kernel state: a poll
-/// set so readers can wait for new records, and a weak handle to the
-/// backing pages produced by `device_mmap` (Some after the first
-/// `mmap(perf_fd)`; None before).
+struct BpfPerfEventState {
+    inner: BpfPerfEvent,
+    /// Weak handle to the contiguous pages backing the ringbuf. The strong
+    /// ref(s) live in the user VMA(s); `strong_count() > 0` means a live
+    /// mapping still exists.
+    pages: Option<Weak<GlobalPage>>,
+}
+
+impl BpfPerfEventState {
+    fn is_mapped(&self) -> bool {
+        self.pages
+            .as_ref()
+            .is_some_and(|pages| pages.strong_count() > 0)
+    }
+}
+
+/// Non-sleeping output capability used by `bpf_perf_event_output`.
+///
+/// The task control plane owns allocation and mapping. This endpoint only
+/// enters the bounded ring write, observes the already-published page anchor,
+/// and emits an IRQ-safe worker notification.
+#[derive(Clone)]
+pub(super) struct BpfPerfOutput {
+    state: Arc<SpinNoIrq<BpfPerfEventState>>,
+    poll_notify: Arc<IrqNotify>,
+}
+
+impl BpfPerfOutput {
+    pub(super) fn write_event(&self, data: &[u8]) -> AxResult<()> {
+        let notify = {
+            let mut state = self.state.lock();
+            if !state.is_mapped() {
+                return Ok(());
+            }
+            state.inner.write_event(data).into_ax_result()?;
+            state.inner.enabled()
+        };
+        if notify {
+            self.poll_notify.notify_irq();
+        }
+        Ok(())
+    }
+}
+
+/// Wraps `kbpf_basic::perf::bpf::BpfPerfEvent` with separate task-control and
+/// non-sleeping output state plus a poll set so readers can wait for records.
 ///
 /// Ownership model: the user VMA owns the ringbuf pages via the strong
 /// `Arc<GlobalPage>` threaded into `DeviceMmap::Physical`'s retainer slot;
-/// this wrapper keeps only a `Weak`. Consequences:
+/// the shared output state keeps only a `Weak`. Consequences:
 ///
 /// * UAF safety — the pages outlive `close(perf_fd)` (which drops this
 ///   wrapper) for as long as a mapping is live, because the VMA holds the
@@ -64,20 +107,15 @@ const BPF_JIT_MEM_PAGES: usize = 4;
 ///   normal `munmap` the same thing happens, matching Linux's allowance to
 ///   re-`mmap` a perf fd.
 ///
-/// `inner` holds a raw pointer into the page buffer; `RingPage` has no
+/// The inner perf event holds a raw pointer into the page buffer; `RingPage` has no
 /// destructor and is never dereferenced once the pages are gone (every
-/// access through `inner` is gated on [`Self::is_mapped`]), so a dangling
-/// pointer left after the pages free is harmless.
+/// access is gated on [`BpfPerfEventState::is_mapped`]), so a dangling pointer
+/// left after the pages free is harmless.
 pub struct BpfPerfEventWrapper {
-    inner: BpfPerfEvent,
+    state: Arc<SpinNoIrq<BpfPerfEventState>>,
     poll_ready: Arc<PollSet>,
     poll_notify: Arc<IrqNotify>,
     poll_alive: Arc<AtomicBool>,
-    /// Weak handle to the contiguous pages backing the ringbuf. The strong
-    /// ref(s) live in the user VMA(s); `strong_count() > 0` means a live
-    /// mapping still exists. See the type-level docs for the ownership
-    /// rationale.
-    pages: Option<Weak<GlobalPage>>,
 }
 
 impl BpfPerfEventWrapper {
@@ -88,36 +126,18 @@ impl BpfPerfEventWrapper {
         let poll_alive = Arc::new(AtomicBool::new(true));
         start_bpf_perf_notify_worker(poll_ready.clone(), poll_notify.clone(), poll_alive.clone());
         Self {
-            inner,
+            state: Arc::new(SpinNoIrq::new(BpfPerfEventState { inner, pages: None })),
             poll_ready,
             poll_notify,
             poll_alive,
-            pages: None,
         }
     }
 
-    /// Whether a live user mapping of the ringbuf currently exists. The
-    /// wrapper only holds a `Weak` to the backing pages, so this is true
-    /// exactly while some VMA still pins them; once every mapping is gone
-    /// (munmap / exit) — or an in-progress mmap was abandoned before a VMA
-    /// adopted the pages — the strong refs drop and this returns false.
-    fn is_mapped(&self) -> bool {
-        self.pages.as_ref().is_some_and(|w| w.strong_count() > 0)
-    }
-
-    /// Write a record into the ringbuf and wake any readers. Calls before a
-    /// mapping exists (or after it is gone) are accepted as no-ops: the
-    /// `kbpf_basic::RingPage` pointer is either still `empty()` or now
-    /// dangling, so dereferencing it would be UB.
-    pub fn write_event(&mut self, data: &[u8]) -> AxResult<()> {
-        if !self.is_mapped() {
-            return Ok(());
-        }
-        self.inner.write_event(data).into_ax_result()?;
-        if self.inner.enabled() {
-            self.poll_notify.notify_irq();
+    pub(super) fn output_handle(&self) -> BpfPerfOutput {
+        BpfPerfOutput {
+            state: Arc::clone(&self.state),
+            poll_notify: Arc::clone(&self.poll_notify),
         }
-        Ok(())
     }
 }
 
@@ -154,12 +174,12 @@ impl Debug for BpfPerfEventWrapper {
 
 impl PerfEventOps for BpfPerfEventWrapper {
     fn enable(&mut self) -> AxResult<()> {
-        self.inner.enable().into_ax_result()?;
+        self.state.lock().inner.enable().into_ax_result()?;
         Ok(())
     }
 
     fn disable(&mut self) -> AxResult<()> {
-        self.inner.disable().into_ax_result()?;
+        self.state.lock().inner.disable().into_ax_result()?;
         Ok(())
     }
 
@@ -168,7 +188,7 @@ impl PerfEventOps for BpfPerfEventWrapper {
     }
 
     fn device_mmap(&mut self, len: usize) -> AxResult<(PhysAddr, Arc<dyn Any + Send + Sync>)> {
-        if self.is_mapped() {
+        if self.state.lock().is_mapped() {
             // Linux allows only one live mmap per perf event fd; a second
             // mapping while the first is alive would orphan it. A stale
             // `Weak` from an abandoned or munmap'd previous attempt does not
@@ -189,7 +209,14 @@ impl PerfEventOps for BpfPerfEventWrapper {
         pages.zero();
         let kvirt = pages.start_vaddr();
         let paddr = virt_to_phys(kvirt);
-        self.inner
+        let pages = Arc::new(pages);
+
+        let mut state = self.state.lock();
+        if state.is_mapped() {
+            return Err(AxError::ResourceBusy);
+        }
+        state
+            .inner
             .do_mmap(kvirt.as_usize(), len, 0)
             .map_err(|_| AxError::InvalidInput)?;
         // kbpf_basic::RingPage::init sets the data-region geometry but leaves
@@ -201,7 +228,6 @@ impl PerfEventOps for BpfPerfEventWrapper {
             core::ptr::addr_of_mut!((*header).version).write(1);
             core::ptr::addr_of_mut!((*header).compat_version).write(0);
         }
-        let pages = Arc::new(pages);
         // Keep only a `Weak`; hand the sole strong ref to the caller, which
         // threads it into `DeviceMmap::Physical`'s retainer so the user VMA
         // pins these frames until `munmap`/exit even if the perf fd (and this
@@ -210,15 +236,16 @@ impl PerfEventOps for BpfPerfEventWrapper {
         // the anchor simply frees the pages and leaves the fd mmap-able again
         // (see the type-level docs). Without the anchor the pages would free
         // under a live mapping.
-        self.pages = Some(Arc::downgrade(&pages));
+        state.pages = Some(Arc::downgrade(&pages));
+        drop(state);
         let anchor: Arc<dyn Any + Send + Sync> = pages;
         Ok((paddr, anchor))
     }
 }
 
 impl Pollable for BpfPerfEventWrapper {
     fn poll(&self) -> axpoll::IoEvents {
-        if self.inner.readable() {
+        if self.state.lock().inner.readable() {
             IoEvents::IN
         } else {
             IoEvents::empty()

os/StarryOS/kernel/src/perf/mod.rs

@@ -37,10 +37,11 @@ use core::{
 
 use ax_errno::{AxError, AxResult};
 use ax_io::{Read, Write};
-use ax_kspin::{SpinNoPreempt, SpinNoPreemptGuard};
+use ax_kspin::SpinNoIrq;
 use ax_lazyinit::LazyInit;
 use ax_memory_addr::{PAGE_SIZE_4K, PhysAddr, PhysAddrRange, VirtAddr, VirtAddrRange};
 use ax_runtime::hal::{paging::MappingFlags, pmu};
+use ax_sync::Mutex;
 use axpoll::Pollable;
 pub use bpf::BpfPerfEventWrapper;
 use hashbrown::HashMap;
@@ -93,8 +94,8 @@ pub trait PerfEventOps: Pollable + Send + Sync + Debug {
     /// Stop firing without tearing down the event.
     fn disable(&mut self) -> AxResult<()>;
 
-    /// `Any` upcast (mutable). Used by `perf_event_output` to recover the
-    /// concrete `BpfPerfEventWrapper` from a `dyn PerfEventOps`.
+    /// `Any` upcast (mutable). Used while constructing [`PerfEvent`] to recover
+    /// capabilities exposed by concrete implementations.
     fn as_any_mut(&mut self) -> &mut dyn Any;
 
     /// Attach a BPF program to this event (`PERF_EVENT_IOC_SET_BPF`).
@@ -196,10 +197,16 @@ pub struct PerfReadValues {
     pub read_format: u64,
 }
 
-/// File-like handle returned by `perf_event_open(2)`. Locks a
-/// `Box<dyn PerfEventOps>` so the inner implementation can stay generic.
+/// File-like handle returned by `perf_event_open(2)`.
+///
+/// Task-context control operations use a blocking mutex because callbacks may
+/// allocate, fault, or reschedule. Software BPF output has a separate
+/// non-sleeping capability containing only the bounded ring-write state needed
+/// by trace and IRQ producers.
 pub struct PerfEvent {
-    event: SpinNoPreempt<Box<dyn PerfEventOps>>,
+    event: Mutex<Box<dyn PerfEventOps>>,
+    /// Bounded non-sleeping output endpoint for software BPF events.
+    irq_output: Option<bpf::BpfPerfOutput>,
     /// Unique, stable perf-event id (see [`NEXT_PERF_EVENT_ID`]). Returned by
     /// `PERF_EVENT_IOC_ID` and used as the `read_format` `PERF_FORMAT_ID` value.
     id: u64,
@@ -221,18 +228,18 @@ impl PerfEvent {
     pub fn new(mut event: Box<dyn PerfEventOps>) -> Self {
         let id = NEXT_PERF_EVENT_ID.fetch_add(1, Ordering::Relaxed);
         event.set_sample_id(id);
+        let irq_output = event
+            .as_any_mut()
+            .downcast_mut::<BpfPerfEventWrapper>()
+            .map(|event| event.output_handle());
         PerfEvent {
-            event: SpinNoPreempt::new(event),
+            event: Mutex::new(event),
+            irq_output,
             id,
             nonblocking: AtomicBool::new(false),
         }
     }
 
-    /// Borrow the inner impl under the lock.
-    pub fn event(&self) -> SpinNoPreemptGuard<'_, Box<dyn PerfEventOps>> {
-        self.event.lock()
-    }
-
     /// Handle `PERF_EVENT_IOC_SET_OUTPUT`: redirect this event's records into the
     /// ring owned by the perf event whose fd is `arg` (or detach when `arg == -1`).
     ///
@@ -258,7 +265,8 @@ impl PerfEvent {
         // event's output at it. If the target has no ring (e.g. it is itself a
         // non-mmap'd or non-sampling event), there is nothing to merge into; the
         // source keeps its own ring — `redirect_output` is then never called.
-        if let Some((ring_vaddr, ring_len, anchor)) = target.event.lock().output_ring() {
+        let target_output = target.event.lock().output_ring();
+        if let Some((ring_vaddr, ring_len, anchor)) = target_output {
             self.event
                 .lock()
                 .redirect_output(ring_vaddr, ring_len, anchor)?;
@@ -491,12 +499,12 @@ pub fn perf_event_open(
 /// Map fd → weak<PerfEvent> so `bpf_perf_event_output` can locate the
 /// target ringbuf without owning a strong reference (the user side owns
 /// it via the fd).
-static PERF_FILE: LazyInit<SpinNoPreempt<HashMap<usize, alloc::sync::Weak<dyn FileLike>>>> =
+static PERF_FILE: LazyInit<SpinNoIrq<HashMap<usize, alloc::sync::Weak<dyn FileLike>>>> =
     LazyInit::new();
 
 /// Initialize the perf-event runtime: build the fd→event lookup table.
 pub fn perf_event_init() {
-    PERF_FILE.init_once(SpinNoPreempt::new(HashMap::new()));
+    PERF_FILE.init_once(SpinNoIrq::new(HashMap::new()));
 }
 
 /// Implementation of `bpf_perf_event_output` helper: walk the fd→event map,
@@ -516,13 +524,43 @@ pub fn perf_event_output(_ctx: *mut c_void, fd: usize, _flags: u32, data: &[u8])
         .into_any_arc()
         .downcast::<PerfEvent>()
         .map_err(|_| AxError::InvalidInput)?;
-    let mut inner = perf_event.event();
-    let bpf_event = inner
-        .as_any_mut()
-        .downcast_mut::<BpfPerfEventWrapper>()
-        .ok_or(AxError::InvalidInput)?;
-    bpf_event.write_event(data)?;
-    Ok(())
+    perf_event
+        .irq_output
+        .as_ref()
+        .ok_or(AxError::InvalidInput)?
+        .write_event(data)
+}
+
+#[cfg(axtest)]
+pub(crate) fn control_callback_runs_preemptible_for_test() -> bool {
+    #[derive(Debug)]
+    struct YieldingControl;
+
+    impl Pollable for YieldingControl {
+        fn poll(&self) -> axpoll::IoEvents {
+            axpoll::IoEvents::empty()
+        }
+
+        fn register(&self, _context: &mut core::task::Context<'_>, _events: axpoll::IoEvents) {}
+    }
+
+    impl PerfEventOps for YieldingControl {
+        fn enable(&mut self) -> AxResult<()> {
+            ax_task::yield_now();
+            Ok(())
+        }
+
+        fn disable(&mut self) -> AxResult<()> {
+            Ok(())
+        }
+
+        fn as_any_mut(&mut self) -> &mut dyn Any {
+            self
+        }
+    }
+
+    let event = PerfEvent::new(Box::new(YieldingControl));
+    event.ioctl(PerfEventIoc::Enable as u32, 0).is_ok()
 }
 
 /// Executable kernel mapping used by rbpf JIT programs on x86_64.

os/StarryOS/kernel/tests/cases/axtest_runtime.rs

@@ -35,3 +35,8 @@ fn time_value_conversion_rules_hold() {
 fn dummy_stat_fs_fields_match_expected_defaults() {
     ax_assert!(axtest_exports::dummy_stat_fs_fields_match_expected_defaults());
 }
+
+#[axtest]
+fn perf_control_callback_runs_preemptible() {
+    ax_assert!(axtest_exports::perf_control_callback_runs_preemptible());
+}