* [PATCH] perf record: Make sure to update build-ID cache
@ 2025-11-04 4:26 Namhyung Kim
2025-11-04 5:15 ` Ian Rogers
0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2025-11-04 4:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers
Cc: Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
Recent change on enabling --buildid-mmap by default brought an issue
with build-id handling. With build-ID in MMAP2 records, we don't need
to save the build-ID table in the header are of a perf data file.
But the actual file contents still need to be cached in the debug
directory for annotation etc. Split the build-ID header processing and
caching and make sure perf record to save hit DSOs in the build-ID cache
by moving perf_session__cache_build_ids() to the end of the record__
finish_output().
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-record.c | 30 +++++++++++++++---------------
| 1 -
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ffb94a8339b03ec2..fe10bb7f35cbea05 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1890,15 +1890,14 @@ record__finish_output(struct record *rec)
}
/* Buildid scanning disabled or build ID in kernel and synthesized map events. */
- if (!rec->no_buildid) {
+ if (!rec->no_buildid || !rec->no_buildid_cache) {
process_buildids(rec);
if (rec->buildid_all)
perf_session__dsos_hit_all(rec->session);
}
perf_session__write_header(rec->session, rec->evlist, fd, true);
-
- return;
+ perf_session__cache_build_ids(rec->session);
}
static int record__synthesize_workload(struct record *rec, bool tail)
@@ -3083,7 +3082,7 @@ static int perf_record_config(const char *var, const char *value, void *cb)
else if (!strcmp(value, "no-cache"))
rec->no_buildid_cache = true;
else if (!strcmp(value, "skip"))
- rec->no_buildid = true;
+ rec->no_buildid = rec->no_buildid_cache = true;
else if (!strcmp(value, "mmap"))
rec->buildid_mmap = true;
else if (!strcmp(value, "no-mmap"))
@@ -4192,24 +4191,25 @@ int cmd_record(int argc, const char **argv)
record.opts.record_switch_events = true;
}
- if (!rec->buildid_mmap) {
- pr_debug("Disabling build id in synthesized mmap2 events.\n");
- symbol_conf.no_buildid_mmap2 = true;
- } else if (rec->buildid_mmap_set) {
- /*
- * Explicitly passing --buildid-mmap disables buildid processing
- * and cache generation.
- */
- rec->no_buildid = true;
- }
if (rec->buildid_mmap && !perf_can_record_build_id()) {
pr_warning("Missing support for build id in kernel mmap events.\n"
"Disable this warning with --no-buildid-mmap\n");
rec->buildid_mmap = false;
}
+
if (rec->buildid_mmap) {
/* Enable perf_event_attr::build_id bit. */
rec->opts.build_id = true;
+ /* Disable build-ID table in the header. */
+ rec->no_buildid = true;
+ } else {
+ pr_debug("Disabling build id in synthesized mmap2 events.\n");
+ symbol_conf.no_buildid_mmap2 = true;
+ }
+
+ if (rec->no_buildid_set && rec->no_buildid) {
+ /* -B implies -N for historic reasons. */
+ rec->no_buildid_cache = true;
}
if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
@@ -4306,7 +4306,7 @@ int cmd_record(int argc, const char **argv)
err = -ENOMEM;
- if (rec->no_buildid_cache || rec->no_buildid) {
+ if (rec->no_buildid_cache) {
disable_buildid_cache();
} else if (rec->switch_output.enabled) {
/*
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4f2a6e10ed5cc0bd..4de7ca16b5522c47 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -335,7 +335,6 @@ static int write_build_id(struct feat_fd *ff,
pr_debug("failed to write buildid table\n");
return err;
}
- perf_session__cache_build_ids(session);
return 0;
}
--
2.51.2.997.g839fc31de9-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf record: Make sure to update build-ID cache
2025-11-04 4:26 [PATCH] perf record: Make sure to update build-ID cache Namhyung Kim
@ 2025-11-04 5:15 ` Ian Rogers
2025-11-04 22:29 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2025-11-04 5:15 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Nov 3, 2025 at 8:26 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Recent change on enabling --buildid-mmap by default brought an issue
> with build-id handling. With build-ID in MMAP2 records, we don't need
> to save the build-ID table in the header are of a perf data file.
nit: s/are of/of/
>
> But the actual file contents still need to be cached in the debug
> directory for annotation etc. Split the build-ID header processing and
> caching and make sure perf record to save hit DSOs in the build-ID cache
> by moving perf_session__cache_build_ids() to the end of the record__
> finish_output().
The changes to builtin-record look okay, but do we need to worry about
`perf mem record` and similar? The header change will impact all
perf.data file generation.
Thanks,
Ian
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-record.c | 30 +++++++++++++++---------------
> tools/perf/util/header.c | 1 -
> 2 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index ffb94a8339b03ec2..fe10bb7f35cbea05 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1890,15 +1890,14 @@ record__finish_output(struct record *rec)
> }
>
> /* Buildid scanning disabled or build ID in kernel and synthesized map events. */
> - if (!rec->no_buildid) {
> + if (!rec->no_buildid || !rec->no_buildid_cache) {
> process_buildids(rec);
>
> if (rec->buildid_all)
> perf_session__dsos_hit_all(rec->session);
> }
> perf_session__write_header(rec->session, rec->evlist, fd, true);
> -
> - return;
> + perf_session__cache_build_ids(rec->session);
> }
>
> static int record__synthesize_workload(struct record *rec, bool tail)
> @@ -3083,7 +3082,7 @@ static int perf_record_config(const char *var, const char *value, void *cb)
> else if (!strcmp(value, "no-cache"))
> rec->no_buildid_cache = true;
> else if (!strcmp(value, "skip"))
> - rec->no_buildid = true;
> + rec->no_buildid = rec->no_buildid_cache = true;
> else if (!strcmp(value, "mmap"))
> rec->buildid_mmap = true;
> else if (!strcmp(value, "no-mmap"))
> @@ -4192,24 +4191,25 @@ int cmd_record(int argc, const char **argv)
> record.opts.record_switch_events = true;
> }
>
> - if (!rec->buildid_mmap) {
> - pr_debug("Disabling build id in synthesized mmap2 events.\n");
> - symbol_conf.no_buildid_mmap2 = true;
> - } else if (rec->buildid_mmap_set) {
> - /*
> - * Explicitly passing --buildid-mmap disables buildid processing
> - * and cache generation.
> - */
> - rec->no_buildid = true;
> - }
> if (rec->buildid_mmap && !perf_can_record_build_id()) {
> pr_warning("Missing support for build id in kernel mmap events.\n"
> "Disable this warning with --no-buildid-mmap\n");
> rec->buildid_mmap = false;
> }
> +
> if (rec->buildid_mmap) {
> /* Enable perf_event_attr::build_id bit. */
> rec->opts.build_id = true;
> + /* Disable build-ID table in the header. */
> + rec->no_buildid = true;
> + } else {
> + pr_debug("Disabling build id in synthesized mmap2 events.\n");
> + symbol_conf.no_buildid_mmap2 = true;
> + }
> +
> + if (rec->no_buildid_set && rec->no_buildid) {
> + /* -B implies -N for historic reasons. */
> + rec->no_buildid_cache = true;
> }
>
> if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> @@ -4306,7 +4306,7 @@ int cmd_record(int argc, const char **argv)
>
> err = -ENOMEM;
>
> - if (rec->no_buildid_cache || rec->no_buildid) {
> + if (rec->no_buildid_cache) {
> disable_buildid_cache();
> } else if (rec->switch_output.enabled) {
> /*
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4f2a6e10ed5cc0bd..4de7ca16b5522c47 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -335,7 +335,6 @@ static int write_build_id(struct feat_fd *ff,
> pr_debug("failed to write buildid table\n");
> return err;
> }
> - perf_session__cache_build_ids(session);
>
> return 0;
> }
> --
> 2.51.2.997.g839fc31de9-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf record: Make sure to update build-ID cache
2025-11-04 5:15 ` Ian Rogers
@ 2025-11-04 22:29 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2025-11-04 22:29 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Nov 03, 2025 at 09:15:57PM -0800, Ian Rogers wrote:
> On Mon, Nov 3, 2025 at 8:26 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Recent change on enabling --buildid-mmap by default brought an issue
> > with build-id handling. With build-ID in MMAP2 records, we don't need
> > to save the build-ID table in the header are of a perf data file.
>
> nit: s/are of/of/
Thanks, will fix in v2.
>
> >
> > But the actual file contents still need to be cached in the debug
> > directory for annotation etc. Split the build-ID header processing and
> > caching and make sure perf record to save hit DSOs in the build-ID cache
> > by moving perf_session__cache_build_ids() to the end of the record__
> > finish_output().
>
> The changes to builtin-record look okay, but do we need to worry about
> `perf mem record` and similar? The header change will impact all
> perf.data file generation.
I believe all other codes just call cmd_record() in builtin-record.c to
share the logic.
Thanks,
Namhyung
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/builtin-record.c | 30 +++++++++++++++---------------
> > tools/perf/util/header.c | 1 -
> > 2 files changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index ffb94a8339b03ec2..fe10bb7f35cbea05 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -1890,15 +1890,14 @@ record__finish_output(struct record *rec)
> > }
> >
> > /* Buildid scanning disabled or build ID in kernel and synthesized map events. */
> > - if (!rec->no_buildid) {
> > + if (!rec->no_buildid || !rec->no_buildid_cache) {
> > process_buildids(rec);
> >
> > if (rec->buildid_all)
> > perf_session__dsos_hit_all(rec->session);
> > }
> > perf_session__write_header(rec->session, rec->evlist, fd, true);
> > -
> > - return;
> > + perf_session__cache_build_ids(rec->session);
> > }
> >
> > static int record__synthesize_workload(struct record *rec, bool tail)
> > @@ -3083,7 +3082,7 @@ static int perf_record_config(const char *var, const char *value, void *cb)
> > else if (!strcmp(value, "no-cache"))
> > rec->no_buildid_cache = true;
> > else if (!strcmp(value, "skip"))
> > - rec->no_buildid = true;
> > + rec->no_buildid = rec->no_buildid_cache = true;
> > else if (!strcmp(value, "mmap"))
> > rec->buildid_mmap = true;
> > else if (!strcmp(value, "no-mmap"))
> > @@ -4192,24 +4191,25 @@ int cmd_record(int argc, const char **argv)
> > record.opts.record_switch_events = true;
> > }
> >
> > - if (!rec->buildid_mmap) {
> > - pr_debug("Disabling build id in synthesized mmap2 events.\n");
> > - symbol_conf.no_buildid_mmap2 = true;
> > - } else if (rec->buildid_mmap_set) {
> > - /*
> > - * Explicitly passing --buildid-mmap disables buildid processing
> > - * and cache generation.
> > - */
> > - rec->no_buildid = true;
> > - }
> > if (rec->buildid_mmap && !perf_can_record_build_id()) {
> > pr_warning("Missing support for build id in kernel mmap events.\n"
> > "Disable this warning with --no-buildid-mmap\n");
> > rec->buildid_mmap = false;
> > }
> > +
> > if (rec->buildid_mmap) {
> > /* Enable perf_event_attr::build_id bit. */
> > rec->opts.build_id = true;
> > + /* Disable build-ID table in the header. */
> > + rec->no_buildid = true;
> > + } else {
> > + pr_debug("Disabling build id in synthesized mmap2 events.\n");
> > + symbol_conf.no_buildid_mmap2 = true;
> > + }
> > +
> > + if (rec->no_buildid_set && rec->no_buildid) {
> > + /* -B implies -N for historic reasons. */
> > + rec->no_buildid_cache = true;
> > }
> >
> > if (rec->opts.record_cgroup && !perf_can_record_cgroup()) {
> > @@ -4306,7 +4306,7 @@ int cmd_record(int argc, const char **argv)
> >
> > err = -ENOMEM;
> >
> > - if (rec->no_buildid_cache || rec->no_buildid) {
> > + if (rec->no_buildid_cache) {
> > disable_buildid_cache();
> > } else if (rec->switch_output.enabled) {
> > /*
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index 4f2a6e10ed5cc0bd..4de7ca16b5522c47 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -335,7 +335,6 @@ static int write_build_id(struct feat_fd *ff,
> > pr_debug("failed to write buildid table\n");
> > return err;
> > }
> > - perf_session__cache_build_ids(session);
> >
> > return 0;
> > }
> > --
> > 2.51.2.997.g839fc31de9-goog
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-04 22:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 4:26 [PATCH] perf record: Make sure to update build-ID cache Namhyung Kim
2025-11-04 5:15 ` Ian Rogers
2025-11-04 22:29 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox