[PATCH] perf record: Make sure to update build-ID cache

[PATCH] perf record: Make sure to update build-ID cache

[Namhyung Kim]

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 +++++++++++++++---------------
 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

Re: [PATCH] perf record: Make sure to update build-ID cache

[Ian Rogers]

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
>

Re: [PATCH] perf record: Make sure to update build-ID cache

[Namhyung Kim]

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
> >