2021-06-22 15:40:17

by Jiri Olsa

Subject: [RFC 00/10] perf: Add build id parsing fault detection/fix

hi,
this *RFC* patchset adds support to detect faults during
mmap2's build id parsing and a way to fix such maps in
generated perf.data.

It adds support to record build id faults count for session
and store it in perf.data and perf inject support to find
these maps and reads build ids for them in user space.

It's probably best explained by the workflow:

Record data with --buildid-mmap option:

# perf record --buildid-mmap ...
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Failed to parse 4 build ids]
[ perf record: Captured and wrote 0.008 MB perf.data ]

Check if there's any build id fault reported:

# perf report --header-only
...
# build id mmap stats: FAULTS 4, LOST 0, NOT FIXED

There is, check the stats:

# perf report --stat

Aggregated stats:
TOTAL events: 104
....
BUILD_ID fails: 4 (14.3%)

Yep, let's fix it:

# perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data

And verify:

# perf report -i perf-fixed.data --stats

Aggregated stats:
TOTAL events: 104
....

Good, let's see how many we fixed:

# perf report --header-only -i perf-fixed.data
...
# build id mmap stats: FAULTS 4, LOST 0, FIXED(4)


I don't have a good way to test it, just by artificially
adding the faults in kernel code, but Ian and Namhyung
might have setup that could generate that.. would be great
to have a perf test for this.

Also available in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/buildid_stats

thoughts?

thanks,
jirka


---
Jiri Olsa (10):
perf: Track build id faults for mmap2 event
perf: Move build_id_parse to check only regular files
perf: Add new read_format bit to read build id faults
perf: Add new read_format bit to read lost events
tools: Sync perf_event.h uapi
libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read
perf record: Add support to read build id fails
perf record: Add new HEADER_BUILD_ID_MMAP feature
perf report: Display build id fails stats
perf inject: Add --buildid-mmap2 option to fix failed build ids

include/linux/perf_event.h | 2 ++
include/uapi/linux/perf_event.h | 20 +++++++++++++-------
kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
kernel/events/ring_buffer.c | 3 +++
tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
tools/lib/perf/evsel.c | 10 ++++++++++
tools/lib/perf/include/perf/evsel.h | 11 ++++++++++-
tools/perf/Documentation/perf-inject.txt | 3 +++
tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++++
tools/perf/builtin-inject.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
tools/perf/builtin-record.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
tools/perf/util/env.h | 6 ++++++
tools/perf/util/evsel.c | 12 ++++++++++++
tools/perf/util/header.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/header.h | 1 +
tools/perf/util/map.h | 15 +++++++++++++++
tools/perf/util/perf_event_attr_fprintf.c | 3 ++-
18 files changed, 407 insertions(+), 24 deletions(-)

2021-06-22 15:40:21

by Jiri Olsa

Subject: [PATCH 01/10] perf: Track build id faults for mmap2 event

Keep track of build id parsing faults per event,
so we can report it in following patches.

Signed-off-by: Jiri Olsa <[email protected]>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 11 +++++++++--
2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index f5a6a2f069ed..5110a998f59b 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -669,6 +669,7 @@ struct perf_event {
unsigned int attach_state;
local64_t count;
atomic64_t child_count;
+ local64_t build_id_faults;

/*
* These are the total time in nanoseconds that the event
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6fee4a7e88d7..d83ccb58a3c1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8183,6 +8183,7 @@ struct perf_mmap_event {
u32 prot, flags;
u8 build_id[BUILD_ID_SIZE_MAX];
u32 build_id_size;
+ bool build_id_fault;

struct {
struct perf_event_header header;
@@ -8244,6 +8245,9 @@ static void perf_event_mmap_output(struct perf_event *event,
if (event->attr.mmap2 && use_build_id)
mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID;

+ if (mmap_event->build_id_fault)
+ local64_inc(&event->build_id_faults);
+
perf_output_put(&handle, mmap_event->event_id);

if (event->attr.mmap2) {
@@ -8386,8 +8390,11 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)

mmap_event->event_id.header.size = sizeof(mmap_event->event_id) + size;

- if (atomic_read(&nr_build_id_events))
- build_id_parse(vma, mmap_event->build_id, &mmap_event->build_id_size);
+ if (atomic_read(&nr_build_id_events)) {
+ int err = build_id_parse(vma, mmap_event->build_id,
+ &mmap_event->build_id_size);
+ mmap_event->build_id_fault = err == -EFAULT;
+ }

perf_iterate_sb(perf_event_mmap_output,
mmap_event,
--
2.31.1

2021-06-22 15:40:28

by Jiri Olsa

Subject: [PATCH 02/10] perf: Move build_id_parse to check only regular files

Moving the build_id_parse function to place where it
parses only regular files.

There's no change wrt maps we generate the build id for,
because we get parsed build ids for only for regular
files (non-special files) anyway.

Signed-off-by: Jiri Olsa <[email protected]>
---
kernel/events/core.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index d83ccb58a3c1..3f1630c06195 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8336,6 +8336,12 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
maj = MAJOR(dev);
min = MINOR(dev);

+ if (atomic_read(&nr_build_id_events) && *name == '/') {
+ int err = build_id_parse(vma, mmap_event->build_id,
+ &mmap_event->build_id_size);
+ mmap_event->build_id_fault = err == -EFAULT;
+ }
+
goto got_name;
} else {
if (vma->vm_ops && vma->vm_ops->name) {
@@ -8390,12 +8396,6 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)

mmap_event->event_id.header.size = sizeof(mmap_event->event_id) + size;

- if (atomic_read(&nr_build_id_events)) {
- int err = build_id_parse(vma, mmap_event->build_id,
- &mmap_event->build_id_size);
- mmap_event->build_id_fault = err == -EFAULT;
- }
-
perf_iterate_sb(perf_event_mmap_output,
mmap_event,
NULL);
--
2.31.1

2021-06-22 15:40:32

by Jiri Olsa

Subject: [PATCH 03/10] perf: Add new read_format bit to read build id faults

It's now possible to retrieve build id faults stats by read
syscall on events with perf_event_attr::build_id set.

Adding PERF_FORMAT_BUILD_ID_FAULTS read_format bit to get
the value of build_id_faults through the read syscall.

Signed-off-by: Jiri Olsa <[email protected]>
---
include/uapi/linux/perf_event.h | 17 ++++++++++-------
kernel/events/core.c | 21 ++++++++++++++++++---
2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index f92880a15645..2424ba7f95fb 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -296,16 +296,18 @@ enum {
*
* struct read_format {
* { u64 value;
- * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
- * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
- * { u64 id; } && PERF_FORMAT_ID
+ * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
+ * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
+ * { u64 id; } && PERF_FORMAT_ID
+ * { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
* } && !PERF_FORMAT_GROUP
*
* { u64 nr;
- * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
- * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
+ * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
+ * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
* { u64 value;
- * { u64 id; } && PERF_FORMAT_ID
+ * { u64 id; } && PERF_FORMAT_ID
+ * { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
* } cntr[nr];
* } && PERF_FORMAT_GROUP
* };
@@ -315,8 +317,9 @@ enum perf_event_read_format {
PERF_FORMAT_TOTAL_TIME_RUNNING = 1U << 1,
PERF_FORMAT_ID = 1U << 2,
PERF_FORMAT_GROUP = 1U << 3,
+ PERF_FORMAT_BUILD_ID_FAULTS = 1U << 4,

- PERF_FORMAT_MAX = 1U << 4, /* non-ABI */
+ PERF_FORMAT_MAX = 1U << 5, /* non-ABI */
};

#define PERF_ATTR_SIZE_VER0 64 /* sizeof first published struct */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3f1630c06195..f3cd06012115 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1840,6 +1840,9 @@ static void __perf_event_read_size(struct perf_event *event, int nr_siblings)
if (event->attr.read_format & PERF_FORMAT_ID)
entry += sizeof(u64);

