Merge pull request OptimatistOpenSource#26 from Thaumy/counting-read-…

…format

Remove `PERF_FORMAT_GROUP` in single event counting

src/perf_event/counting/config/mod.rs

@@ -63,7 +63,7 @@ impl Config {
         &self.perf_event_attr
     }
 
-    pub fn as_mut_raw(&mut self) -> &mut PerfEventAttr {
+    pub fn as_raw_mut(&mut self) -> &mut PerfEventAttr {
         &mut self.perf_event_attr
     }
 }

src/perf_event/counting/config/new.rs

@@ -33,15 +33,9 @@ pub fn new<'t>(
         // not use in counting mode
         __bindgen_anon_1: perf_event_attr__bindgen_ty_1::default(),
         sample_type: 0, // ditto
-        read_format: {
-            #[allow(clippy::identity_op)] // for readable
-            let val = 0
-                | PERF_FORMAT_TOTAL_TIME_ENABLED
-                | PERF_FORMAT_TOTAL_TIME_RUNNING
-                | PERF_FORMAT_ID
-                | PERF_FORMAT_GROUP;
-            val as _
-        },
+        // `Counter::new` or `CounterGroup::add_member` will clone this struct
+        // and set `read_format` for their read format
+        read_format: 0,
         _bitfield_align_1: [],
         // set later via perf_event_attr.set_...
         _bitfield_1: __BindgenBitfieldUnit::new([0u8; 8usize]),

src/perf_event/counting/group/mod.rs

@@ -29,6 +29,7 @@ use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard};
 use crate::config;
 use crate::config::{Cpu, Process};
 use crate::counting::group::inner::Inner;
+use crate::syscall::bindings::*;
 pub use fixed::*;
 #[allow(unused_imports)]
 pub use guard::*;
@@ -61,10 +62,20 @@ impl CounterGroup {
         self.inner.write().unwrap()
     }
 
