From dd821b48e819d23c0b264c6188faf6178b91d8e4 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Fri, 15 Apr 2016 04:45:34 -0700 Subject: [PATCH] perf-source: fix acquisition of time for comm and mmap events We were not getting the proper value for time. We need sample_id_all set according to my cursory reading of the core.c in the kernel. Also, the time is aligned to 64-bit (after the comm field). --- lib/sp-perf-counter.h | 12 ------------ lib/sp-perf-source.c | 33 ++++++++++++++++++++------------- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/lib/sp-perf-counter.h b/lib/sp-perf-counter.h index 2d870ad6..c97eb0c8 100644 --- a/lib/sp-perf-counter.h +++ b/lib/sp-perf-counter.h @@ -31,18 +31,6 @@ typedef struct _SpPerfCounter SpPerfCounter; #pragma pack(push, 1) -typedef struct -{ - /* - * These fields are available as the suffix only because we have specified - * them when creating attributes. Be careful about using them. - * Ideally, we would probably switch from using structures overlaid with - * casts to a reader design, which knows about the attributes. - */ - guint32 pid, tid; - guint64 time; -} SpPerfCounterSuffix; - typedef struct { struct perf_event_header header; diff --git a/lib/sp-perf-source.c b/lib/sp-perf-source.c index e5d44559..5406fbeb 100644 --- a/lib/sp-perf-source.c +++ b/lib/sp-perf-source.c @@ -165,13 +165,21 @@ sp_perf_source_handle_sample (SpPerfSource *self, n_ips); } +static inline void +realign (gsize *pos, + gsize align) +{ + *pos = (*pos + align - 1) & ~(align - 1); +} + static void sp_perf_source_handle_event (SpPerfCounterEvent *event, guint cpu, gpointer user_data) { SpPerfSource *self = user_data; - SpPerfCounterSuffix *suffix; + gsize offset; + gint64 time; g_assert (SP_IS_PERF_SOURCE (self)); g_assert (event != NULL); @@ -179,13 +187,12 @@ sp_perf_source_handle_event (SpPerfCounterEvent *event, switch (event->header.type) { case PERF_RECORD_COMM: - suffix = (SpPerfCounterSuffix *)event->raw - + G_STRUCT_OFFSET (SpPerfCounterEventComm, comm) - + strlen (event->comm.comm) - + 1; + offset = strlen (event->comm.comm) + 1; + realign (&offset, sizeof (guint64)); + memcpy (&time, event->comm.comm + offset, sizeof time); sp_capture_writer_add_process (self->writer, - suffix->time, + time, cpu, event->comm.pid, event->comm.comm); @@ -230,13 +237,12 @@ sp_perf_source_handle_event (SpPerfCounterEvent *event, break; case PERF_RECORD_MMAP: - suffix = (SpPerfCounterSuffix *)event->raw - + G_STRUCT_OFFSET (SpPerfCounterEventMmap, filename) - + strlen (event->mmap.filename) - + 1; + offset = strlen (event->mmap.filename) + 1; + realign (&offset, sizeof (guint64)); + memcpy (&time, event->mmap.filename + offset, sizeof time); sp_capture_writer_add_map (self->writer, - suffix->time, + time, cpu, event->mmap.pid, event->mmap.addr, @@ -275,7 +281,6 @@ sp_perf_source_start_pid (SpPerfSource *self, g_assert (SP_IS_PERF_SOURCE (self)); attr.sample_type = PERF_SAMPLE_IP - | PERF_SAMPLE_TID | PERF_SAMPLE_CALLCHAIN | PERF_SAMPLE_TIME; attr.wakeup_events = N_WAKEUP_EVENTS; @@ -284,9 +289,11 @@ sp_perf_source_start_pid (SpPerfSource *self, attr.comm = 1; attr.task = 1; attr.exclude_idle = 1; - attr.size = sizeof attr; attr.clockid = sp_clock; attr.use_clockid = 1; + attr.sample_id_all = 1; + + attr.size = sizeof attr; if (pid != -1) {