[PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
Adding --buildid-mmap option to enable build id in mmap2 events. It will only work if there's kernel support for that and it disables build id cache (implies --no-buildid). It's also possible to enable it permanently via config option in ~.perfconfig file: [record] build-id=mmap Also added build_id bit in the verbose output for perf_event_attr: # perf record --buildid-mmap -vv ... perf_event_attr: type 1 size 120 ... build_id 1 Adding also missing text_poke bit. Signed-off-by: Jiri Olsa --- tools/perf/Documentation/perf-config.txt | 3 ++- tools/perf/Documentation/perf-record.txt | 3 +++ tools/perf/builtin-record.c | 20 tools/perf/util/evsel.c | 10 ++ tools/perf/util/perf_api_probe.c | 10 ++ tools/perf/util/perf_api_probe.h | 1 + tools/perf/util/perf_event_attr_fprintf.c | 2 ++ tools/perf/util/record.h | 1 + 8 files changed, 45 insertions(+), 5 deletions(-) diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt index 15fad32b9885..66a697ed9655 100644 --- a/tools/perf/Documentation/perf-config.txt +++ b/tools/perf/Documentation/perf-config.txt @@ -559,11 +559,12 @@ kmem.*:: record.*:: record.build-id:: - This option can be 'cache', 'no-cache' or 'skip'. + This option can be 'cache', 'no-cache', 'skip' or 'mmap'. 'cache' is to post-process data and save/update the binaries into the build-id cache (in ~/.debug). This is the default. But if this option is 'no-cache', it will not update the build-id cache. 'skip' skips post-processing and does not update the cache. + 'mmap' skips post-processing and reads build-ids from MMAP events. record.call-graph:: This is identical to 'call-graph.record-mode', except it is diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index 76b9326a..1bcf51e24979 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -482,6 +482,9 @@ Specify vmlinux path which has debuginfo. --buildid-all:: Record build-id of all DSOs regardless whether it's actually hit or not. +--buildid-mmap:: +Record build ids in mmap2 events, disables build id cache (implies --no-buildid). + --aio[=n]:: Use control blocks in asynchronous (Posix AIO) trace writing mode (default: 1, max: 4). Asynchronous mode is supported only when linking Perf tool with libc library diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index adf311d15d3d..47bae9d82d43 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -102,6 +102,7 @@ struct record { boolno_buildid_cache; boolno_buildid_cache_set; boolbuildid_all; + boolbuildid_mmap; booltimestamp_filename; booltimestamp_boundary; struct switch_outputswitch_output; @@ -2139,6 +2140,8 @@ static int perf_record_config(const char *var, const char *value, void *cb) rec->no_buildid_cache = true; else if (!strcmp(value, "skip")) rec->no_buildid = true; + else if (!strcmp(value, "mmap")) + rec->buildid_mmap = true; else return -1; return 0; @@ -2554,6 +2557,8 @@ static struct option __record_options[] = { "file", "vmlinux pathname"), OPT_BOOLEAN(0, "buildid-all", _all, "Record build-id of all DSOs regardless of hits"), + OPT_BOOLEAN(0, "buildid-mmap", _mmap, + "Record build-id in map events"), OPT_BOOLEAN(0, "timestamp-filename", _filename, "append timestamp to output filename"), OPT_BOOLEAN(0, "timestamp-boundary", _boundary, @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) } + if (rec->buildid_mmap) { + if (!perf_can_record_build_id()) { + pr_err("Failed: no support to record build id in mmap events, update your kernel.\n"); + err = -EINVAL; + goto out_opts; + } + pr_debug("Enabling build id in mmap2 events.\n"); + /* Enable mmap build id synthesizing. */ + symbol_conf.buildid_mmap2 = true; + /* Enable perf_event_attr::build_id bit. */ + rec->opts.build_id = true; + /* Disable build id cache. */ + rec->no_buildid = true; + } + if (rec->opts.kcore)
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Sat, Nov 14, 2020 at 09:31:56AM +0900, Namhyung Kim wrote: > On Fri, Nov 13, 2020 at 8:09 PM Jiri Olsa wrote: > > > > On Fri, Nov 13, 2020 at 01:40:00PM +0900, Namhyung Kim wrote: > > > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > > > Adding --buildid-mmap option to enable build id in mmap2 events. > > > > It will only work if there's kernel support for that and it disables > > > > build id cache (implies --no-buildid). > > > > > > > > It's also possible to enable it permanently via config option > > > > in ~.perfconfig file: > > > > > > > > [record] > > > > build-id=mmap > > > > > > You also need to update the documentation. > > > > right, forgot doc for the config option > > > > SNIP > > > > > > "append timestamp to output filename"), > > > > OPT_BOOLEAN(0, "timestamp-boundary", _boundary, > > > > @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) > > > > > > > > } > > > > > > > > + if (rec->buildid_mmap) { > > > > + if (!perf_can_record_build_id()) { > > > > + pr_err("Failed: no support to record build id in > > > > mmap events, update your kernel.\n"); > > > > + err = -EINVAL; > > > > + goto out_opts; > > > > + } > > > > + pr_debug("Enabling build id in mmap2 events.\n"); > > > > + /* Enable mmap build id synthesizing. */ > > > > + symbol_conf.buildid_mmap2 = true; > > > > + /* Enable perf_event_attr::build_id bit. */ > > > > + rec->opts.build_id = true; > > > > + /* Disable build id cache. */ > > > > + rec->no_buildid = true; > > > > > > I'm afraid this can make it miss some build-id in the end because of > > > the possibility of the failure. > > > > with following fix (already merged): > > b33164f2bd1c bpf: Iterate through all PT_NOTE sections when looking for > > build id > > > > I could see high rate of build id being retrieved > > > > I'll make new numbers for next version, but I think we can neglect > > the failure, considering that we pick only 'hit' objects out of all > > of them > > > > also enabling the build id cache for this would go against the > > purpose why I'd like to have this.. so hopefuly the numbers > > will be convincing ;-) > > Yeah, I think it'd be ok for most cases but we cannot guarantee.. > What about checking the dso list at the end of a record session > and check all of them having build-id? Then we can safely skip > the build-id collecting stage. Hmm.. but it won't work for the pipe. how about inject command that would add missing buildids to mmap2 events jirka
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Fri, Nov 13, 2020 at 8:09 PM Jiri Olsa wrote: > > On Fri, Nov 13, 2020 at 01:40:00PM +0900, Namhyung Kim wrote: > > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > > Adding --buildid-mmap option to enable build id in mmap2 events. > > > It will only work if there's kernel support for that and it disables > > > build id cache (implies --no-buildid). > > > > > > It's also possible to enable it permanently via config option > > > in ~.perfconfig file: > > > > > > [record] > > > build-id=mmap > > > > You also need to update the documentation. > > right, forgot doc for the config option > > SNIP > > > > "append timestamp to output filename"), > > > OPT_BOOLEAN(0, "timestamp-boundary", _boundary, > > > @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) > > > > > > } > > > > > > + if (rec->buildid_mmap) { > > > + if (!perf_can_record_build_id()) { > > > + pr_err("Failed: no support to record build id in mmap > > > events, update your kernel.\n"); > > > + err = -EINVAL; > > > + goto out_opts; > > > + } > > > + pr_debug("Enabling build id in mmap2 events.\n"); > > > + /* Enable mmap build id synthesizing. */ > > > + symbol_conf.buildid_mmap2 = true; > > > + /* Enable perf_event_attr::build_id bit. */ > > > + rec->opts.build_id = true; > > > + /* Disable build id cache. */ > > > + rec->no_buildid = true; > > > > I'm afraid this can make it miss some build-id in the end because of > > the possibility of the failure. > > with following fix (already merged): > b33164f2bd1c bpf: Iterate through all PT_NOTE sections when looking for > build id > > I could see high rate of build id being retrieved > > I'll make new numbers for next version, but I think we can neglect > the failure, considering that we pick only 'hit' objects out of all > of them > > also enabling the build id cache for this would go against the > purpose why I'd like to have this.. so hopefuly the numbers > will be convincing ;-) Yeah, I think it'd be ok for most cases but we cannot guarantee.. What about checking the dso list at the end of a record session and check all of them having build-id? Then we can safely skip the build-id collecting stage. Hmm.. but it won't work for the pipe. Thanks, Namhyung
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Fri, Nov 13, 2020 at 01:40:00PM +0900, Namhyung Kim wrote: > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > Adding --buildid-mmap option to enable build id in mmap2 events. > > It will only work if there's kernel support for that and it disables > > build id cache (implies --no-buildid). > > > > It's also possible to enable it permanently via config option > > in ~.perfconfig file: > > > > [record] > > build-id=mmap > > You also need to update the documentation. right, forgot doc for the config option SNIP > > "append timestamp to output filename"), > > OPT_BOOLEAN(0, "timestamp-boundary", _boundary, > > @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) > > > > } > > > > + if (rec->buildid_mmap) { > > + if (!perf_can_record_build_id()) { > > + pr_err("Failed: no support to record build id in mmap > > events, update your kernel.\n"); > > + err = -EINVAL; > > + goto out_opts; > > + } > > + pr_debug("Enabling build id in mmap2 events.\n"); > > + /* Enable mmap build id synthesizing. */ > > + symbol_conf.buildid_mmap2 = true; > > + /* Enable perf_event_attr::build_id bit. */ > > + rec->opts.build_id = true; > > + /* Disable build id cache. */ > > + rec->no_buildid = true; > > I'm afraid this can make it miss some build-id in the end because of > the possibility of the failure. with following fix (already merged): b33164f2bd1c bpf: Iterate through all PT_NOTE sections when looking for build id I could see high rate of build id being retrieved I'll make new numbers for next version, but I think we can neglect the failure, considering that we pick only 'hit' objects out of all of them also enabling the build id cache for this would go against the purpose why I'd like to have this.. so hopefuly the numbers will be convincing ;-) jirka
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > Adding --buildid-mmap option to enable build id in mmap2 events. > It will only work if there's kernel support for that and it disables > build id cache (implies --no-buildid). > > It's also possible to enable it permanently via config option > in ~.perfconfig file: > > [record] > build-id=mmap You also need to update the documentation. > > Also added build_id bit in the verbose output for perf_event_attr: > > # perf record --buildid-mmap -vv > ... > perf_event_attr: > type 1 > size 120 > ... > build_id 1 > > Signed-off-by: Jiri Olsa > --- > tools/perf/Documentation/perf-record.txt | 3 +++ > tools/perf/builtin-record.c | 20 > tools/perf/util/evsel.c | 10 ++ > tools/perf/util/perf_api_probe.c | 10 ++ > tools/perf/util/perf_api_probe.h | 1 + > tools/perf/util/perf_event_attr_fprintf.c | 1 + > tools/perf/util/record.h | 1 + > 7 files changed, 42 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/Documentation/perf-record.txt > b/tools/perf/Documentation/perf-record.txt > index 76b9326a..1bcf51e24979 100644 > --- a/tools/perf/Documentation/perf-record.txt > +++ b/tools/perf/Documentation/perf-record.txt > @@ -482,6 +482,9 @@ Specify vmlinux path which has debuginfo. > --buildid-all:: > Record build-id of all DSOs regardless whether it's actually hit or not. > > +--buildid-mmap:: > +Record build ids in mmap2 events, disables build id cache (implies > --no-buildid). > + > --aio[=n]:: > Use control blocks in asynchronous (Posix AIO) trace writing mode > (default: 1, max: 4). > Asynchronous mode is supported only when linking Perf tool with libc library > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index adf311d15d3d..47bae9d82d43 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -102,6 +102,7 @@ struct record { > boolno_buildid_cache; > boolno_buildid_cache_set; > boolbuildid_all; > + boolbuildid_mmap; > booltimestamp_filename; > booltimestamp_boundary; > struct switch_outputswitch_output; > @@ -2139,6 +2140,8 @@ static int perf_record_config(const char *var, const > char *value, void *cb) > rec->no_buildid_cache = true; > else if (!strcmp(value, "skip")) > rec->no_buildid = true; > + else if (!strcmp(value, "mmap")) > + rec->buildid_mmap = true; > else > return -1; > return 0; > @@ -2554,6 +2557,8 @@ static struct option __record_options[] = { > "file", "vmlinux pathname"), > OPT_BOOLEAN(0, "buildid-all", _all, > "Record build-id of all DSOs regardless of hits"), > + OPT_BOOLEAN(0, "buildid-mmap", _mmap, > + "Record build-id in map events"), > OPT_BOOLEAN(0, "timestamp-filename", _filename, > "append timestamp to output filename"), > OPT_BOOLEAN(0, "timestamp-boundary", _boundary, > @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) > > } > > + if (rec->buildid_mmap) { > + if (!perf_can_record_build_id()) { > + pr_err("Failed: no support to record build id in mmap > events, update your kernel.\n"); > + err = -EINVAL; > + goto out_opts; > + } > + pr_debug("Enabling build id in mmap2 events.\n"); > + /* Enable mmap build id synthesizing. */ > + symbol_conf.buildid_mmap2 = true; > + /* Enable perf_event_attr::build_id bit. */ > + rec->opts.build_id = true; > + /* Disable build id cache. */ > + rec->no_buildid = true; I'm afraid this can make it miss some build-id in the end because of the possibility of the failure. > + } > + > if (rec->opts.kcore) > rec->data.is_dir = true; > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 1cad6051d8b0..749d806ee1d1 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -1170,10 +1170,12 @@ void evsel__config(struct evsel *evsel, struct > record_opts *opts, > if (opts->sample_weight) > evsel__set_sample_bit(evsel, WEIGHT); > > - attr->task = track; > - attr->mmap = track; > - attr->mmap2 = track && !perf_missing_features.mmap2; > - attr->comm = track; > + attr->task = track; > + attr->mmap = track; > + attr->mmap2= track && !perf_missing_features.mmap2; > + attr->comm = track; >
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Thu, Nov 12, 2020 at 05:39:52PM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Nov 12, 2020 at 12:57:10PM +0100, Jiri Olsa escreveu: > > On Wed, Nov 11, 2020 at 09:00:46AM -0800, Andi Kleen wrote: > > > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > > > Adding --buildid-mmap option to enable build id in mmap2 events. > > > > It will only work if there's kernel support for that and it disables > > > > build id cache (implies --no-buildid). > > > > What's the point of the option? Why not enable it by default > > > if the kernel supports it? > > > > With the option most user won't get the benefit. > > > > The only reason I can think of for an option would be to disable > > > so that old tools can still process. > > > yes, that was request in the rfc post, we want the new default > > perf.data be still readable by older perf tools > > We need to change perf so that when it finds some option it doesn't > grok, it just ignores extra things in a record like MMAP2 and just warns > the user that things are being ignored. > > So that we can add new stuff by default without requiring an ever longer > command line option, like with --all-cgroups, etc. > > And provide the options to avoid using new stuff if we know that the > perf.data file will be processed by someone with an older tool that > can't update. hum, can we just stop being this way compatible? ;-) I can't see too much benefit in it, but not sure how common is to report perf.data with older perf than it was recorded with most of the time it will probably work anyway, just big changes list this one will screw that jirka
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
Em Thu, Nov 12, 2020 at 12:57:10PM +0100, Jiri Olsa escreveu: > On Wed, Nov 11, 2020 at 09:00:46AM -0800, Andi Kleen wrote: > > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > > Adding --buildid-mmap option to enable build id in mmap2 events. > > > It will only work if there's kernel support for that and it disables > > > build id cache (implies --no-buildid). > > What's the point of the option? Why not enable it by default > > if the kernel supports it? > > With the option most user won't get the benefit. > > The only reason I can think of for an option would be to disable > > so that old tools can still process. > yes, that was request in the rfc post, we want the new default > perf.data be still readable by older perf tools We need to change perf so that when it finds some option it doesn't grok, it just ignores extra things in a record like MMAP2 and just warns the user that things are being ignored. So that we can add new stuff by default without requiring an ever longer command line option, like with --all-cgroups, etc. And provide the options to avoid using new stuff if we know that the perf.data file will be processed by someone with an older tool that can't update. - Arnaldo
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Wed, Nov 11, 2020 at 09:00:46AM -0800, Andi Kleen wrote: > On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > > Adding --buildid-mmap option to enable build id in mmap2 events. > > It will only work if there's kernel support for that and it disables > > build id cache (implies --no-buildid). > > What's the point of the option? Why not enable it by default > if the kernel supports it? > > With the option most user won't get the benefit. > > The only reason I can think of for an option would be to disable > so that old tools can still process. yes, that was request in the rfc post, we want the new default perf.data be still readable by older perf tools jirka
Re: [PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
On Mon, Nov 09, 2020 at 10:54:15PM +0100, Jiri Olsa wrote: > Adding --buildid-mmap option to enable build id in mmap2 events. > It will only work if there's kernel support for that and it disables > build id cache (implies --no-buildid). What's the point of the option? Why not enable it by default if the kernel supports it? With the option most user won't get the benefit. The only reason I can think of for an option would be to disable so that old tools can still process. -Andi
[PATCH 24/24] perf record: Add --buildid-mmap option to enable mmap's build id
Adding --buildid-mmap option to enable build id in mmap2 events. It will only work if there's kernel support for that and it disables build id cache (implies --no-buildid). It's also possible to enable it permanently via config option in ~.perfconfig file: [record] build-id=mmap Also added build_id bit in the verbose output for perf_event_attr: # perf record --buildid-mmap -vv ... perf_event_attr: type 1 size 120 ... build_id 1 Signed-off-by: Jiri Olsa --- tools/perf/Documentation/perf-record.txt | 3 +++ tools/perf/builtin-record.c | 20 tools/perf/util/evsel.c | 10 ++ tools/perf/util/perf_api_probe.c | 10 ++ tools/perf/util/perf_api_probe.h | 1 + tools/perf/util/perf_event_attr_fprintf.c | 1 + tools/perf/util/record.h | 1 + 7 files changed, 42 insertions(+), 4 deletions(-) diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index 76b9326a..1bcf51e24979 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -482,6 +482,9 @@ Specify vmlinux path which has debuginfo. --buildid-all:: Record build-id of all DSOs regardless whether it's actually hit or not. +--buildid-mmap:: +Record build ids in mmap2 events, disables build id cache (implies --no-buildid). + --aio[=n]:: Use control blocks in asynchronous (Posix AIO) trace writing mode (default: 1, max: 4). Asynchronous mode is supported only when linking Perf tool with libc library diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index adf311d15d3d..47bae9d82d43 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -102,6 +102,7 @@ struct record { boolno_buildid_cache; boolno_buildid_cache_set; boolbuildid_all; + boolbuildid_mmap; booltimestamp_filename; booltimestamp_boundary; struct switch_outputswitch_output; @@ -2139,6 +2140,8 @@ static int perf_record_config(const char *var, const char *value, void *cb) rec->no_buildid_cache = true; else if (!strcmp(value, "skip")) rec->no_buildid = true; + else if (!strcmp(value, "mmap")) + rec->buildid_mmap = true; else return -1; return 0; @@ -2554,6 +2557,8 @@ static struct option __record_options[] = { "file", "vmlinux pathname"), OPT_BOOLEAN(0, "buildid-all", _all, "Record build-id of all DSOs regardless of hits"), + OPT_BOOLEAN(0, "buildid-mmap", _mmap, + "Record build-id in map events"), OPT_BOOLEAN(0, "timestamp-filename", _filename, "append timestamp to output filename"), OPT_BOOLEAN(0, "timestamp-boundary", _boundary, @@ -2657,6 +2662,21 @@ int cmd_record(int argc, const char **argv) } + if (rec->buildid_mmap) { + if (!perf_can_record_build_id()) { + pr_err("Failed: no support to record build id in mmap events, update your kernel.\n"); + err = -EINVAL; + goto out_opts; + } + pr_debug("Enabling build id in mmap2 events.\n"); + /* Enable mmap build id synthesizing. */ + symbol_conf.buildid_mmap2 = true; + /* Enable perf_event_attr::build_id bit. */ + rec->opts.build_id = true; + /* Disable build id cache. */ + rec->no_buildid = true; + } + if (rec->opts.kcore) rec->data.is_dir = true; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 1cad6051d8b0..749d806ee1d1 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -1170,10 +1170,12 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts, if (opts->sample_weight) evsel__set_sample_bit(evsel, WEIGHT); - attr->task = track; - attr->mmap = track; - attr->mmap2 = track && !perf_missing_features.mmap2; - attr->comm = track; + attr->task = track; + attr->mmap = track; + attr->mmap2= track && !perf_missing_features.mmap2; + attr->comm = track; + attr->build_id = track && opts->build_id; + /* * ksymbol is tracked separately with text poke because it needs to be * system wide and enabled immediately. diff --git a/tools/perf/util/perf_api_probe.c b/tools/perf/util/perf_api_probe.c index 3840d02f0f7b..829af17a0867 100644 ---