-    pub fn add_member(&mut self, cfg: &Config) -> io::Result<CounterGuard> {
+    pub fn add_member(&mut self, cfg: &mut Config) -> io::Result<CounterGuard> {
+        let perf_event_attr = cfg.as_raw_mut();
+        // not inline `read_format` for readable
+        #[rustfmt::skip]
+        let read_format =
+              PERF_FORMAT_TOTAL_TIME_ENABLED
+            | PERF_FORMAT_TOTAL_TIME_RUNNING
+            | PERF_FORMAT_ID
+            | PERF_FORMAT_GROUP;
+        perf_event_attr.read_format = read_format as _;
+
         let event_id = self
             .inner_mut()
-            .add_member(self.cpu, self.pid, cfg.as_raw())?;
+            .add_member(self.cpu, self.pid, perf_event_attr)?;
         CounterGuard::new(event_id, self.inner.clone()).wrap_ok()
     }
 

src/perf_event/counting/group/stat.rs

@@ -14,12 +14,27 @@
 
 use crate::counting::group::guard::CounterGuard;
 use crate::counting::group::inner::Inner;
-use crate::counting::{ReadFormatHead, ReadFormatValue};
 use crate::infra::{BoxSliceExt, WrapResult};
 use std::collections::HashMap;
 use std::io::{ErrorKind, Read};
 use std::{io, slice};
 
+#[repr(C)]
+#[derive(Debug, Clone)]
+struct ReadFormatHead {
+    pub members_len: u64,  // u64 nr;
+    pub time_enabled: u64, // u64 time_enabled;
+    pub time_running: u64, // u64 time_running;
+                           // ReadFormatValue values[nr];
+}
+
+#[repr(C)]
+#[derive(Debug, Clone)]
+struct ReadFormatValue {
+    pub event_count: u64, // u64 value;
+    pub event_id: u64,    // u64 id;
+}
+
 #[derive(Debug, Clone)]
 pub struct CounterGroupStat {
     pub time_enabled: u64,
@@ -33,7 +48,7 @@ impl CounterGroupStat {
         (*self.member_counts.get(&guard.event_id()).unwrap()).wrap_ok()
     }
 
-    pub(crate) fn from_raw(head: &ReadFormatHead, values: &[ReadFormatValue]) -> Self {
+    fn from_raw(head: &ReadFormatHead, values: &[ReadFormatValue]) -> Self {
         Self {
             time_enabled: head.time_enabled,
             time_running: head.time_running,

src/perf_event/counting/group/tests/mod.rs

@@ -44,8 +44,8 @@ where
     F: FnMut(),
 {
     let mut group = gen_group();
-    let ev_1_guard = group.add_member(&gen_cfg(ev_1)).unwrap();
-    let ev_2_guard = group.add_member(&gen_cfg(ev_2)).unwrap();
+    let ev_1_guard = group.add_member(&mut gen_cfg(ev_1)).unwrap();
+    let ev_2_guard = group.add_member(&mut gen_cfg(ev_2)).unwrap();
 
     {
         let result = group.stat().unwrap();
@@ -81,8 +81,8 @@ where
     F: FnMut(),
 {
     let mut group = gen_group();
-    let ev_1_guard = group.add_member(&gen_cfg(ev_1)).unwrap();
-    let ev_2_guard = group.add_member(&gen_cfg(ev_2)).unwrap();
+    let ev_1_guard = group.add_member(&mut gen_cfg(ev_1)).unwrap();
+    let ev_2_guard = group.add_member(&mut gen_cfg(ev_2)).unwrap();
 
     {
         let result = group.stat().unwrap();
@@ -118,8 +118,8 @@ where
     F: FnMut(),
 {
     let mut group = gen_group();
-    let ev_1_guard = group.add_member(&gen_cfg(ev_1)).unwrap();
-    let ev_2_guard = group.add_member(&gen_cfg(ev_2)).unwrap();
+    let ev_1_guard = group.add_member(&mut gen_cfg(ev_1)).unwrap();
+    let ev_2_guard = group.add_member(&mut gen_cfg(ev_2)).unwrap();
 
     let mut group = group.enable().unwrap();
     workload();
@@ -149,8 +149,8 @@ where
     F: FnMut(),
 {
     let mut group = gen_group();
-    let mut ev_1_guard = group.add_member(&gen_cfg(ev_1)).unwrap();
-    let mut ev_2_guard = group.add_member(&gen_cfg(ev_2)).unwrap();
+    let mut ev_1_guard = group.add_member(&mut gen_cfg(ev_1)).unwrap();
+    let mut ev_2_guard = group.add_member(&mut gen_cfg(ev_2)).unwrap();
 
     {
         let ev_1 = ev_1_guard.stat().unwrap().event_count;

src/perf_event/counting/mod.rs

@@ -20,19 +20,3 @@ mod single;
 pub use config::*;
 pub use group::*;
 pub use single::*;
-
-#[repr(C)]
-#[derive(Debug, Clone)]
-pub(crate) struct ReadFormatHead {
-    pub members_len: u64,  // u64 nr;
-    pub time_enabled: u64, // u64 time_enabled;
-    pub time_running: u64, // u64 time_running;
-                           // ReadFormatValue values[nr];
-}
-
-#[repr(C)]
-#[derive(Debug, Clone)]
-pub(crate) struct ReadFormatValue {
-    pub event_count: u64, // u64 value;
-    pub event_id: u64,    // u64 id;
-}

src/perf_event/counting/single/mod.rs

@@ -32,12 +32,22 @@ pub struct Counter {
 }
 
 impl Counter {
-    pub fn new(process: &Process, cpu: &Cpu, cfg: &Config) -> config::Result<Self> {
+    pub fn new(process: &Process, cpu: &Cpu, cfg: &mut Config) -> config::Result<Self> {
         let (pid, cpu) = match (process.as_i32()?, cpu.as_i32()) {
             (-1, -1) => return Err(Error::InvalidProcessCpu),
             (pid, cpu) => (pid, cpu),
         };
-        let fd = unsafe { perf_event_open_wrapped(cfg.as_raw(), pid, cpu, -1, 0) }
+
+        let perf_event_attr = cfg.as_raw_mut();
+        // not inline `read_format` for readable
+        #[rustfmt::skip]
+        let read_format =
+              PERF_FORMAT_TOTAL_TIME_ENABLED
+            | PERF_FORMAT_TOTAL_TIME_RUNNING
+            | PERF_FORMAT_ID;
+        perf_event_attr.read_format = read_format as _;
+
+        let fd = unsafe { perf_event_open_wrapped(perf_event_attr, pid, cpu, -1, 0) }
             .map_err(Error::SyscallFailed)?;
         let file = unsafe { File::from_raw_fd(fd) };
 

src/perf_event/counting/single/stat.rs

@@ -12,7 +12,7 @@
 // You should have received a copy of the GNU Lesser General Public License along with Perf-event-rs. If not,
 // see <https://www.gnu.org/licenses/>.
 
-use crate::counting::{Counter, ReadFormatHead, ReadFormatValue};
+use crate::counting::Counter;
 use crate::infra::{SizedExt, WrapResult};
 use std::io;
 use std::io::Read;
@@ -28,21 +28,34 @@ pub struct CounterStat {
 
 #[inline]
 pub fn counter_stat(counter: &mut Counter) -> io::Result<CounterStat> {
+    /*
+    struct read_format {
+        u64 value;         /* The value of the event */
+        u64 time_enabled;  /* if PERF_FORMAT_TOTAL_TIME_ENABLED */
+        u64 time_running;  /* if PERF_FORMAT_TOTAL_TIME_RUNNING */
+        u64 id;            /* if PERF_FORMAT_ID */
+        u64 lost;          /* if PERF_FORMAT_LOST */
+    };
+    */
     #[repr(C)]
     struct Layout {
-        head: ReadFormatHead,
-        value: ReadFormatValue,
+        event_count: u64,
+        time_enabled: u64,
+        time_running: u64,
+        event_id: u64,
     }
 
     let mut buf = unsafe { <[u8; size_of::<Layout>()]>::uninit() };
     counter.file.read_exact(&mut buf)?;
 
-    let layout = unsafe { &*(buf.as_ptr() as *const Layout) };
+    let layout_ptr = buf.as_ptr() as *const Layout;
+    let layout = unsafe { &*layout_ptr };
+
     CounterStat {
-        event_id: layout.value.event_id,
-        event_count: layout.value.event_count,
-        time_enabled: layout.head.time_enabled,
-        time_running: layout.head.time_running,
+        event_id: layout.event_id,
+        event_count: layout.event_count,
+        time_enabled: layout.time_enabled,
+        time_running: layout.time_running,
     }
     .wrap_ok()
 }

src/perf_event/counting/single/tests/mod.rs

@@ -30,9 +30,9 @@ where
 
 fn gen_counter(ev: &Event) -> Counter {
     let scopes = [EventScope::User, EventScope::Host];
-    let cfg = Config::new(ev, &scopes);
+    let mut cfg = Config::new(ev, &scopes);
 
-    Counter::new(&Process::Current, &Cpu::Any, &cfg).unwrap()
+    Counter::new(&Process::Current, &Cpu::Any, &mut cfg).unwrap()
 }
 
 fn test_stat<F>(ev: &Event, workload: &mut F)