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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQp-UmG_4EGrgEHD@google.com>
Date: Tue, 4 Nov 2025 14:29:38 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	James Clark <james.clark@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf record: Make sure to update build-ID cache

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@...nel.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@...nel.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
> >