+ if (event->attr.read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ entry += sizeof(u64);
+
if (event->attr.read_format & PERF_FORMAT_GROUP) {
nr += nr_siblings;
size += sizeof(u64);
@@ -5247,11 +5250,15 @@ static int __perf_read_group_add(struct perf_event *leader,
values[n++] += perf_event_count(leader);
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(leader);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&leader->build_id_faults);

for_each_sibling_event(sub, leader) {
values[n++] += perf_event_count(sub);
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(sub);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&sub->build_id_faults);
}

raw_spin_unlock_irqrestore(&ctx->lock, flags);
@@ -5308,7 +5315,7 @@ static int perf_read_one(struct perf_event *event,
u64 read_format, char __user *buf)
{
u64 enabled, running;
- u64 values[4];
+ u64 values[5];
int n = 0;

values[n++] = __perf_event_read_value(event, &enabled, &running);
@@ -5318,6 +5325,8 @@ static int perf_read_one(struct perf_event *event,
values[n++] = running;
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(event);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&event->build_id_faults);

if (copy_to_user(buf, values, n * sizeof(u64)))
return -EFAULT;
@@ -6820,7 +6829,7 @@ static void perf_output_read_one(struct perf_output_handle *handle,
u64 enabled, u64 running)
{
u64 read_format = event->attr.read_format;
- u64 values[4];
+ u64 values[5];
int n = 0;

values[n++] = perf_event_count(event);
@@ -6834,6 +6843,8 @@ static void perf_output_read_one(struct perf_output_handle *handle,
}
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(event);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&event->build_id_faults);

__output_copy(handle, values, n * sizeof(u64));
}
@@ -6844,7 +6855,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,
{
struct perf_event *leader = event->group_leader, *sub;
u64 read_format = event->attr.read_format;
- u64 values[5];
+ u64 values[6];
int n = 0;

values[n++] = 1 + leader->nr_siblings;
@@ -6862,6 +6873,8 @@ static void perf_output_read_group(struct perf_output_handle *handle,
values[n++] = perf_event_count(leader);
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(leader);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&leader->build_id_faults);

__output_copy(handle, values, n * sizeof(u64));

@@ -6875,6 +6888,8 @@ static void perf_output_read_group(struct perf_output_handle *handle,
values[n++] = perf_event_count(sub);
if (read_format & PERF_FORMAT_ID)
values[n++] = primary_event_id(sub);
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ values[n++] = local64_read(&sub->build_id_faults);

__output_copy(handle, values, n * sizeof(u64));
}
--
2.31.1

2021-06-22 15:40:38

by Jiri Olsa

Subject: [PATCH 04/10] perf: Add new read_format bit to read lost events

It's now possible to retrieve lost stats by read
syscall on events.

Adding PERF_FORMAT_LOST read_format bit to get the
value of lost events through the read syscall.

Signed-off-by: Jiri Olsa <[email protected]>
---
include/linux/perf_event.h | 1 +
include/uapi/linux/perf_event.h | 5 ++++-
kernel/events/core.c | 21 ++++++++++++++++++---
kernel/events/ring_buffer.c | 3 +++
4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 5110a998f59b..209c66a01797 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -670,6 +670,7 @@ struct perf_event {
local64_t count;
atomic64_t child_count;
local64_t build_id_faults;
+ local64_t lost;

/*
* These are the total time in nanoseconds that the event
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 2424ba7f95fb..e742c8f43a18 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -300,6 +300,7 @@ enum {
* { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
* { u64 id; } && PERF_FORMAT_ID
* { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
+ * { u64 lost; } && PERF_FORMAT_LOST
* } && !PERF_FORMAT_GROUP
*
* { u64 nr;
@@ -308,6 +309,7 @@ enum {
* { u64 value;
* { u64 id; } && PERF_FORMAT_ID
* { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
+ * { u64 lost; } && PERF_FORMAT_LOST
* } cntr[nr];
* } && PERF_FORMAT_GROUP
* };
@@ -318,8 +320,9 @@ enum perf_event_read_format {
PERF_FORMAT_ID = 1U << 2,
PERF_FORMAT_GROUP = 1U << 3,
PERF_FORMAT_BUILD_ID_FAULTS = 1U << 4,
+ PERF_FORMAT_LOST = 1U << 5,

- PERF_FORMAT_MAX = 1U << 5, /* non-ABI */
+ PERF_FORMAT_MAX = 1U << 6, /* non-ABI */
};

#define PERF_ATTR_SIZE_VER0 64 /* sizeof first published struct */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f3cd06012115..ba02ce9e9134 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1843,6 +1843,9 @@ static void __perf_event_read_size(struct perf_event *event, int nr_siblings)
if (event->attr.read_format & PERF_FORMAT_BUILD_ID_FAULTS)
entry += sizeof(u64);

+ if (event->attr.read_format & PERF_FORMAT_LOST)
+ entry += sizeof(u64);
+
if (event->attr.read_format & PERF_FORMAT_GROUP) {
nr += nr_siblings;
size += sizeof(u64);
@@ -5252,6 +5255,8 @@ static int __perf_read_group_add(struct perf_event *leader,
values[n++] = primary_event_id(leader);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&leader->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&leader->lost);

for_each_sibling_event(sub, leader) {
values[n++] += perf_event_count(sub);
@@ -5259,6 +5264,8 @@ static int __perf_read_group_add(struct perf_event *leader,
values[n++] = primary_event_id(sub);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&sub->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&sub->lost);
}

raw_spin_unlock_irqrestore(&ctx->lock, flags);
@@ -5315,7 +5322,7 @@ static int perf_read_one(struct perf_event *event,
u64 read_format, char __user *buf)
{
u64 enabled, running;
- u64 values[5];
+ u64 values[6];
int n = 0;

values[n++] = __perf_event_read_value(event, &enabled, &running);
@@ -5327,6 +5334,8 @@ static int perf_read_one(struct perf_event *event,
values[n++] = primary_event_id(event);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&event->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&event->lost);

if (copy_to_user(buf, values, n * sizeof(u64)))
return -EFAULT;
@@ -6829,7 +6838,7 @@ static void perf_output_read_one(struct perf_output_handle *handle,
u64 enabled, u64 running)
{
u64 read_format = event->attr.read_format;
- u64 values[5];
+ u64 values[6];
int n = 0;

values[n++] = perf_event_count(event);
@@ -6845,6 +6854,8 @@ static void perf_output_read_one(struct perf_output_handle *handle,
values[n++] = primary_event_id(event);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&event->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&event->lost);

__output_copy(handle, values, n * sizeof(u64));
}
@@ -6855,7 +6866,7 @@ static void perf_output_read_group(struct perf_output_handle *handle,
{
struct perf_event *leader = event->group_leader, *sub;
u64 read_format = event->attr.read_format;
- u64 values[6];
+ u64 values[7];
int n = 0;

values[n++] = 1 + leader->nr_siblings;
@@ -6875,6 +6886,8 @@ static void perf_output_read_group(struct perf_output_handle *handle,
values[n++] = primary_event_id(leader);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&leader->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&leader->lost);

__output_copy(handle, values, n * sizeof(u64));

@@ -6890,6 +6903,8 @@ static void perf_output_read_group(struct perf_output_handle *handle,
values[n++] = primary_event_id(sub);
if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
values[n++] = local64_read(&sub->build_id_faults);
+ if (read_format & PERF_FORMAT_LOST)
+ values[n++] = local64_read(&sub->lost);

__output_copy(handle, values, n * sizeof(u64));
}
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 52868716ec35..51738bc7cf44 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -248,6 +248,9 @@ __perf_output_begin(struct perf_output_handle *handle,
perf_event_header__init_id(&lost_event.header, data, event);
perf_output_put(handle, lost_event);
perf_event__output_id_sample(event, handle, data);
+
+ /* Keep track of lost events in event for PERF_FORMAT_LOST */
+ local64_add(lost_event.lost, &event->lost);
}

return 0;
--
2.31.1

2021-06-22 15:40:59

by Jiri Olsa

Subject: [PATCH 05/10] tools: Sync perf_event.h uapi

Update tools uapi headers with latest changes.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index f92880a15645..e742c8f43a18 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -296,16 +296,20 @@ enum {
*
* struct read_format {
* { u64 value;
- * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
- * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
- * { u64 id; } && PERF_FORMAT_ID
+ * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
+ * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
+ * { u64 id; } && PERF_FORMAT_ID
+ * { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
+ * { u64 lost; } && PERF_FORMAT_LOST
* } && !PERF_FORMAT_GROUP
*
* { u64 nr;
- * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
- * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
+ * { u64 time_enabled; } && PERF_FORMAT_TOTAL_TIME_ENABLED
+ * { u64 time_running; } && PERF_FORMAT_TOTAL_TIME_RUNNING
* { u64 value;
- * { u64 id; } && PERF_FORMAT_ID
+ * { u64 id; } && PERF_FORMAT_ID
+ * { u64 build_id_faults; } && PERF_FORMAT_BUILD_ID_FAULTS
+ * { u64 lost; } && PERF_FORMAT_LOST
* } cntr[nr];
* } && PERF_FORMAT_GROUP
* };
@@ -315,8 +319,10 @@ enum perf_event_read_format {
PERF_FORMAT_TOTAL_TIME_RUNNING = 1U << 1,
PERF_FORMAT_ID = 1U << 2,
PERF_FORMAT_GROUP = 1U << 3,
+ PERF_FORMAT_BUILD_ID_FAULTS = 1U << 4,
+ PERF_FORMAT_LOST = 1U << 5,

- PERF_FORMAT_MAX = 1U << 4, /* non-ABI */
+ PERF_FORMAT_MAX = 1U << 6, /* non-ABI */
};

#define PERF_ATTR_SIZE_VER0 64 /* sizeof first published struct */
--
2.31.1

2021-06-22 15:41:23

by Jiri Olsa

Subject: [PATCH 09/10] perf report: Display build id fails stats

Adding support to display build id fails in --stats option:

# perf report --stat

Aggregated stats:
TOTAL events: 104
COMM events: 2 ( 1.9%)
....
BUILD_ID fails: 4 (14.3%)

This stat is displayed only for session recorded with --buildid-mmap
and contains HEADER_BUILD_ID_MMAP header feature.

We process all MMAP2 events and in case it does not contain build id
and it should - it's regular file, we count the BUILD_ID fail and
display it.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
tools/perf/util/map.h | 15 +++++++++++++++
2 files changed, 50 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bc5c393021dc..b5c03bcc4395 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -13,6 +13,7 @@
#include "util/annotate.h"
#include "util/color.h"
#include "util/dso.h"
+#include "util/vdso.h"
#include <linux/list.h>
#include <linux/rbtree.h>
#include <linux/err.h>
@@ -100,6 +101,8 @@ struct report {
u64 nr_entries;
u64 queue_size;
u64 total_cycles;
+ u64 buildid_fails;
+ u64 buildid_total;
int socket_filter;
DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
struct branch_type_stat brtype_stat;
@@ -729,10 +732,36 @@ static int count_sample_event(struct perf_tool *tool __maybe_unused,
return 0;
}

+static int count_buildid_fails(struct perf_tool *tool,
+ union perf_event *event,
+ struct perf_sample *sample __maybe_unused,
+ struct machine *machine __maybe_unused)
+{
+ struct report *rep = container_of(tool, struct report, tool);
+ struct perf_record_mmap2 *mmap2 = &event->mmap2;
+
+ /* No build id should be generated */
+ if (!is_buildid_memory(mmap2->filename))
+ return 0;
+
+ rep->buildid_total++;
+
+ /* The build id should be generated, but wasn't - fault. */
+ if (!(mmap2->header.misc & PERF_RECORD_MISC_MMAP_BUILD_ID))
+ rep->buildid_fails++;
+
+ return 0;
+}
+
static void stats_setup(struct report *rep)
{
memset(&rep->tool, 0, sizeof(rep->tool));
rep->tool.sample = count_sample_event;
+
+ if (perf_header__has_feat(&rep->session->header,
+ HEADER_BUILD_ID_MMAP))
+ rep->tool.mmap2 = count_buildid_fails;
+
rep->tool.no_warn = true;
}

@@ -742,6 +771,12 @@ static int stats_print(struct report *rep)

perf_session__fprintf_nr_events(session, stdout, rep->skip_empty);
evlist__fprintf_nr_events(session->evlist, stdout, rep->skip_empty);
+
+ if (rep->buildid_fails) {
+ fprintf(stdout, "%23s: %10" PRIu64 " (%4.1f%%)\n", "BUILD_ID fails",
+ rep->buildid_fails,
+ 100.0 * rep->buildid_fails / rep->buildid_total);
+ }
return 0;
}

diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index d32f5b28c1fb..9b96ebed412d 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -10,6 +10,7 @@
#include <string.h>
#include <stdbool.h>
#include <linux/types.h>
+#include "vdso.h"

struct dso;
struct maps;
@@ -186,4 +187,18 @@ static inline int is_no_dso_memory(const char *filename)
!strncmp(filename, "/SYSV", 5) ||
!strcmp(filename, "[heap]");
}
+
+static inline int is_vsyscall_memory(const char *filename)
+{
+ return !strcmp(filename, "[vsyscall]");
+}
+
+static inline int is_buildid_memory(const char *filename)
+{
+ return !is_anon_memory(filename) &&
+ !is_vdso_map(filename) &&
+ !is_no_dso_memory(filename) &&
+ !is_vsyscall_memory(filename);
+}
+
#endif /* __PERF_MAP_H */
--
2.31.1

2021-06-22 15:41:23

by Jiri Olsa

Subject: [PATCH 10/10] perf inject: Add --buildid-mmap2 option to fix failed build ids

Adding --buildid-mmap2 option that tried to fix failed build ids
in mmap2 events.

Record data with --buildid-mmap option:

# perf record --buildid-mmap ...
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Failed to parse 4 build ids]
[ perf record: Captured and wrote 0.008 MB perf.data ]

Check if there's only build id fault reported:

# perf report --header-only
...
# build id mmap stats: FAULTS 4, LOST 0, NOT FIXED

There is, check the stats:

# perf report --stat

Aggregated stats:
TOTAL events: 104
....
BUILD_ID fails: 4 (14.3%)

Yep, let's fix it:

# perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data

And verify:

# perf report -i perf-fixed.data --stats

Aggregated stats:
TOTAL events: 104
....

Good, let's see how many we fixed:

# perf report --header-only -i perf-fixed.data
...
# build id mmap stats: FAULTS 4, LOST 0, FIXED(4)

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-inject.txt | 3 ++
tools/perf/builtin-inject.c | 45 ++++++++++++++++++++++--
2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-inject.txt b/tools/perf/Documentation/perf-inject.txt
index 91108fe3ad5f..172d6942ca68 100644
--- a/tools/perf/Documentation/perf-inject.txt
+++ b/tools/perf/Documentation/perf-inject.txt
@@ -30,6 +30,9 @@ OPTIONS
--buildid-all:
Inject build-ids of all DSOs into the output stream

+--buildid-mmap2:
+ Resolve failed buildids in MMAP2 events.
+
-v::
--verbose::
Be more verbose.
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 5d6f583e2cd3..5c6c37c581ca 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -40,6 +40,7 @@ struct perf_inject {
struct perf_session *session;
bool build_ids;
bool build_id_all;
+ bool build_id_mmap2;
bool sched_stat;
bool have_auxtrace;
bool strip;
@@ -389,13 +390,43 @@ static int perf_event__repipe_buildid_mmap(struct perf_tool *tool,
return perf_event__repipe(tool, event, sample, machine);
}

+static bool mmap2_fix_buildid(union perf_event *event, struct build_id *bid)
+{
+ struct perf_record_mmap2 *mmap2 = &event->mmap2;
+
+ /*
+ * Filter maps that should have build id, but do not carry one.
+ */
+ if (!is_buildid_memory(mmap2->filename) ||
+ mmap2->header.misc & PERF_RECORD_MISC_MMAP_BUILD_ID)
+ return false;
+
+ return filename__read_build_id(mmap2->filename, bid) > 0 ? true : false;
+}
+
static int perf_event__repipe_mmap2(struct perf_tool *tool,
union perf_event *event,
struct perf_sample *sample,
struct machine *machine)
{
+ struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
+ union perf_event *tmp = NULL;
+ struct build_id bid;
int err;

+ if (inject->build_id_mmap2 && mmap2_fix_buildid(event, &bid)) {
+ tmp = memdup(event, event->header.size);
+ if (!tmp)
+ return -ENOMEM;
+ memcpy(tmp->mmap2.build_id, bid.data, sizeof(bid.data));
+ tmp->header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID;
+ tmp->mmap2.build_id_size = (u8) bid.size;
+ tmp->mmap2.__reserved_1 = 0;
+ tmp->mmap2.__reserved_2 = 0;
+ event = tmp;
+ inject->session->header.env.build_id_mmap.fixed++;
+ }
+
err = perf_event__process_mmap2(tool, event, sample, machine);
perf_event__repipe(tool, event, sample, machine);

@@ -411,6 +442,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
dso__put(dso);
}

+ free(tmp);
return err;
}

@@ -764,7 +796,8 @@ static int __cmd_inject(struct perf_inject *inject)
signal(SIGINT, sig_handler);

if (inject->build_ids || inject->sched_stat ||
- inject->itrace_synth_opts.set || inject->build_id_all) {
+ inject->itrace_synth_opts.set || inject->build_id_all ||
+ inject->build_id_mmap2) {
inject->tool.mmap = perf_event__repipe_mmap;
inject->tool.mmap2 = perf_event__repipe_mmap2;
inject->tool.fork = perf_event__repipe_fork;
@@ -916,13 +949,15 @@ int cmd_inject(int argc, const char **argv)
.mode = PERF_DATA_MODE_READ,
.use_stdio = true,
};
- int ret;
+ int ret = -1;

struct option options[] = {
OPT_BOOLEAN('b', "build-ids", &inject.build_ids,
"Inject build-ids into the output stream"),
OPT_BOOLEAN(0, "buildid-all", &inject.build_id_all,
"Inject build-ids of all DSOs into the output stream"),
+ OPT_BOOLEAN(0, "buildid-mmap2", &inject.build_id_mmap2,
+ "Resolve failed buildids in MMAP2 events"),
OPT_STRING('i', "input", &inject.input_name, "file",
"input file name"),
OPT_STRING('o', "output", &inject.output.path, "file",
@@ -995,6 +1030,12 @@ int cmd_inject(int argc, const char **argv)
if (IS_ERR(inject.session))
return PTR_ERR(inject.session);

+ if (inject.build_id_mmap2 &&
+ !perf_header__has_feat(&inject.session->header, HEADER_BUILD_ID_MMAP)) {
+ pr_err("The data does not have HEADER_BUILD_ID_MMAP, exiting..\n");
+ goto out_delete;
+ }
+
if (zstd_init(&(inject.session->zstd_data), 0) < 0)
pr_warning("Decompression initialization failed.\n");

--
2.31.1

2021-06-22 15:41:23

by Jiri Olsa

Subject: [PATCH 07/10] perf record: Add support to read build id fails

Adding support to read build id fails and lost events count
and display it at the end of the record session, when recording
with --buildid-mmap.

# perf record ...
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Failed to parse 4 build ids]
[ perf record: Captured and wrote 0.008 MB perf.data ]

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/lib/perf/evsel.c | 6 ++
tools/lib/perf/include/perf/evsel.h | 11 ++-
tools/perf/builtin-record.c | 90 +++++++++++++++++++++++
tools/perf/util/evsel.c | 12 +++
tools/perf/util/perf_event_attr_fprintf.c | 3 +-
5 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index 04e8386b3ed4..9d7b2fd49b90 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -248,6 +248,12 @@ int perf_evsel__read_size(struct perf_evsel *evsel)
if (read_format & PERF_FORMAT_ID)
entry += sizeof(u64);

+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ entry += sizeof(u64);
+
+ if (read_format & PERF_FORMAT_LOST)
+ entry += sizeof(u64);
+
if (read_format & PERF_FORMAT_GROUP) {
nr = evsel->nr_members;
size += sizeof(u64);
diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
index 60eae25076d3..294fc5929e1d 100644
--- a/tools/lib/perf/include/perf/evsel.h
+++ b/tools/lib/perf/include/perf/evsel.h
@@ -12,12 +12,21 @@ struct perf_thread_map;

struct perf_counts_values {
union {
+ /* Struct for specific perf interfaces. */
struct {
uint64_t val;
uint64_t ena;
uint64_t run;
};
- uint64_t values[3];
+ /*
+ * Values to store all non-group data:
+ * PERF_FORMAT_TOTAL_TIME_ENABLED
+ * PERF_FORMAT_TOTAL_TIME_RUNNING
+ * PERF_FORMAT_ID
+ * PERF_FORMAT_BUILD_ID_FAULTS
+ * PERF_FORMAT_LOST
+ */
+ uint64_t values[6];
};
};

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bc3dd379eb67..bf3958ce18e3 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -49,6 +49,8 @@
#include "util/clockid.h"
#include "util/pmu-hybrid.h"
#include "util/evlist-hybrid.h"
+#include "util/counts.h"
+#include "util/stat.h"
#include "asm/bug.h"
#include "perf.h"

@@ -1226,6 +1228,90 @@ static void record__init_features(struct record *rec)
perf_header__clear_feat(&session->header, HEADER_STAT);
}

+struct session_stats {
+ u64 build_id_faults;
+ u64 lost;
+};
+
+static int
+evsel__read_session_stats(struct evsel *evsel, struct session_stats *st,
+ int nr_cpus, int nr_threads)
+{
+ u64 read_format = evsel->core.attr.read_format;
+ int idx = 1, idx_faults = 0, idx_lost = 0;
+ int cpu, thread;
+
+ if (read_format & PERF_FORMAT_GROUP)
+ return 0;
+
+ if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
+ idx++;
+ if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
+ idx++;
+ if (read_format & PERF_FORMAT_ID)
+ idx++;
+ if (read_format & PERF_FORMAT_BUILD_ID_FAULTS)
+ idx_faults = idx++;
+ if (read_format & PERF_FORMAT_LOST)
+ idx_lost = idx;
+
+ if (!idx_faults && !idx_lost)
+ return 0;
+
+ for (cpu = 0; cpu < nr_cpus; cpu++) {
+ for (thread = 0; thread < nr_threads; thread++) {
+ struct perf_counts_values count;
+
+ if (perf_evsel__read(&evsel->core, cpu, thread, &count))
+ return -1;
+
+ if (idx_faults)
+ st->build_id_faults += count.values[idx_faults];
+ if (idx_lost)
+ st->lost += count.values[idx_lost];
+ }
+ }
+
+ return 0;
+}
+
+static int
+evlist__read_session_stats(struct evlist *evlist, struct session_stats *st)
+{
+ int nr_threads = perf_thread_map__nr(evlist->core.threads);
+ int nr_cpus = perf_cpu_map__nr(evlist->core.cpus);
+ struct evsel *evsel;
+
+ memset(st, 0, sizeof(*st));
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (evsel__read_session_stats(evsel, st, nr_cpus, nr_threads)) {
+ pr_err("FAILED to read event stats\n");
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+static void read_session_stats(struct record *rec)
+{
+ struct session_stats st;
+
+ if (evlist__read_session_stats(rec->evlist, &st))
+ return;
+
+ if (st.build_id_faults) {
+ fprintf(stderr, "[ perf record: Failed to parse %lu build ids]\n",
+ st.build_id_faults);
+ }
+
+ if (st.lost) {
+ fprintf(stderr, "[ perf record: Lost %lu chunks]\n",
+ st.lost);
+ }
+}
+
static void
record__finish_output(struct record *rec)
{
@@ -1244,6 +1330,10 @@ record__finish_output(struct record *rec)
if (rec->buildid_all)
dsos__hit_all(rec->session);
}
+
+ if (rec->buildid_mmap)
+ read_session_stats(rec);
+
perf_session__write_header(rec->session, rec->evlist, fd, true);

return;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f81ac6962aec..f862cae8874f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1239,6 +1239,18 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
PERF_FORMAT_TOTAL_TIME_RUNNING;
}

+ /*
+ * We skip post processing for build_id, so we need
+ * to read stats via read syscall:
+ * - faults for mmap events
+ * - lost for each event
+ */
+ if (attr->build_id)
+ attr->read_format |= PERF_FORMAT_BUILD_ID_FAULTS;
+
+ if (opts->build_id)
+ attr->read_format |= PERF_FORMAT_LOST;
+
/*
* XXX see the function comment above
*
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 30481825515b..946073024d7a 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -64,7 +64,8 @@ static void __p_read_format(char *buf, size_t size, u64 value)
#define bit_name(n) { PERF_FORMAT_##n, #n }
struct bit_names bits[] = {
bit_name(TOTAL_TIME_ENABLED), bit_name(TOTAL_TIME_RUNNING),
- bit_name(ID), bit_name(GROUP),
+ bit_name(ID), bit_name(GROUP), bit_name(BUILD_ID_FAULTS),
+ bit_name(LOST),
{ .name = NULL, }
};
#undef bit_name
--
2.31.1

2021-06-22 15:41:23

by Jiri Olsa

Subject: [PATCH 08/10] perf record: Add new HEADER_BUILD_ID_MMAP feature

Adding new HEADER_BUILD_ID_MMAP feature to store faulst/lost/fixed
counts for --buildid-mmap setup. We skip post processing for build_id,
so we need to store session stats.

The feature data has following format:

struct {
u32 version;
u64 faults;
u64 lost;
u64 fixed;
};

The version is set to 1.
The faults has the value of faulted build id retrievals for the session.
The lost has the value of faulted lost events for the session.
The fixed has the value of fixed build ids by post-processing.

The perf report --header-only display for when fixes is 0:

# build id mmap stats: FAULTS 4, LOST 0, NOT FIXED

If fixed is defined:

# build id mmap stats: FAULTS 4, LOST 0, FIXED(4)

Signed-off-by: Jiri Olsa <[email protected]>
---
.../Documentation/perf.data-file-format.txt | 19 +++++
tools/perf/builtin-record.c | 7 ++
tools/perf/util/env.h | 6 ++
tools/perf/util/header.c | 80 +++++++++++++++++++
tools/perf/util/header.h | 1 +
5 files changed, 113 insertions(+)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index e6ff8c898ada..223fea2ba662 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -438,6 +438,25 @@ struct {
other bits are reserved and should ignored for now
HEADER_FEAT_BITS = 256,

+ HEADER_BUILD_ID_MMAP = 32,
+
+ It contains stats values for session with --buildid-mmap option.
+
+struct {
+ u32 version;
+ u64 faults;
+ u64 lost;
+ u64 fixed;
+};
+
+ The version is set to 1.
+ The faults has the value of faulted build id retrievals for the session.
+ The lost has the value of faulted lost events for the session.
+ The fixed has the value of fixed build ids by post-processing.
+
+ other bits are reserved and should ignored for now
+ HEADER_FEAT_BITS = 256,
+
Attributes

This is an array of perf_event_attrs, each attr_size bytes long, which defines
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bf3958ce18e3..cae1a38a9e2a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1221,6 +1221,9 @@ static void record__init_features(struct record *rec)
if (!rec->opts.use_clockid)
perf_header__clear_feat(&session->header, HEADER_CLOCK_DATA);

+ if (!rec->buildid_mmap)
+ perf_header__clear_feat(&session->header, HEADER_BUILD_ID_MMAP);
+
perf_header__clear_feat(&session->header, HEADER_DIR_FORMAT);
if (!record__comp_enabled(rec))
perf_header__clear_feat(&session->header, HEADER_COMPRESSED);
@@ -1296,6 +1299,7 @@ evlist__read_session_stats(struct evlist *evlist, struct session_stats *st)

static void read_session_stats(struct record *rec)
{
+ struct perf_session *session = rec->session;
struct session_stats st;

if (evlist__read_session_stats(rec->evlist, &st))
@@ -1310,6 +1314,9 @@ static void read_session_stats(struct record *rec)
fprintf(stderr, "[ perf record: Lost %lu chunks]\n",
st.lost);
}
+
+ session->header.env.build_id_mmap.faults = st.build_id_faults;
+ session->header.env.build_id_mmap.lost = st.lost;
}

static void
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 6824a7423a2d..8d45c774ad75 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -128,6 +128,12 @@ struct perf_env {
*/
bool enabled;
} clock;
+
+ struct {
+ u64 faults;
+ u64 lost;
+ u64 fixed;
+ } build_id_mmap;
};

enum perf_compress_type {
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0158d2945bab..ac4f62170107 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1528,6 +1528,39 @@ static int write_hybrid_cpu_pmu_caps(struct feat_fd *ff,
return 0;
}

+static int write_build_id_mmap(struct feat_fd *ff,
+ struct evlist *evlist __maybe_unused)
+{
+ u64 data64;
+ u32 data32;
+ int ret;
+
+ /* version */
+ data32 = 1;
+
+ ret = do_write(ff, &data32, sizeof(data32));
+ if (ret < 0)
+ return ret;
+
+ /* faults */
+ data64 = ff->ph->env.build_id_mmap.faults;
+
+ ret = do_write(ff, &data64, sizeof(data64));
+ if (ret < 0)
+ return ret;
+
+ /* lost */
+ data64 = ff->ph->env.build_id_mmap.lost;
+
+ ret = do_write(ff, &data64, sizeof(data64));
+ if (ret < 0)
+ return ret;
+
+ /* fixed */
+ data64 = ff->ph->env.build_id_mmap.fixed;
+ return do_write(ff, &data64, sizeof(data64));
+}
+
static void print_hostname(struct feat_fd *ff, FILE *fp)
{
fprintf(fp, "# hostname : %s\n", ff->ph->env.hostname);
@@ -2048,6 +2081,19 @@ static void print_hybrid_cpu_pmu_caps(struct feat_fd *ff, FILE *fp)
}
}

+static void print_build_id_mmap(struct feat_fd *ff, FILE *fp)
+{
+ fprintf(fp, "# build id mmap stats: FAULTS %" PRIu64 ", LOST %" PRIu64 ",%s FIXED",
+ ff->ph->env.build_id_mmap.faults,
+ ff->ph->env.build_id_mmap.lost,
+ ff->ph->env.build_id_mmap.fixed ? "" : " NOT");
+
+ if (ff->ph->env.build_id_mmap.fixed)
+ fprintf(fp, "(%" PRIu64 ")", ff->ph->env.build_id_mmap.fixed);
+
+ fprintf(fp, "\n");
+}
+
static void print_pmu_mappings(struct feat_fd *ff, FILE *fp)
{
const char *delimiter = "# pmu mappings: ";
@@ -3265,6 +3311,39 @@ static int process_hybrid_cpu_pmu_caps(struct feat_fd *ff,
return ret;
}

+static int process_build_id_mmap(struct feat_fd *ff,
+ void *data __maybe_unused)
+{
+ u32 data32;
+ u64 data64;
+
+ /* version */
+ if (do_read_u32(ff, &data32))
+ return -1;
+
+ if (data32 != 1)
+ return -1;
+
+ /* faults */
+ if (do_read_u64(ff, &data64))
+ return -1;
+
+ ff->ph->env.build_id_mmap.faults = data64;
+
+ /* lost */
+ if (do_read_u64(ff, &data64))
+ return -1;
+
+ ff->ph->env.build_id_mmap.lost = data64;
+
+ /* fixed */
+ if (do_read_u64(ff, &data64))
+ return -1;
+
+ ff->ph->env.build_id_mmap.fixed = data64;
+ return 0;
+}
+
#define FEAT_OPR(n, func, __full_only) \
[HEADER_##n] = { \
.name = __stringify(n), \
@@ -3328,6 +3407,7 @@ const struct perf_header_feature_ops feat_ops[HEADER_LAST_FEATURE] = {
FEAT_OPR(CLOCK_DATA, clock_data, false),
FEAT_OPN(HYBRID_TOPOLOGY, hybrid_topology, true),
FEAT_OPR(HYBRID_CPU_PMU_CAPS, hybrid_cpu_pmu_caps, false),
+ FEAT_OPR(BUILD_ID_MMAP, build_id_mmap, false),
};

struct header_print_data {
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index ae6b1cf19a7d..a9fe37bb03cc 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -47,6 +47,7 @@ enum {
HEADER_CLOCK_DATA,
HEADER_HYBRID_TOPOLOGY,
HEADER_HYBRID_CPU_PMU_CAPS,
+ HEADER_BUILD_ID_MMAP,
HEADER_LAST_FEATURE,
HEADER_FEAT_BITS = 256,
};
--
2.31.1

2021-06-22 15:42:11

by Jiri Olsa

Subject: [PATCH 06/10] libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read

The struct perf_counts_values can hold only first three
read_format bits. We can't allow PERF_FORMAT_GROUP,
otherwise we get segfault.

Signed-off-by: Jiri Olsa <[email protected]>
---
tools/lib/perf/evsel.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index bd8c2f19ef74..04e8386b3ed4 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -260,8 +260,12 @@ int perf_evsel__read_size(struct perf_evsel *evsel)
int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
struct perf_counts_values *count)
{
+ u64 read_format = evsel->attr.read_format;
size_t size = perf_evsel__read_size(evsel);

+ if (read_format & PERF_FORMAT_GROUP)
+ return -EINVAL;
+
memset(count, 0, sizeof(*count));

if (FD(evsel, cpu, thread) < 0)
--
2.31.1

2021-06-22 17:40:38

by Arnaldo Carvalho de Melo

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> hi,
> this *RFC* patchset adds support to detect faults during
> mmap2's build id parsing and a way to fix such maps in
> generated perf.data.
>
> It adds support to record build id faults count for session
> and store it in perf.data and perf inject support to find
> these maps and reads build ids for them in user space.

> It's probably best explained by the workflow:
>
> Record data with --buildid-mmap option:
>
> # perf record --buildid-mmap ...
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Failed to parse 4 build ids]
> [ perf record: Captured and wrote 0.008 MB perf.data ]
>
> Check if there's any build id fault reported:
>
> # perf report --header-only
> ...
> # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
>
> There is, check the stats:
>
> # perf report --stat
>
> Aggregated stats:
> TOTAL events: 104
> ....
> BUILD_ID fails: 4 (14.3%)
>
> Yep, let's fix it:
>
> # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data

Can we make it possible to automate this with --fixup-buildids or a
perfconfig 'record' knob?

This would entail requesting that build-ids that _fail_ be sent to the
side-band thread we have in 'perf record', this way we wouldn't have to
traverse the whole perf.data file, be it with 'perf-record' at the end
of a session with faulty build ids, or in a similar fashion using 'perf
inject' as you suggest.

I even think that we can have all these modes and let the user to decide
how important is this for them and how convenient they want the whole
process to be.

- Arnaldo

> And verify:
>
> # perf report -i perf-fixed.data --stats
>
> Aggregated stats:
> TOTAL events: 104
> ....
>
> Good, let's see how many we fixed:
>
> # perf report --header-only -i perf-fixed.data
> ...
> # build id mmap stats: FAULTS 4, LOST 0, FIXED(4)
>
>
> I don't have a good way to test it, just by artificially
> adding the faults in kernel code, but Ian and Namhyung
> might have setup that could generate that.. would be great
> to have a perf test for this.
>
> Also available in here:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> perf/buildid_stats
>
> thoughts?
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (10):
> perf: Track build id faults for mmap2 event
> perf: Move build_id_parse to check only regular files
> perf: Add new read_format bit to read build id faults
> perf: Add new read_format bit to read lost events
> tools: Sync perf_event.h uapi
> libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read
> perf record: Add support to read build id fails
> perf record: Add new HEADER_BUILD_ID_MMAP feature
> perf report: Display build id fails stats
> perf inject: Add --buildid-mmap2 option to fix failed build ids
>
> include/linux/perf_event.h | 2 ++
> include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> kernel/events/ring_buffer.c | 3 +++
> tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> tools/lib/perf/evsel.c | 10 ++++++++++
> tools/lib/perf/include/perf/evsel.h | 11 ++++++++++-
> tools/perf/Documentation/perf-inject.txt | 3 +++
> tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++++
> tools/perf/builtin-inject.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> tools/perf/builtin-record.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
> tools/perf/util/env.h | 6 ++++++
> tools/perf/util/evsel.c | 12 ++++++++++++
> tools/perf/util/header.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/header.h | 1 +
> tools/perf/util/map.h | 15 +++++++++++++++
> tools/perf/util/perf_event_attr_fprintf.c | 3 ++-
> 18 files changed, 407 insertions(+), 24 deletions(-)
>

--

- Arnaldo

2021-06-22 17:49:27

by Ian Rogers

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > hi,
> > this *RFC* patchset adds support to detect faults during
> > mmap2's build id parsing and a way to fix such maps in
> > generated perf.data.
> >
> > It adds support to record build id faults count for session
> > and store it in perf.data and perf inject support to find
> > these maps and reads build ids for them in user space.
>
> > It's probably best explained by the workflow:
> >
> > Record data with --buildid-mmap option:
> >
> > # perf record --buildid-mmap ...
> > ...
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Failed to parse 4 build ids]
> > [ perf record: Captured and wrote 0.008 MB perf.data ]
> >
> > Check if there's any build id fault reported:
> >
> > # perf report --header-only
> > ...
> > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> >
> > There is, check the stats:
> >
> > # perf report --stat
> >
> > Aggregated stats:
> > TOTAL events: 104
> > ....
> > BUILD_ID fails: 4 (14.3%)
> >
> > Yep, let's fix it:
> >
> > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
>
> Can we make it possible to automate this with --fixup-buildids or a
> perfconfig 'record' knob?
>
> This would entail requesting that build-ids that _fail_ be sent to the
> side-band thread we have in 'perf record', this way we wouldn't have to
> traverse the whole perf.data file, be it with 'perf-record' at the end
> of a session with faulty build ids, or in a similar fashion using 'perf
> inject' as you suggest.
>
> I even think that we can have all these modes and let the user to decide
> how important is this for them and how convenient they want the whole
> process to be.

Firstly thanks for the patches! To Arnaldo's sideband idea, I wonder
if we have a thread doing sideband buildid generation whether the same
thread or threads could also be doing the synthesis job. Perhaps such
work could be postponed until when the session closes, like with tail
synthesis. It's a particular shame with tail synthesis that we
synthesize mmap events for processes with no samples.

Thanks,
Ian

> - Arnaldo
>
> > And verify:
> >
> > # perf report -i perf-fixed.data --stats
> >
> > Aggregated stats:
> > TOTAL events: 104
> > ....
> >
> > Good, let's see how many we fixed:
> >
> > # perf report --header-only -i perf-fixed.data
> > ...
> > # build id mmap stats: FAULTS 4, LOST 0, FIXED(4)
> >
> >
> > I don't have a good way to test it, just by artificially
> > adding the faults in kernel code, but Ian and Namhyung
> > might have setup that could generate that.. would be great
> > to have a perf test for this.
> >
> > Also available in here:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > perf/buildid_stats
> >
> > thoughts?
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Jiri Olsa (10):
> > perf: Track build id faults for mmap2 event
> > perf: Move build_id_parse to check only regular files
> > perf: Add new read_format bit to read build id faults
> > perf: Add new read_format bit to read lost events
> > tools: Sync perf_event.h uapi
> > libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read
> > perf record: Add support to read build id fails
> > perf record: Add new HEADER_BUILD_ID_MMAP feature
> > perf report: Display build id fails stats
> > perf inject: Add --buildid-mmap2 option to fix failed build ids
> >
> > include/linux/perf_event.h | 2 ++
> > include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> > kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> > kernel/events/ring_buffer.c | 3 +++
> > tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> > tools/lib/perf/evsel.c | 10 ++++++++++
> > tools/lib/perf/include/perf/evsel.h | 11 ++++++++++-
> > tools/perf/Documentation/perf-inject.txt | 3 +++
> > tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++++
> > tools/perf/builtin-inject.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> > tools/perf/builtin-record.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
> > tools/perf/util/env.h | 6 ++++++
> > tools/perf/util/evsel.c | 12 ++++++++++++
> > tools/perf/util/header.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/header.h | 1 +
> > tools/perf/util/map.h | 15 +++++++++++++++
> > tools/perf/util/perf_event_attr_fprintf.c | 3 ++-
> > 18 files changed, 407 insertions(+), 24 deletions(-)
> >
>
> --
>
> - Arnaldo

2021-06-22 18:15:35

by Arnaldo Carvalho de Melo

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > hi,
> > > this *RFC* patchset adds support to detect faults during
> > > mmap2's build id parsing and a way to fix such maps in
> > > generated perf.data.
> > >
> > > It adds support to record build id faults count for session
> > > and store it in perf.data and perf inject support to find
> > > these maps and reads build ids for them in user space.
> >
> > > It's probably best explained by the workflow:
> > >
> > > Record data with --buildid-mmap option:
> > >
> > > # perf record --buildid-mmap ...
> > > ...
> > > [ perf record: Woken up 1 times to write data ]
> > > [ perf record: Failed to parse 4 build ids]
> > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > >
> > > Check if there's any build id fault reported:
> > >
> > > # perf report --header-only
> > > ...
> > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > >
> > > There is, check the stats:
> > >
> > > # perf report --stat
> > >
> > > Aggregated stats:
> > > TOTAL events: 104
> > > ....
> > > BUILD_ID fails: 4 (14.3%)
> > >
> > > Yep, let's fix it:
> > >
> > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> >
> > Can we make it possible to automate this with --fixup-buildids or a
> > perfconfig 'record' knob?
> >
> > This would entail requesting that build-ids that _fail_ be sent to the
> > side-band thread we have in 'perf record', this way we wouldn't have to
> > traverse the whole perf.data file, be it with 'perf-record' at the end
> > of a session with faulty build ids, or in a similar fashion using 'perf
> > inject' as you suggest.
> >
> > I even think that we can have all these modes and let the user to decide
> > how important is this for them and how convenient they want the whole
> > process to be.
>
> Firstly thanks for the patches! To Arnaldo's sideband idea, I wonder
> if we have a thread doing sideband buildid generation whether the same
> thread or threads could also be doing the synthesis job. Perhaps such
> work could be postponed until when the session closes, like with tail

I didn't suggest synthesizing the failed build-ids in the sideband
thread, just receiving the MMAP2 records for the build-ids that
faulted.

It may be interesting to do it right away, to avoid building up a
potentially large number of entries to do at the end, but if this is
something uncommon, with just a few entries, then leaving it for after
the workload finishes may be a good idea.

Or perhaps this needs to be a knob, since for long running sessions such
as with 'perf daemon' the "workload" may never end, so we better flush
these things as the files where we'll get it from may go away.

> synthesis. It's a particular shame with tail synthesis that we
> synthesize mmap events for processes with no samples.

Sure, but it is also very costly to process a potentially large
perf.data file for looking at what MMAPs have samples. That is the
raison d'?tre for PERF_RECORD_MMAP2 to carry build-ids :-)

I.e. there are pros and cons for tail synthesis, for looking at all
samples to generate only build-ids for MMAPs with hits, for synthesizing
it in the sideband thread immediately, for leaving this generation to
be done at the end by traversing the list of MMAP records without
build-ids, etc.

- Arnaldo

> Thanks,
> Ian
>
> > - Arnaldo
> >
> > > And verify:
> > >
> > > # perf report -i perf-fixed.data --stats
> > >
> > > Aggregated stats:
> > > TOTAL events: 104
> > > ....
> > >
> > > Good, let's see how many we fixed:
> > >
> > > # perf report --header-only -i perf-fixed.data
> > > ...
> > > # build id mmap stats: FAULTS 4, LOST 0, FIXED(4)
> > >
> > >
> > > I don't have a good way to test it, just by artificially
> > > adding the faults in kernel code, but Ian and Namhyung
> > > might have setup that could generate that.. would be great
> > > to have a perf test for this.
> > >
> > > Also available in here:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > > perf/buildid_stats
> > >
> > > thoughts?
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > ---
> > > Jiri Olsa (10):
> > > perf: Track build id faults for mmap2 event
> > > perf: Move build_id_parse to check only regular files
> > > perf: Add new read_format bit to read build id faults
> > > perf: Add new read_format bit to read lost events
> > > tools: Sync perf_event.h uapi
> > > libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read
> > > perf record: Add support to read build id fails
> > > perf record: Add new HEADER_BUILD_ID_MMAP feature
> > > perf report: Display build id fails stats
> > > perf inject: Add --buildid-mmap2 option to fix failed build ids
> > >
> > > include/linux/perf_event.h | 2 ++
> > > include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> > > kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> > > kernel/events/ring_buffer.c | 3 +++
> > > tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> > > tools/lib/perf/evsel.c | 10 ++++++++++
> > > tools/lib/perf/include/perf/evsel.h | 11 ++++++++++-
> > > tools/perf/Documentation/perf-inject.txt | 3 +++
> > > tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++++
> > > tools/perf/builtin-inject.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> > > tools/perf/builtin-record.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
> > > tools/perf/util/env.h | 6 ++++++
> > > tools/perf/util/evsel.c | 12 ++++++++++++
> > > tools/perf/util/header.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > tools/perf/util/header.h | 1 +
> > > tools/perf/util/map.h | 15 +++++++++++++++
> > > tools/perf/util/perf_event_attr_fprintf.c | 3 ++-
> > > 18 files changed, 407 insertions(+), 24 deletions(-)
> > >
> >
> > --
> >
> > - Arnaldo

--

- Arnaldo

2021-06-22 21:22:49

by Jiri Olsa

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> > >
> > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > hi,
> > > > this *RFC* patchset adds support to detect faults during
> > > > mmap2's build id parsing and a way to fix such maps in
> > > > generated perf.data.
> > > >
> > > > It adds support to record build id faults count for session
> > > > and store it in perf.data and perf inject support to find
> > > > these maps and reads build ids for them in user space.
> > >
> > > > It's probably best explained by the workflow:
> > > >
> > > > Record data with --buildid-mmap option:
> > > >
> > > > # perf record --buildid-mmap ...
> > > > ...
> > > > [ perf record: Woken up 1 times to write data ]
> > > > [ perf record: Failed to parse 4 build ids]
> > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > >
> > > > Check if there's any build id fault reported:
> > > >
> > > > # perf report --header-only
> > > > ...
> > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > >
> > > > There is, check the stats:
> > > >
> > > > # perf report --stat
> > > >
> > > > Aggregated stats:
> > > > TOTAL events: 104
> > > > ....
> > > > BUILD_ID fails: 4 (14.3%)
> > > >
> > > > Yep, let's fix it:
> > > >
> > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > >
> > > Can we make it possible to automate this with --fixup-buildids or a
> > > perfconfig 'record' knob?
> > >
> > > This would entail requesting that build-ids that _fail_ be sent to the
> > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > inject' as you suggest.
> > >
> > > I even think that we can have all these modes and let the user to decide
> > > how important is this for them and how convenient they want the whole
> > > process to be.

right, that might be good to decide first.. because as I said,
I never hit faulted build id, so it probably needs the special
setup you guys are using.. could you try on your setup and check
how many faulted build ids you see?

thanks,
jirka

2021-06-23 19:51:31

by Namhyung Kim

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

Hi Jiri,

Thanks for your work!

On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <[email protected]> wrote:
>
> On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > <[email protected]> wrote:
> > > >
> > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > hi,
> > > > > this *RFC* patchset adds support to detect faults during
> > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > generated perf.data.
> > > > >
> > > > > It adds support to record build id faults count for session
> > > > > and store it in perf.data and perf inject support to find
> > > > > these maps and reads build ids for them in user space.
> > > >
> > > > > It's probably best explained by the workflow:
> > > > >
> > > > > Record data with --buildid-mmap option:
> > > > >
> > > > > # perf record --buildid-mmap ...
> > > > > ...
> > > > > [ perf record: Woken up 1 times to write data ]
> > > > > [ perf record: Failed to parse 4 build ids]
> > > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > >
> > > > > Check if there's any build id fault reported:
> > > > >
> > > > > # perf report --header-only
> > > > > ...
> > > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > >
> > > > > There is, check the stats:
> > > > >
> > > > > # perf report --stat
> > > > >
> > > > > Aggregated stats:
> > > > > TOTAL events: 104
> > > > > ....
> > > > > BUILD_ID fails: 4 (14.3%)
> > > > >
> > > > > Yep, let's fix it:
> > > > >
> > > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > >
> > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > perfconfig 'record' knob?
> > > >
> > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > inject' as you suggest.
> > > >
> > > > I even think that we can have all these modes and let the user to decide
> > > > how important is this for them and how convenient they want the whole
> > > > process to be.
>
> right, that might be good to decide first.. because as I said,
> I never hit faulted build id, so it probably needs the special
> setup you guys are using.. could you try on your setup and check
> how many faulted build ids you see?

Did you check data mmaps? It might be easy to get faults
from data files and we don't know if it's an ELF or not
before reading the ELF header in the first page.

I'm not sure if we can limit it to exec mappings, there might
be data-only DSOs and we may want to symbolize them too.

Thanks,
Namhyung

2021-06-23 20:17:18

by Namhyung Kim

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Tue, Jun 22, 2021 at 8:39 AM Jiri Olsa <[email protected]> wrote:
>
> hi,
> this *RFC* patchset adds support to detect faults during
> mmap2's build id parsing and a way to fix such maps in
> generated perf.data.
>
> It adds support to record build id faults count for session
> and store it in perf.data and perf inject support to find
> these maps and reads build ids for them in user space.
>
> It's probably best explained by the workflow:
>
> Record data with --buildid-mmap option:
>
> # perf record --buildid-mmap ...
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Failed to parse 4 build ids]
> [ perf record: Captured and wrote 0.008 MB perf.data ]
>
> Check if there's any build id fault reported:
>
> # perf report --header-only
> ...
> # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
>
> There is, check the stats:
>
> # perf report --stat
>
> Aggregated stats:
> TOTAL events: 104
> ....
> BUILD_ID fails: 4 (14.3%)
>
> Yep, let's fix it:

Depending on the failure, it might not need to be fixed.
Say, one process mmapped a file A and succeeded.
And then another process mmaped the same file A,
but it failed to get a build-id (mmap itself was ok).
And vice versa is fine too.

So even if it sees failures, we didn't lose anything.

>
> # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data

Not sure this is really needed since `perf inject -j`
can add BUILD_ID records without fixing MMAP2.

>
> And verify:
>
> # perf report -i perf-fixed.data --stats
>
> Aggregated stats:
> TOTAL events: 104
> ....
>
> Good, let's see how many we fixed:
>
> # perf report --header-only -i perf-fixed.data
> ...
> # build id mmap stats: FAULTS 4, LOST 0, FIXED(4)
>
>
> I don't have a good way to test it, just by artificially
> adding the faults in kernel code, but Ian and Namhyung
> might have setup that could generate that.. would be great
> to have a perf test for this.
>
> Also available in here:
> git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> perf/buildid_stats
>
> thoughts?
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (10):
> perf: Track build id faults for mmap2 event
> perf: Move build_id_parse to check only regular files
> perf: Add new read_format bit to read build id faults
> perf: Add new read_format bit to read lost events
> tools: Sync perf_event.h uapi
> libperf: Do not allow PERF_FORMAT_GROUP in perf_evsel__read
> perf record: Add support to read build id fails
> perf record: Add new HEADER_BUILD_ID_MMAP feature
> perf report: Display build id fails stats
> perf inject: Add --buildid-mmap2 option to fix failed build ids
>
> include/linux/perf_event.h | 2 ++
> include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> kernel/events/core.c | 49 +++++++++++++++++++++++++++++++++++++++++++------
> kernel/events/ring_buffer.c | 3 +++
> tools/include/uapi/linux/perf_event.h | 20 +++++++++++++-------
> tools/lib/perf/evsel.c | 10 ++++++++++
> tools/lib/perf/include/perf/evsel.h | 11 ++++++++++-
> tools/perf/Documentation/perf-inject.txt | 3 +++
> tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++++
> tools/perf/builtin-inject.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> tools/perf/builtin-record.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/builtin-report.c | 35 +++++++++++++++++++++++++++++++++++
> tools/perf/util/env.h | 6 ++++++
> tools/perf/util/evsel.c | 12 ++++++++++++
> tools/perf/util/header.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/header.h | 1 +
> tools/perf/util/map.h | 15 +++++++++++++++
> tools/perf/util/perf_event_attr_fprintf.c | 3 ++-
> 18 files changed, 407 insertions(+), 24 deletions(-)
>

2021-06-24 11:46:15

by Michael Petlan

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Wed, 23 Jun 2021, Namhyung Kim wrote:
> Hi Jiri,
>
> Thanks for your work!
>
> On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <[email protected]> wrote:
> >
> > On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > > <[email protected]> wrote:
> > > > >
> > > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > > hi,
> > > > > > this *RFC* patchset adds support to detect faults during
> > > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > > generated perf.data.
> > > > > >
> > > > > > It adds support to record build id faults count for session
> > > > > > and store it in perf.data and perf inject support to find
> > > > > > these maps and reads build ids for them in user space.
> > > > >
> > > > > > It's probably best explained by the workflow:
> > > > > >
> > > > > > Record data with --buildid-mmap option:
> > > > > >
> > > > > > # perf record --buildid-mmap ...
> > > > > > ...
> > > > > > [ perf record: Woken up 1 times to write data ]
> > > > > > [ perf record: Failed to parse 4 build ids]
> > > > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > > >
> > > > > > Check if there's any build id fault reported:
> > > > > >
> > > > > > # perf report --header-only
> > > > > > ...
> > > > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > > >
> > > > > > There is, check the stats:
> > > > > >
> > > > > > # perf report --stat
> > > > > >
> > > > > > Aggregated stats:
> > > > > > TOTAL events: 104
> > > > > > ....
> > > > > > BUILD_ID fails: 4 (14.3%)
> > > > > >
> > > > > > Yep, let's fix it:
> > > > > >
> > > > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > > >
> > > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > > perfconfig 'record' knob?
> > > > >
> > > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > > inject' as you suggest.
> > > > >
> > > > > I even think that we can have all these modes and let the user to decide
> > > > > how important is this for them and how convenient they want the whole
> > > > > process to be.
> >
> > right, that might be good to decide first.. because as I said,
> > I never hit faulted build id, so it probably needs the special
> > setup you guys are using.. could you try on your setup and check
> > how many faulted build ids you see?
>
> Did you check data mmaps? It might be easy to get faults
> from data files and we don't know if it's an ELF or not
> before reading the ELF header in the first page.

Hi. Long ago, I have noticed samples pointing to purely data files,
such as if the program execution was sampled just in the middle of
them. However, these files couldn't certainly contain any executable
code... It was quite hard to reproduce this.

Maybe what Namhyung says might have been a culprit for it? Just an
idea...

Michael
>
> I'm not sure if we can limit it to exec mappings, there might
> be data-only DSOs and we may want to symbolize them too.
>
> Thanks,
> Namhyung
>
>

2021-06-27 17:28:58

by Jiri Olsa

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Wed, Jun 23, 2021 at 12:48:30PM -0700, Namhyung Kim wrote:
> Hi Jiri,
>
> Thanks for your work!
>
> On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <[email protected]> wrote:
> >
> > On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > > <[email protected]> wrote:
> > > > >
> > > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > > hi,
> > > > > > this *RFC* patchset adds support to detect faults during
> > > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > > generated perf.data.
> > > > > >
> > > > > > It adds support to record build id faults count for session
> > > > > > and store it in perf.data and perf inject support to find
> > > > > > these maps and reads build ids for them in user space.
> > > > >
> > > > > > It's probably best explained by the workflow:
> > > > > >
> > > > > > Record data with --buildid-mmap option:
> > > > > >
> > > > > > # perf record --buildid-mmap ...
> > > > > > ...
> > > > > > [ perf record: Woken up 1 times to write data ]
> > > > > > [ perf record: Failed to parse 4 build ids]
> > > > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > > >
> > > > > > Check if there's any build id fault reported:
> > > > > >
> > > > > > # perf report --header-only
> > > > > > ...
> > > > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > > >
> > > > > > There is, check the stats:
> > > > > >
> > > > > > # perf report --stat
> > > > > >
> > > > > > Aggregated stats:
> > > > > > TOTAL events: 104
> > > > > > ....
> > > > > > BUILD_ID fails: 4 (14.3%)
> > > > > >
> > > > > > Yep, let's fix it:
> > > > > >
> > > > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > > >
> > > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > > perfconfig 'record' knob?
> > > > >
> > > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > > inject' as you suggest.
> > > > >
> > > > > I even think that we can have all these modes and let the user to decide
> > > > > how important is this for them and how convenient they want the whole
> > > > > process to be.
> >
> > right, that might be good to decide first.. because as I said,
> > I never hit faulted build id, so it probably needs the special
> > setup you guys are using.. could you try on your setup and check
> > how many faulted build ids you see?
>
> Did you check data mmaps? It might be easy to get faults
> from data files and we don't know if it's an ELF or not
> before reading the ELF header in the first page.

well, AFAICS the mmap event is sent right after the elf file
is loaded, so it does not have a chance to be swapped out

>
> I'm not sure if we can limit it to exec mappings, there might
> be data-only DSOs and we may want to symbolize them too.

hum, I haven't checked the data-only DSO, which we'd load
manually, not the loader.. will check

thanks,
jirka

2021-06-27 17:29:38

by Jiri Olsa

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Thu, Jun 24, 2021 at 01:44:43PM +0200, Michael Petlan wrote:
> On Wed, 23 Jun 2021, Namhyung Kim wrote:
> > Hi Jiri,
> >
> > Thanks for your work!
> >
> > On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <[email protected]> wrote:
> > >
> > > On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > > > hi,
> > > > > > > this *RFC* patchset adds support to detect faults during
> > > > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > > > generated perf.data.
> > > > > > >
> > > > > > > It adds support to record build id faults count for session
> > > > > > > and store it in perf.data and perf inject support to find
> > > > > > > these maps and reads build ids for them in user space.
> > > > > >
> > > > > > > It's probably best explained by the workflow:
> > > > > > >
> > > > > > > Record data with --buildid-mmap option:
> > > > > > >
> > > > > > > # perf record --buildid-mmap ...
> > > > > > > ...
> > > > > > > [ perf record: Woken up 1 times to write data ]
> > > > > > > [ perf record: Failed to parse 4 build ids]
> > > > > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > > > >
> > > > > > > Check if there's any build id fault reported:
> > > > > > >
> > > > > > > # perf report --header-only
> > > > > > > ...
> > > > > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > > > >
> > > > > > > There is, check the stats:
> > > > > > >
> > > > > > > # perf report --stat
> > > > > > >
> > > > > > > Aggregated stats:
> > > > > > > TOTAL events: 104
> > > > > > > ....
> > > > > > > BUILD_ID fails: 4 (14.3%)
> > > > > > >
> > > > > > > Yep, let's fix it:
> > > > > > >
> > > > > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > > > >
> > > > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > > > perfconfig 'record' knob?
> > > > > >
> > > > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > > > inject' as you suggest.
> > > > > >
> > > > > > I even think that we can have all these modes and let the user to decide
> > > > > > how important is this for them and how convenient they want the whole
> > > > > > process to be.
> > >
> > > right, that might be good to decide first.. because as I said,
> > > I never hit faulted build id, so it probably needs the special
> > > setup you guys are using.. could you try on your setup and check
> > > how many faulted build ids you see?
> >
> > Did you check data mmaps? It might be easy to get faults
> > from data files and we don't know if it's an ELF or not
> > before reading the ELF header in the first page.
>
> Hi. Long ago, I have noticed samples pointing to purely data files,
> such as if the program execution was sampled just in the middle of
> them. However, these files couldn't certainly contain any executable
> code... It was quite hard to reproduce this.
>
> Maybe what Namhyung says might have been a culprit for it? Just an
> idea...

yea, that sounds bad.. I guess you can no longer reproduce?

thanks,
jirka

2021-06-28 03:42:07

by Namhyung Kim

Subject: Re: [RFC 00/10] perf: Add build id parsing fault detection/fix

On Sun, Jun 27, 2021 at 10:25 AM Jiri Olsa <[email protected]> wrote:
>
> On Wed, Jun 23, 2021 at 12:48:30PM -0700, Namhyung Kim wrote:
> > Hi Jiri,
> >
> > Thanks for your work!
> >
> > On Tue, Jun 22, 2021 at 2:19 PM Jiri Olsa <[email protected]> wrote:
> > >
> > > On Tue, Jun 22, 2021 at 03:14:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Tue, Jun 22, 2021 at 10:47:54AM -0700, Ian Rogers escreveu:
> > > > > On Tue, Jun 22, 2021 at 10:39 AM Arnaldo Carvalho de Melo
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > Em Tue, Jun 22, 2021 at 05:39:08PM +0200, Jiri Olsa escreveu:
> > > > > > > hi,
> > > > > > > this *RFC* patchset adds support to detect faults during
> > > > > > > mmap2's build id parsing and a way to fix such maps in
> > > > > > > generated perf.data.
> > > > > > >
> > > > > > > It adds support to record build id faults count for session
> > > > > > > and store it in perf.data and perf inject support to find
> > > > > > > these maps and reads build ids for them in user space.
> > > > > >
> > > > > > > It's probably best explained by the workflow:
> > > > > > >
> > > > > > > Record data with --buildid-mmap option:
> > > > > > >
> > > > > > > # perf record --buildid-mmap ...
> > > > > > > ...
> > > > > > > [ perf record: Woken up 1 times to write data ]
> > > > > > > [ perf record: Failed to parse 4 build ids]
> > > > > > > [ perf record: Captured and wrote 0.008 MB perf.data ]
> > > > > > >
> > > > > > > Check if there's any build id fault reported:
> > > > > > >
> > > > > > > # perf report --header-only
> > > > > > > ...
> > > > > > > # build id mmap stats: FAULTS 4, LOST 0, NOT FIXED
> > > > > > >
> > > > > > > There is, check the stats:
> > > > > > >
> > > > > > > # perf report --stat
> > > > > > >
> > > > > > > Aggregated stats:
> > > > > > > TOTAL events: 104
> > > > > > > ....
> > > > > > > BUILD_ID fails: 4 (14.3%)
> > > > > > >
> > > > > > > Yep, let's fix it:
> > > > > > >
> > > > > > > # perf inject --buildid-mmap2 -i perf.data -o perf-fixed.data
> > > > > >
> > > > > > Can we make it possible to automate this with --fixup-buildids or a
> > > > > > perfconfig 'record' knob?
> > > > > >
> > > > > > This would entail requesting that build-ids that _fail_ be sent to the
> > > > > > side-band thread we have in 'perf record', this way we wouldn't have to
> > > > > > traverse the whole perf.data file, be it with 'perf-record' at the end
> > > > > > of a session with faulty build ids, or in a similar fashion using 'perf
> > > > > > inject' as you suggest.
> > > > > >
> > > > > > I even think that we can have all these modes and let the user to decide
> > > > > > how important is this for them and how convenient they want the whole
> > > > > > process to be.
> > >
> > > right, that might be good to decide first.. because as I said,
> > > I never hit faulted build id, so it probably needs the special
> > > setup you guys are using.. could you try on your setup and check
> > > how many faulted build ids you see?
> >
> > Did you check data mmaps? It might be easy to get faults
> > from data files and we don't know if it's an ELF or not
> > before reading the ELF header in the first page.
>
> well, AFAICS the mmap event is sent right after the elf file
> is loaded, so it does not have a chance to be swapped out

I'm talking about the normal data files when we use
perf record -d. Those mmap files might not have page 0
in memory. I'm afraid it's reported as a build-id fault.

Thanks,
Namhyung