[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZlS8pc39t2c1WFye@x1>
Date: Mon, 27 May 2024 14:02:29 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Artem Savkov <asavkov@...hat.com>
Cc: linux-perf-users@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf record: add a shortcut for metrics
On Mon, May 27, 2024 at 12:15:19PM +0200, Artem Savkov wrote:
> Add -M/--metrics option to perf-record providing a shortcut to record
> metrics and metricgroups. This option mirrors the one in perf-stat.
>
> Suggested-by: Arnaldo Carvalho de Melo <acme@...nel.org>
> Signed-off-by: Artem Savkov <asavkov@...hat.com>
Not building for me, I needed to add the rblist.h header and also I
think we need to use metricgroup__rblist_init(&mevents), right?
Testing it now.
- Arnaldo
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 18da3ce380152ad1..5d67b0711c166fae 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -27,6 +27,7 @@
#include "util/session.h"
#include "util/tool.h"
#include "util/symbol.h"
+#include "util/rblist.h"
#include "util/record.h"
#include "util/cpumap.h"
#include "util/thread_map.h"
@@ -4017,6 +4018,7 @@ int cmd_record(int argc, const char **argv)
set_nobuild('\0', "off-cpu", "no BUILD_BPF_SKEL=1", true);
# undef set_nobuild
#endif
+ metricgroup__rblist_init(&mevents);
/* Disable eager loading of kernel symbols that adds overhead to perf record. */
symbol_conf.lazy_load_kernel_maps = true;
> ---
> tools/perf/Documentation/perf-record.txt | 7 +++-
> tools/perf/builtin-record.c | 43 ++++++++++++++++++++++++
> 2 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 6015fdd08fb63..ebb560d137e62 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -18,7 +18,6 @@ from it, into perf.data - without displaying anything.
>
> This file can then be inspected later on, using 'perf report'.
>
> -
> OPTIONS
> -------
> <command>...::
> @@ -216,6 +215,12 @@ OPTIONS
> na, by_data, by_addr (for mem_blk)
> hops0, hops1, hops2, hops3 (for mem_hops)
>
> +-M::
> +--metrics::
> +Record metrics or metricgroups specified in a comma separated list.
> +For a group all metrics from the group are added.
> +See perf list output for the possible metrics and metricgroups.
> +
> --exclude-perf::
> Don't record events issued by perf itself. This option should follow
> an event selector (-e) which selects tracepoint event(s). It adds a
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 66a3de8ac6618..5828051ff2736 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -40,6 +40,7 @@
> #include "util/trigger.h"
> #include "util/perf-hooks.h"
> #include "util/cpu-set-sched.h"
> +#include "util/metricgroup.h"
> #include "util/synthetic-events.h"
> #include "util/time-utils.h"
> #include "util/units.h"
> @@ -188,6 +189,7 @@ static volatile int done;
> static volatile int auxtrace_record__snapshot_started;
> static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
> static DEFINE_TRIGGER(switch_output_trigger);
> +static char *metrics;
>
> static const char *affinity_tags[PERF_AFFINITY_MAX] = {
> "SYS", "NODE", "CPU"
> @@ -200,6 +202,25 @@ static inline pid_t gettid(void)
> }
> #endif
>
> +static int append_metric_groups(const struct option *opt __maybe_unused,
> + const char *str,
> + int unset __maybe_unused)
> +{
> + if (metrics) {
> + char *tmp;
> +
> + if (asprintf(&tmp, "%s,%s", metrics, str) < 0)
> + return -ENOMEM;
> + free(metrics);
> + metrics = tmp;
> + } else {
> + metrics = strdup(str);
> + if (!metrics)
> + return -ENOMEM;
> + }
> + return 0;
> +}
> +
> static int record__threads_enabled(struct record *rec)
> {
> return rec->opts.threads_spec;
> @@ -3382,6 +3403,9 @@ static struct option __record_options[] = {
> parse_events_option),
> OPT_CALLBACK(0, "filter", &record.evlist, "filter",
> "event filter", parse_filter),
> + OPT_CALLBACK('M', "metrics", &record.evlist, "metric/metric group list",
> + "monitor specified metrics or metric groups (separated by ,)",
> + append_metric_groups),
> OPT_CALLBACK_NOOPT(0, "exclude-perf", &record.evlist,
> NULL, "don't record events from perf itself",
> exclude_perf),
> @@ -3984,6 +4008,7 @@ int cmd_record(int argc, const char **argv)
> int err;
> struct record *rec = &record;
> char errbuf[BUFSIZ];
> + struct rblist mevents;
>
> setlocale(LC_ALL, "");
>
> @@ -4153,6 +4178,23 @@ int cmd_record(int argc, const char **argv)
> if (record.opts.overwrite)
> record.opts.tail_synthesize = true;
>
> + if (metrics) {
> + const char *pmu = parse_events_option_args.pmu_filter ?: "all";
> + int ret = metricgroup__parse_groups(rec->evlist, pmu, metrics,
> + false, /* metric_no_group */
> + false, /* metric_no_merge */
> + false, /* metric_no_threshold */
> + rec->opts.target.cpu_list,
> + rec->opts.target.system_wide,
> + false, /* hardware_aware_grouping */
> + &mevents);
> + if (ret) {
> + err = ret;
> + goto out;
> + }
> + zfree(&metrics);
> + }
> +
> if (rec->evlist->core.nr_entries == 0) {
> bool can_profile_kernel = perf_event_paranoid_check(1);
>
> @@ -4264,6 +4306,7 @@ int cmd_record(int argc, const char **argv)
> out_opts:
> record__free_thread_masks(rec, rec->nr_threads);
> rec->nr_threads = 0;
> + metricgroup__rblist_exit(&mevents);
> evlist__close_control(rec->opts.ctl_fd, rec->opts.ctl_fd_ack, &rec->opts.ctl_fd_close);
> return err;
> }
> --
> 2.45.1
Powered by blists - more mailing lists