2023-02-23 07:02:34

by Namhyung Kim

Subject: [PATCH] perf inject: Fix --buildid-all not to eat up MMAP2

When MMAP2 has PERF_RECORD_MISC_MMAP_BUILD_ID flag, it means the record
already has the build-id info. So it marks the DSO as hit, to skip if
the same DSO is not processed if it happens to miss the build-id later.

But it missed to copy the MMAP2 record itself so it'd fail to symbolize
samples for those regions.

For example, the following generates 249 MMAP2 events.

$ perf record --buildid-mmap -o- true | perf report --stat -i- | grep MMAP2
MMAP2 events: 249 (86.8%)

Adding perf inject should not change the number of events like this

$ perf record --buildid-mmap -o- true | perf inject -b | \
> perf report --stat -i- | grep MMAP2
MMAP2 events: 249 (86.5%)

But when --buildid-all is used, it eats most of the MMAP2 events.

$ perf record --buildid-mmap -o- true | perf inject -b --buildid-all | \
> perf report --stat -i- | grep MMAP2
MMAP2 events: 1 ( 2.5%)

With this patch, it shows the original number now.

$ perf record --buildid-mmap -o- true | perf inject -b --buildid-all | \
> perf report --stat -i- | grep MMAP2
MMAP2 events: 249 (86.5%)

Cc: [email protected]
Fixes: f7fc0d1c915a ("perf inject: Do not inject BUILD_ID record if MMAP2 has it")
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-inject.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index f8182417b734..10bb1d494258 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -538,6 +538,7 @@ static int perf_event__repipe_buildid_mmap2(struct perf_tool *tool,
dso->hit = 1;
}
dso__put(dso);
+ perf_event__repipe(tool, event, sample, machine);
return 0;
}

--
2.39.2.637.g21b0678d19-goog

2023-02-23 13:32:07

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH] perf inject: Fix --buildid-all not to eat up MMAP2

Em Wed, Feb 22, 2023 at 11:01:55PM -0800, Namhyung Kim escreveu:
> When MMAP2 has PERF_RECORD_MISC_MMAP_BUILD_ID flag, it means the record
> already has the build-id info. So it marks the DSO as hit, to skip if
> the same DSO is not processed if it happens to miss the build-id later.
>
> But it missed to copy the MMAP2 record itself so it'd fail to symbolize
> samples for those regions.

Thanks, applied.

- Arnaldo


> For example, the following generates 249 MMAP2 events.
>
> $ perf record --buildid-mmap -o- true | perf report --stat -i- | grep MMAP2
> MMAP2 events: 249 (86.8%)
>
> Adding perf inject should not change the number of events like this
>
> $ perf record --buildid-mmap -o- true | perf inject -b | \
> > perf report --stat -i- | grep MMAP2
> MMAP2 events: 249 (86.5%)
>
> But when --buildid-all is used, it eats most of the MMAP2 events.
>
> $ perf record --buildid-mmap -o- true | perf inject -b --buildid-all | \
> > perf report --stat -i- | grep MMAP2
> MMAP2 events: 1 ( 2.5%)
>
> With this patch, it shows the original number now.
>
> $ perf record --buildid-mmap -o- true | perf inject -b --buildid-all | \
> > perf report --stat -i- | grep MMAP2
> MMAP2 events: 249 (86.5%)
>
> Cc: [email protected]
> Fixes: f7fc0d1c915a ("perf inject: Do not inject BUILD_ID record if MMAP2 has it")
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/builtin-inject.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index f8182417b734..10bb1d494258 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -538,6 +538,7 @@ static int perf_event__repipe_buildid_mmap2(struct perf_tool *tool,
> dso->hit = 1;
> }
> dso__put(dso);
> + perf_event__repipe(tool, event, sample, machine);
> return 0;
> }
>
> --
> 2.39.2.637.g21b0678d19-goog
>

--

- Arnaldo