From 0a248df055004fa18d5ab50507f9c80adda83a34 Mon Sep 17 00:00:00 2001 From: Daniel Levin Date: Thu, 21 Aug 2025 11:38:22 +0200 Subject: [PATCH 1/5] Rework and simplify internals This change simplifies Builder's internals and makes it harder to misuse. It also makes tracepoint support explicit, and an example of working tracepoints is supplied. Finally, I took a dependency on this error to generate some previously hand-written code. --- perf-event/Cargo.toml | 1 + perf-event/examples/tracepoint.rs | 21 ++ perf-event/src/builder.rs | 359 +++++++++++++++++++++++----- perf-event/src/events/hardware.rs | 3 + perf-event/src/events/mod.rs | 4 +- perf-event/src/events/tracepoint.rs | 43 ++-- perf-event/src/group.rs | 2 +- perf-event/src/lib.rs | 11 +- 8 files changed, 350 insertions(+), 94 deletions(-) create mode 100644 perf-event/examples/tracepoint.rs diff --git a/perf-event/Cargo.toml b/perf-event/Cargo.toml index 662d90d..5d67901 100644 --- a/perf-event/Cargo.toml +++ b/perf-event/Cargo.toml @@ -29,6 +29,7 @@ libc = "0.2" memmap2 = "0.9" perf-event-data = "0.1.8" perf-event-open-sys2 = "5.0.4" +thiserror = "2.0.14" [dev-dependencies] ctrlc = "3.4.5" diff --git a/perf-event/examples/tracepoint.rs b/perf-event/examples/tracepoint.rs new file mode 100644 index 0000000..218a1e1 --- /dev/null +++ b/perf-event/examples/tracepoint.rs @@ -0,0 +1,21 @@ +use perf_event::data::Record; +use perf_event::events::Tracepoint; +use perf_event::{Builder, CpuPid}; + +fn main() -> Result<(), Box> { + let mut counter = Builder::new(Tracepoint::with_name("net/net_dev_xmit")?) + .targeting(CpuPid::AnyProcessOneCpu { cpu: 0 }) + .build()? + .sampled(8192)?; + + counter.enable()?; + + while let Some(record) = counter.next_blocking(None) { + println!("received event"); + if let Ok(Record::Sample(sample)) = record.parse_record() { + dbg!(sample); + } + } + + Ok(()) +} diff --git a/perf-event/src/builder.rs b/perf-event/src/builder.rs index fbb1fef..911edf5 100644 --- a/perf-event/src/builder.rs +++ b/perf-event/src/builder.rs @@ -1,7 +1,8 @@ use std::fmt; use std::fs::File; use std::io::{self, ErrorKind}; -use std::os::raw::{c_int, c_ulong}; +use std::os::fd::{AsFd, OwnedFd}; +use std::os::raw::c_ulong; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use std::panic::{RefUnwindSafe, UnwindSafe}; use std::sync::Arc; @@ -9,6 +10,7 @@ use std::sync::Arc; use libc::pid_t; use perf_event_data::parse::ParseConfig; use perf_event_open_sys::bindings; +use thiserror::Error; use crate::events::{Event, EventData}; use crate::sys::bindings::perf_event_attr; @@ -65,11 +67,11 @@ use crate::{ /// perf_event_attr` type. /// /// [`enable`]: Counter::enable -#[derive(Clone)] -pub struct Builder<'a> { +pub struct Builder { attrs: perf_event_attr, - who: EventPid<'a>, - cpu: Option, + /// Possibly holds onto a reference to a cgroup in the form of an open file + /// whose fd is used to identify said cgroup. + who: CpuPid, // Some events need to hold onto data that is referenced in the builder. // The perf_event_attr struct obviously doesn't have lifetimes so the only @@ -78,47 +80,198 @@ pub struct Builder<'a> { } // Needed for backwards compat -impl UnwindSafe for Builder<'_> {} -impl RefUnwindSafe for Builder<'_> {} +impl UnwindSafe for Builder {} +impl RefUnwindSafe for Builder {} -#[derive(Clone, Debug)] -enum EventPid<'a> { - /// Monitor the calling process. - ThisProcess, +/// Couple the CPU and PID options because their validity is mutually dependent. +/// While the kernel documentation explains the meaning of the various +/// combinations of signedness and values, we want to just use the +/// variant that says what it does in words. +/// +/// This enum's methods ensure that all subsequent modifications preserve +/// the validity of the options requested. We use unsigned integers to ensure +/// that invalid states are unrepresentable. Under the hood, we will convert +/// these values to the correct, correponding signed equivalents. +#[derive(Debug, Default)] +#[allow(clippy::enum_variant_names)] +pub enum CpuPid { + /// Measure the calling process (thread) on any CPU. + #[default] + ThisProcessAnyCpu, + + /// Measure the calling process (thread) on a particular CPU. + ThisProcessOneCpu { + /// The CPU core to observe + cpu: usize, + }, + + /// Measure a particular process (thread) on any CPU. + OneProcessAnyCpu { + /// The process ID to observe + pid: usize, + }, + + /// Measure a particular process (thread) on a particular CPU. + OneProcessOneCpu { + /// The process ID to observe + pid: usize, + /// The CPU core to observe + cpu: usize, + }, + + /// Measure all processes (threads) on a particular CPU. Requires + /// `CAP_SYS_ADMIN`, or `CAP_PERFMON` (since Linux 5.8), or having + /// `/proc/sys/kernel/perf_event_paranoid` set to less than 1. + AnyProcessOneCpu { + /// The CPU core to observe + cpu: usize, + }, + + /// Measure a particular cgroup on any CPU. + CGroupAnyCpu { + /// File descriptor for the cgroup to observe + cgroup: OwnedFd, + }, + + /// Measure a particular cgroup on a particular CPU. + CGroupOneCpu { + /// File descriptor for the cgroup to observe + cgroup: OwnedFd, + /// The CPU core to observe + cpu: usize, + }, +} - /// Monitor the given pid. - Other(pid_t), +impl CpuPid { + /// Update settings to observe just the calling process. + /// Note: it will overwrite any previously set cgroup settings. + pub fn observe_calling_process(&mut self) { + if let Self::OneProcessAnyCpu { .. } = self { + *self = Self::ThisProcessAnyCpu + } + } + + /// Update settings to observe just one PID. + /// Note: it will overwrite any previously set cgroup settings. + pub fn observe_pid(&mut self, pid: pid_t) { + let pid = pid as usize; + match self { + Self::ThisProcessAnyCpu => *self = Self::OneProcessAnyCpu { pid }, + Self::ThisProcessOneCpu { cpu } => *self = Self::OneProcessOneCpu { pid, cpu: *cpu }, + Self::OneProcessAnyCpu { .. } => *self = Self::OneProcessAnyCpu { pid }, + Self::OneProcessOneCpu { cpu, .. } => *self = Self::OneProcessOneCpu { pid, cpu: *cpu }, + Self::AnyProcessOneCpu { cpu } => *self = Self::OneProcessOneCpu { pid, cpu: *cpu }, + Self::CGroupAnyCpu { .. } => *self = Self::OneProcessAnyCpu { pid }, + Self::CGroupOneCpu { cpu, .. } => *self = Self::OneProcessOneCpu { pid, cpu: *cpu }, + } + } - /// Monitor members of the given cgroup. - CGroup(&'a File), + /// Update settings to observe just one CPU. + pub fn one_cpu(&mut self, cpu: usize) { + match self { + Self::ThisProcessAnyCpu => *self = Self::ThisProcessOneCpu { cpu }, + Self::ThisProcessOneCpu { .. } => *self = Self::ThisProcessOneCpu { cpu }, + Self::OneProcessAnyCpu { pid } => *self = Self::OneProcessOneCpu { pid: *pid, cpu }, + Self::OneProcessOneCpu { pid, .. } => *self = Self::OneProcessOneCpu { pid: *pid, cpu }, + Self::AnyProcessOneCpu { .. } => *self = Self::AnyProcessOneCpu { cpu }, + Self::CGroupAnyCpu { cgroup } => { + let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); + *self = Self::CGroupOneCpu { + cgroup: owned_fd, + cpu, + }; + } + Self::CGroupOneCpu { cgroup, .. } => { + let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); + *self = Self::CGroupOneCpu { + cgroup: owned_fd, + cpu, + }; + } + } + } - /// Monitor any process on some given CPU. - Any, -} + /// Update settings to observe a specific cgroup. + /// Note: it will overwrite any previously set process/PID settings. + pub fn observe_cgroup(&mut self, cgroup: OwnedFd) { + match self { + Self::ThisProcessOneCpu { cpu } + | Self::OneProcessOneCpu { cpu, .. } + | Self::AnyProcessOneCpu { cpu } + | Self::CGroupOneCpu { cpu, .. } => { + *self = Self::CGroupOneCpu { cgroup, cpu: *cpu }; + } + _ => { + *self = Self::CGroupAnyCpu { cgroup }; + } + } + } -impl<'a> EventPid<'a> { - // Return the `pid` arg and the `flags` bits representing `self`. - fn as_args(&self) -> (pid_t, u32) { + /// Update settings to observe any PID. + /// Note: it will overwrite any previously set cgroup settings. + pub fn observe_any_pid(&mut self) { match self { - EventPid::Any => (-1, 0), - EventPid::ThisProcess => (0, 0), - EventPid::Other(pid) => (*pid, 0), - EventPid::CGroup(file) => (file.as_raw_fd(), sys::bindings::PERF_FLAG_PID_CGROUP), + Self::ThisProcessOneCpu { cpu } + | Self::OneProcessOneCpu { cpu, .. } + | Self::AnyProcessOneCpu { cpu } + | Self::CGroupOneCpu { cpu, .. } => { + *self = Self::AnyProcessOneCpu { cpu: *cpu }; + } + _ => { + *self = Self::AnyProcessOneCpu { cpu: 0 }; + } } } + + /// Update settings to observe any CPU. + /// Note: it will overwrite previously set CPU settings. + pub fn observe_any_cpu(&mut self) { + match self { + Self::ThisProcessOneCpu { .. } | Self::AnyProcessOneCpu { .. } => { + *self = Self::ThisProcessAnyCpu + } + Self::OneProcessOneCpu { pid, .. } => *self = Self::OneProcessAnyCpu { pid: *pid }, + Self::CGroupOneCpu { cgroup, .. } => { + let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); + *self = Self::CGroupAnyCpu { cgroup: owned_fd }; + } + _ => {} + } + } + + /// Return a valid combination of `pid`, `cpu` and `flags` arguments. + fn pid_cpu_flags(&self) -> (pid_t, i32, u32) { + let (pid, cpu, flags) = match self { + Self::ThisProcessAnyCpu => (0, -1, 0), + Self::ThisProcessOneCpu { cpu } => (0, *cpu as i32, 0), + Self::OneProcessAnyCpu { pid } => (*pid as pid_t, -1, 0), + Self::OneProcessOneCpu { pid, cpu } => (*pid as pid_t, *cpu as i32, 0), + Self::AnyProcessOneCpu { cpu } => (-1, *cpu as i32, 0), + Self::CGroupAnyCpu { cgroup } => { + (cgroup.as_raw_fd(), -1, sys::bindings::PERF_FLAG_PID_CGROUP) + } + Self::CGroupOneCpu { cgroup, cpu } => ( + cgroup.as_raw_fd(), + *cpu as i32, + sys::bindings::PERF_FLAG_PID_CGROUP, + ), + }; + + debug_assert!(!(pid == -1 && cpu == -1)); + + (pid, cpu, flags) + } } // Methods that actually do work on the builder and aren't just setting // config values. -impl<'a> Builder<'a> { +impl Builder { /// Return a new `Builder`, with all parameters set to their defaults. /// /// Return a new `Builder` for the specified event. pub fn new(event: E) -> Self { let mut attrs = perf_event_attr::default(); - // Do the update_attrs bit before we set any of the default state so - // that user code can't break configuration we really care about. let data = event.update_attrs_with_data(&mut attrs); // Setting `size` accurately will not prevent the code from working @@ -128,14 +281,10 @@ impl<'a> Builder<'a> { let mut builder = Self { attrs, - who: EventPid::ThisProcess, - cpu: None, + who: CpuPid::default(), event_data: data, }; - builder.enabled(false); - builder.exclude_kernel(true); - builder.exclude_hv(true); builder.read_format(ReadFormat::TOTAL_TIME_ENABLED | ReadFormat::TOTAL_TIME_RUNNING); builder } @@ -298,12 +447,7 @@ impl<'a> Builder<'a> { // must not exceed the size of perf_event_attr. assert!(self.attrs.size <= std::mem::size_of::() as u32); - let cpu = match self.cpu { - Some(cpu) => cpu as c_int, - None => -1, - }; - - let (pid, flags) = self.who.as_args(); + let (pid, cpu, flags) = self.who.pid_cpu_flags(); let group_fd = group_fd.unwrap_or(-1); // Enable CLOEXEC by default. This the behaviour that the rust stdlib @@ -330,9 +474,7 @@ impl<'a> Builder<'a> { Err(e) => Err(e), } } -} -impl<'a> Builder<'a> { /// Directly access the [`perf_event_attr`] within this builder. pub fn attrs(&self) -> &perf_event_attr { &self.attrs @@ -345,7 +487,7 @@ impl<'a> Builder<'a> { /// Observe the calling process. (This is the default.) pub fn observe_self(&mut self) -> &mut Self { - self.who = EventPid::ThisProcess; + self.who.observe_calling_process(); self } @@ -354,7 +496,7 @@ impl<'a> Builder<'a> { /// /// [man-capabilities]: https://www.mankier.com/7/capabilities pub fn observe_pid(&mut self, pid: pid_t) -> &mut Self { - self.who = EventPid::Other(pid); + self.who.observe_pid(pid); self } @@ -374,7 +516,7 @@ impl<'a> Builder<'a> { /// [`one_cpu`]: Builder::one_cpu /// [cap]: https://www.mankier.com/7/capabilities pub fn any_pid(&mut self) -> &mut Self { - self.who = EventPid::Any; + self.who.observe_any_pid(); self } @@ -383,14 +525,15 @@ impl<'a> Builder<'a> { /// in the cgroupfs filesystem. /// /// [man-cgroups]: https://www.mankier.com/7/cgroups - pub fn observe_cgroup(&mut self, cgroup: &'a File) -> &mut Self { - self.who = EventPid::CGroup(cgroup); + pub fn observe_cgroup(&mut self, cgroup: &File) -> &mut Self { + let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); + self.who.observe_cgroup(owned_fd); self } /// Observe only code running on the given CPU core. pub fn one_cpu(&mut self, cpu: usize) -> &mut Self { - self.cpu = Some(cpu); + self.who.one_cpu(cpu); self } @@ -407,7 +550,7 @@ impl<'a> Builder<'a> { /// [`observe_pid`]: Builder::observe_pid /// [`observe_cgroup`]: Builder::observe_cgroup pub fn any_cpu(&mut self) -> &mut Self { - self.cpu = None; + self.who.observe_any_cpu(); self } @@ -461,7 +604,7 @@ impl<'a> Builder<'a> { // Section for methods which directly modify attrs. These should correspond // roughly 1-to-1 with the entries as documented in the manpage. -impl<'a> Builder<'a> { +impl Builder { /// Whether this counter should start off enabled. /// /// When this is set, the counter will immediately start being recorded as @@ -965,14 +1108,19 @@ impl<'a> Builder<'a> { self.attrs.aux_sample_size = sample_size; self } + + /// Which CPU and PID to target. + pub fn targeting(&mut self, cpu_pid: CpuPid) -> &mut Self { + self.who = cpu_pid; + self + } } -impl fmt::Debug for Builder<'_> { +impl fmt::Debug for Builder { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Builder") .field("attrs", &self.attrs) .field("who", &self.who) - .field("cpu", &self.cpu) .field( "event_data", &self.event_data.as_ref().map(|_| ""), @@ -1014,7 +1162,8 @@ impl fmt::Debug for Builder<'_> { /// /// println!("The expected size was {}", inner.expected_size()); /// ``` -#[derive(Debug)] +#[derive(Debug, Error)] +#[error("perf_event_attr contained options not valid for the current kernel")] pub struct UnsupportedOptionsError { expected_size: u32, } @@ -1030,10 +1179,106 @@ impl UnsupportedOptionsError { } } -impl fmt::Display for UnsupportedOptionsError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("perf_event_attr contained options not valid for the current kernel") +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_observe_pid() { + let mut cpu_pid = CpuPid::ThisProcessAnyCpu; + cpu_pid.observe_pid(1234); + assert!(matches!(cpu_pid, CpuPid::OneProcessAnyCpu { pid: 1234 })); + + let mut cpu_pid = CpuPid::ThisProcessOneCpu { cpu: 2 }; + cpu_pid.observe_pid(5678); + assert!(matches!( + cpu_pid, + CpuPid::OneProcessOneCpu { pid: 5678, cpu: 2 } + )); + + let mut cpu_pid = CpuPid::AnyProcessOneCpu { cpu: 1 }; + cpu_pid.observe_pid(999); + assert!(matches!( + cpu_pid, + CpuPid::OneProcessOneCpu { pid: 999, cpu: 1 } + )); } -} -impl std::error::Error for UnsupportedOptionsError {} + #[test] + fn test_one_cpu() { + let mut cpu_pid = CpuPid::ThisProcessAnyCpu; + cpu_pid.one_cpu(3); + assert!(matches!(cpu_pid, CpuPid::ThisProcessOneCpu { cpu: 3 })); + + let mut cpu_pid = CpuPid::OneProcessAnyCpu { pid: 1234 }; + cpu_pid.one_cpu(5); + assert!(matches!( + cpu_pid, + CpuPid::OneProcessOneCpu { pid: 1234, cpu: 5 } + )); + + let mut cpu_pid = CpuPid::AnyProcessOneCpu { cpu: 1 }; + cpu_pid.one_cpu(7); + assert!(matches!(cpu_pid, CpuPid::AnyProcessOneCpu { cpu: 7 })); + } + + #[test] + fn test_observe_any_pid() { + let mut cpu_pid = CpuPid::ThisProcessOneCpu { cpu: 2 }; + cpu_pid.observe_any_pid(); + assert!(matches!(cpu_pid, CpuPid::AnyProcessOneCpu { cpu: 2 })); + + let mut cpu_pid = CpuPid::OneProcessOneCpu { pid: 1234, cpu: 3 }; + cpu_pid.observe_any_pid(); + assert!(matches!(cpu_pid, CpuPid::AnyProcessOneCpu { cpu: 3 })); + + let mut cpu_pid = CpuPid::ThisProcessAnyCpu; + cpu_pid.observe_any_pid(); + assert!(matches!(cpu_pid, CpuPid::AnyProcessOneCpu { cpu: 0 })); + } + + #[test] + fn test_observe_any_cpu() { + let mut cpu_pid = CpuPid::ThisProcessOneCpu { cpu: 2 }; + cpu_pid.observe_any_cpu(); + assert!(matches!(cpu_pid, CpuPid::ThisProcessAnyCpu)); + + let mut cpu_pid = CpuPid::OneProcessOneCpu { pid: 1234, cpu: 3 }; + cpu_pid.observe_any_cpu(); + assert!(matches!(cpu_pid, CpuPid::OneProcessAnyCpu { pid: 1234 })); + + let mut cpu_pid = CpuPid::AnyProcessOneCpu { cpu: 5 }; + cpu_pid.observe_any_cpu(); + assert!(matches!(cpu_pid, CpuPid::ThisProcessAnyCpu)); + } + + #[test] + fn test_pid_cpu_flags() { + let cpu_pid = CpuPid::ThisProcessAnyCpu; + assert_eq!(cpu_pid.pid_cpu_flags(), (0, -1, 0)); + + let cpu_pid = CpuPid::ThisProcessOneCpu { cpu: 2 }; + assert_eq!(cpu_pid.pid_cpu_flags(), (0, 2, 0)); + + let cpu_pid = CpuPid::OneProcessAnyCpu { pid: 1234 }; + assert_eq!(cpu_pid.pid_cpu_flags(), (1234, -1, 0)); + + let cpu_pid = CpuPid::OneProcessOneCpu { pid: 5678, cpu: 3 }; + assert_eq!(cpu_pid.pid_cpu_flags(), (5678, 3, 0)); + + let cpu_pid = CpuPid::AnyProcessOneCpu { cpu: 4 }; + assert_eq!(cpu_pid.pid_cpu_flags(), (-1, 4, 0)); + } + + #[test] + fn test_observe_calling_process() { + let mut cpu_pid = CpuPid::OneProcessAnyCpu { pid: 1234 }; + cpu_pid.observe_calling_process(); + assert!(matches!(cpu_pid, CpuPid::ThisProcessAnyCpu)); + + let mut cpu_pid = CpuPid::ThisProcessOneCpu { cpu: 2 }; + cpu_pid.observe_calling_process(); + // Should remain unchanged since it's not OneProcessAnyCpu + assert!(matches!(cpu_pid, CpuPid::ThisProcessOneCpu { cpu: 2 })); + } +} diff --git a/perf-event/src/events/hardware.rs b/perf-event/src/events/hardware.rs index 56c872e..9ec62e2 100644 --- a/perf-event/src/events/hardware.rs +++ b/perf-event/src/events/hardware.rs @@ -55,5 +55,8 @@ impl Event for Hardware { fn update_attrs(self, attr: &mut bindings::perf_event_attr) { attr.type_ = bindings::PERF_TYPE_HARDWARE; attr.config = self.into(); + attr.set_disabled(1); + attr.set_exclude_kernel(1); + attr.set_exclude_hv(1); } } diff --git a/perf-event/src/events/mod.rs b/perf-event/src/events/mod.rs index a4d254f..f11620e 100644 --- a/perf-event/src/events/mod.rs +++ b/perf-event/src/events/mod.rs @@ -91,6 +91,6 @@ pub trait Event: Sized { /// /// This is automatically implemented for any type which is both `Send` and /// `Sync`. -pub trait EventData: Send + Sync {} +pub trait EventData: Send + Sync + 'static {} -impl EventData for T {} +impl EventData for T {} diff --git a/perf-event/src/events/tracepoint.rs b/perf-event/src/events/tracepoint.rs index 1e2f2ee..35a9f6f 100644 --- a/perf-event/src/events/tracepoint.rs +++ b/perf-event/src/events/tracepoint.rs @@ -1,8 +1,9 @@ +use std::io; use std::num::ParseIntError; use std::path::{Path, PathBuf}; -use std::{fmt, io}; use perf_event_open_sys::bindings; +use thiserror::Error; use crate::events::Event; @@ -57,14 +58,14 @@ impl Tracepoint { /// # let _ = run(); /// ``` pub fn with_name(name: impl AsRef) -> io::Result { - let mut path = PathBuf::from("/sys/kernel/debug/tracing/events"); - path.push(name.as_ref()); - path.push("id"); + let path = PathBuf::from("/sys/kernel/debug/tracing/events") + .join(&name) + .join("id"); let id = std::fs::read_to_string(&path)? .trim_end() .parse() - .map_err(move |e| io::Error::other(UnparseableIdFile::new(path, e)))?; + .map_err(|source| io::Error::other(UnparseableIdFile { path, source }))?; Ok(Self::with_id(id)) } @@ -78,33 +79,19 @@ impl Tracepoint { impl Event for Tracepoint { fn update_attrs(self, attr: &mut bindings::perf_event_attr) { attr.type_ = bindings::PERF_TYPE_TRACEPOINT; + attr.set_exclude_kernel(0); + attr.sample_type |= crate::SampleFlag::RAW.bits(); + attr.sample_period = 1; attr.config = self.id; + attr.set_watermark(0); + attr.wakeup_events = 1; } } -#[derive(Debug)] -struct UnparseableIdFile { +#[derive(Debug, Error)] +#[error("unparseable tracepoint id file `{path:?}`")] +pub struct UnparseableIdFile { path: PathBuf, + #[source] source: ParseIntError, } - -impl UnparseableIdFile { - fn new(path: PathBuf, source: ParseIntError) -> Self { - Self { path, source } - } -} - -impl fmt::Display for UnparseableIdFile { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_fmt(format_args!( - "unparseable tracepoint id file `{}`", - self.path.display() - )) - } -} - -impl std::error::Error for UnparseableIdFile { - fn cause(&self) -> Option<&dyn std::error::Error> { - Some(&self.source) - } -} diff --git a/perf-event/src/group.rs b/perf-event/src/group.rs index 1594ea1..d2814db 100644 --- a/perf-event/src/group.rs +++ b/perf-event/src/group.rs @@ -119,7 +119,7 @@ impl Group { /// /// [`read_format`]: Builder::read_format /// [`build_group`]: Builder::build_group - pub fn builder() -> Builder<'static> { + pub fn builder() -> Builder { let mut builder = Builder::new(Software::DUMMY); builder.read_format( ReadFormat::GROUP diff --git a/perf-event/src/lib.rs b/perf-event/src/lib.rs index 742b90b..70f8378 100644 --- a/perf-event/src/lib.rs +++ b/perf-event/src/lib.rs @@ -64,7 +64,7 @@ //! //! Linux's `perf_event_open` API can report all sorts of things this crate //! doesn't yet understand: stack traces, logs of executable and shared library -//! activity, tracepoints, kprobes, uprobes, and so on. And beyond the counters +//! activity, kprobes, uprobes, and so on. And beyond the counters //! in the kernel header files, there are others that can only be found at //! runtime by consulting `sysfs`, specific to particular processors and //! devices. For example, modern Intel processors have counters that measure @@ -142,7 +142,7 @@ pub use perf_event_data as data; #[cfg(not(feature = "hooks"))] use perf_event_open_sys as sys; -pub use crate::builder::{Builder, UnsupportedOptionsError}; +pub use crate::builder::{Builder, CpuPid, UnsupportedOptionsError}; #[doc(inline)] pub use crate::data::{ReadFormat, SampleFlags as SampleFlag}; pub use crate::flags::{Clock, SampleBranchFlag, SampleSkid}; @@ -884,10 +884,9 @@ mod tests { // CPU_CLOCK is literally always supported so we don't have to worry // about test failures when in VMs. - let builder = Builder::new(events::Software::CPU_CLOCK) - // There should _hopefully_ never be a system with this many CPUs. - .one_cpu(i32::MAX as usize) - .clone(); + let mut prep = Builder::new(events::Software::CPU_CLOCK); + // There should _hopefully_ never be a system with this many CPUs. + let builder = prep.one_cpu(i32::MAX as usize); match builder.build() { Ok(_) => panic!("counter construction was not supposed to succeed"), From b4810812c71a843d5a63188d8ae1c5460f942222 Mon Sep 17 00:00:00 2001 From: Daniel Levin Date: Fri, 22 Aug 2025 10:52:29 +0200 Subject: [PATCH 2/5] Make Builder impl Clone --- perf-event/src/builder.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/perf-event/src/builder.rs b/perf-event/src/builder.rs index 911edf5..4287ecb 100644 --- a/perf-event/src/builder.rs +++ b/perf-event/src/builder.rs @@ -67,6 +67,7 @@ use crate::{ /// perf_event_attr` type. /// /// [`enable`]: Counter::enable +#[derive(Clone)] pub struct Builder { attrs: perf_event_attr, /// Possibly holds onto a reference to a cgroup in the form of an open file @@ -92,7 +93,7 @@ impl RefUnwindSafe for Builder {} /// the validity of the options requested. We use unsigned integers to ensure /// that invalid states are unrepresentable. Under the hood, we will convert /// these values to the correct, correponding signed equivalents. -#[derive(Debug, Default)] +#[derive(Debug, Clone, Default)] #[allow(clippy::enum_variant_names)] pub enum CpuPid { /// Measure the calling process (thread) on any CPU. @@ -130,13 +131,13 @@ pub enum CpuPid { /// Measure a particular cgroup on any CPU. CGroupAnyCpu { /// File descriptor for the cgroup to observe - cgroup: OwnedFd, + cgroup: Arc, }, /// Measure a particular cgroup on a particular CPU. CGroupOneCpu { /// File descriptor for the cgroup to observe - cgroup: OwnedFd, + cgroup: Arc, /// The CPU core to observe cpu: usize, }, @@ -175,16 +176,14 @@ impl CpuPid { Self::OneProcessOneCpu { pid, .. } => *self = Self::OneProcessOneCpu { pid: *pid, cpu }, Self::AnyProcessOneCpu { .. } => *self = Self::AnyProcessOneCpu { cpu }, Self::CGroupAnyCpu { cgroup } => { - let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); *self = Self::CGroupOneCpu { - cgroup: owned_fd, + cgroup: cgroup.clone(), cpu, }; } Self::CGroupOneCpu { cgroup, .. } => { - let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); *self = Self::CGroupOneCpu { - cgroup: owned_fd, + cgroup: cgroup.clone(), cpu, }; } @@ -194,6 +193,7 @@ impl CpuPid { /// Update settings to observe a specific cgroup. /// Note: it will overwrite any previously set process/PID settings. pub fn observe_cgroup(&mut self, cgroup: OwnedFd) { + let cgroup = Arc::new(cgroup); match self { Self::ThisProcessOneCpu { cpu } | Self::OneProcessOneCpu { cpu, .. } @@ -232,8 +232,9 @@ impl CpuPid { } Self::OneProcessOneCpu { pid, .. } => *self = Self::OneProcessAnyCpu { pid: *pid }, Self::CGroupOneCpu { cgroup, .. } => { - let owned_fd = cgroup.as_fd().try_clone_to_owned().unwrap(); - *self = Self::CGroupAnyCpu { cgroup: owned_fd }; + *self = Self::CGroupAnyCpu { + cgroup: cgroup.clone(), + }; } _ => {} } From e6f8cc89d219fbdc166d1941b9d1fa84bfc15d59 Mon Sep 17 00:00:00 2001 From: Daniel Levin Date: Fri, 22 Aug 2025 10:56:09 +0200 Subject: [PATCH 3/5] Bump version number --- perf-event/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/perf-event/Cargo.toml b/perf-event/Cargo.toml index 5d67901..ef3e54b 100644 --- a/perf-event/Cargo.toml +++ b/perf-event/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "perf-event2" -version = "0.7.4" +version = "0.8.0" description = "A Rust interface to Linux performance monitoring" license = "MIT OR Apache-2.0" authors = ["Sean Lynch ", "Jim Blandy "] From 30a866d404cf46c1322900401ef4aed337c1efd8 Mon Sep 17 00:00:00 2001 From: Daniel Levin Date: Fri, 22 Aug 2025 11:43:03 +0200 Subject: [PATCH 4/5] Better documentation for Builder --- perf-event/src/builder.rs | 22 +++++++++++++++++++++- perf-event/src/events/tracepoint.rs | 8 ++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/perf-event/src/builder.rs b/perf-event/src/builder.rs index 4287ecb..4eb68a9 100644 --- a/perf-event/src/builder.rs +++ b/perf-event/src/builder.rs @@ -267,7 +267,27 @@ impl CpuPid { // Methods that actually do work on the builder and aren't just setting // config values. impl Builder { - /// Return a new `Builder`, with all parameters set to their defaults. + /// Return a new `Builder`, with all parameters set to their defaults for + /// the given event type. Each event type sets opinionated, unique + /// defaults. The defaults are chosen so as to make the resultant Builder + /// maximally useful. See the documentation for [crate::events::Tracepoint] + /// as an example. + /// + /// ``` + /// # use perf_event::events::Tracepoint; + /// # use perf_event::Builder; + /// # use perf_event::events::Hardware; + /// let sched_switch = Tracepoint::with_id(0xabcd); + /// let cpu_cycles = Hardware::CPU_CYCLES; + /// + /// // b1 has defaults sensible for tracepoints. + /// let b1 = Builder::new(sched_switch); + /// assert_eq!(b1.attrs().wakeup_events, 1); + /// + /// // b2 has defaults sensible for hardware events. + /// let b2 = Builder::new(cpu_cycles); + /// assert_eq!(b2.attrs().wakeup_events, 0); + /// ``` /// /// Return a new `Builder` for the specified event. pub fn new(event: E) -> Self { diff --git a/perf-event/src/events/tracepoint.rs b/perf-event/src/events/tracepoint.rs index 35a9f6f..400d19f 100644 --- a/perf-event/src/events/tracepoint.rs +++ b/perf-event/src/events/tracepoint.rs @@ -22,6 +22,14 @@ use crate::events::Event; /// [`perf probe`]. /// /// [`perf probe`]: https://man7.org/linux/man-pages/man1/perf-probe.1.html +/// +/// By default [Self::update_attrs] updates the defaults to those that make +/// sense for tracepoints: +/// 1. Named, static tracepoints are intrinsically kernel events, so +/// `exclude_kernel` is set to false. +/// 2. The watermark and wakeup values are set so that +/// [crate::Sampler::next_blocking] works by default. +/// 3. Sampling period is set to 1 - all tracepoints trigger. #[derive(Clone, Copy, Debug)] pub struct Tracepoint { id: u64, From 44a1a6462c99ba0cebc1a6b506a544257344410c Mon Sep 17 00:00:00 2001 From: Daniel Levin Date: Fri, 22 Aug 2025 11:51:48 +0200 Subject: [PATCH 5/5] Restored test's usage of Builder::clone --- perf-event/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/perf-event/src/lib.rs b/perf-event/src/lib.rs index 70f8378..db3e718 100644 --- a/perf-event/src/lib.rs +++ b/perf-event/src/lib.rs @@ -884,9 +884,11 @@ mod tests { // CPU_CLOCK is literally always supported so we don't have to worry // about test failures when in VMs. - let mut prep = Builder::new(events::Software::CPU_CLOCK); // There should _hopefully_ never be a system with this many CPUs. - let builder = prep.one_cpu(i32::MAX as usize); + let builder = Builder::new(events::Software::CPU_CLOCK) + // There should _hopefully_ never be a system with this many CPUs. + .one_cpu(i32::MAX as usize) + .clone(); match builder.build() { Ok(_) => panic!("counter construction was not supposed to succeed"),