[v3,10/10] perf list: Add json output option

Message ID 20221114210723.2749751-11-irogers@google.com
State New
Headers
Series Restructure perf list and add json output |

Commit Message

Ian Rogers Nov. 14, 2022, 9:07 p.m. UTC
  Output events and metrics in a json format by overriding the print
callbacks. Currently other command line options aren't supported and
metrics are repeated once per metric group.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Documentation/perf-list.txt |   4 +
 tools/perf/builtin-list.c              | 294 ++++++++++++++++++++-----
 2 files changed, 240 insertions(+), 58 deletions(-)
  

Comments

Arnaldo Carvalho de Melo Nov. 15, 2022, 1:44 p.m. UTC | #1
Em Mon, Nov 14, 2022 at 01:07:23PM -0800, Ian Rogers escreveu:
> Output events and metrics in a json format by overriding the print
> callbacks. Currently other command line options aren't supported and
> metrics are repeated once per metric group.

Applied the patch with a few fixes and added this to the last cset:

commit c9367a0658ebcfe8ab0bc4af2648f144c64b53a4
Author: Ian Rogers <irogers@google.com>
Date:   Mon Nov 14 13:07:23 2022 -0800

    perf list: Add JSON output option
    
    Output events and metrics in a JSON format by overriding the print
    callbacks. Currently other command line options aren't supported and
    metrics are repeated once per metric group.
    
    Committer testing:
    
      $ perf list cache
    
      List of pre-defined events (to be used in -e or -M):
    
        L1-dcache-load-misses                              [Hardware cache event]
        L1-dcache-loads                                    [Hardware cache event]
        L1-dcache-prefetches                               [Hardware cache event]
        L1-icache-load-misses                              [Hardware cache event]
        L1-icache-loads                                    [Hardware cache event]
        branch-load-misses                                 [Hardware cache event]
        branch-loads                                       [Hardware cache event]
        dTLB-load-misses                                   [Hardware cache event]
        dTLB-loads                                         [Hardware cache event]
        iTLB-load-misses                                   [Hardware cache event]
        iTLB-loads                                         [Hardware cache event]
      $ perf list --json cache
      [
      {
              "Unit": "cache",
              "EventName": "L1-dcache-load-misses",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "L1-dcache-loads",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "L1-dcache-prefetches",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "L1-icache-load-misses",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "L1-icache-loads",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "branch-load-misses",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "branch-loads",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "dTLB-load-misses",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "dTLB-loads",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "iTLB-load-misses",
              "EventType": "Hardware cache event"
      },
      {
              "Unit": "cache",
              "EventName": "iTLB-loads",
              "EventType": "Hardware cache event"
      }
      ]
      $
    
    Signed-off-by: Ian Rogers <irogers@google.com>
    Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Caleb Biggers <caleb.biggers@intel.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Kajol Jain <kjain@linux.ibm.com>
    Cc: Kan Liang <kan.liang@linux.intel.com>
    Cc: Leo Yan <leo.yan@linaro.org>
    Cc: Mark Rutland <mark.rutland@arm.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Perry Taylor <perry.taylor@intel.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Ravi Bangoria <ravi.bangoria@amd.com>
    Cc: Rob Herring <robh@kernel.org>
    Cc: Sandipan Das <sandipan.das@amd.com>
    Cc: Stephane Eranian <eranian@google.com>
    Cc: Weilin Wang <weilin.wang@intel.com>
    Cc: Xin Gao <gaoxin@cdjrlc.com>
    Cc: Xing Zhengjun <zhengjun.xing@linux.intel.com>
    Link: http://lore.kernel.org/lkml/20221114210723.2749751-11-irogers@google.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 44a819af573dea4c..c5a3cb0f57c7cb8b 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -43,6 +43,10 @@ Print deprecated events. By default the deprecated events are hidden.
 Print PMU events and metrics limited to the specific PMU name.
 (e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
 
+-j::
+--json::
+Output in JSON format.
+
 [[EVENT_MODIFIERS]]
 EVENT MODIFIERS
 ---------------
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 12811fc40a3067cc..aec139f7fbb2d558 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@
 #include "util/strlist.h"
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
+#include <stdarg.h>
 #include <stdio.h>
 
 /**
@@ -228,10 +229,176 @@ static void default_print_metric(void *ps,
 	}
 }
 
+struct json_print_state {
+	/** Should a separator be printed prior to the next item? */
+	bool need_sep;
+};
+
+static void json_print_start(void *print_state __maybe_unused)
+{
+	printf("[\n");
+}
+
+static void json_print_end(void *ps)
+{
+	struct json_print_state *print_state = ps;
+
+	printf("%s]\n", print_state->need_sep ? "\n" : "");
+}
+
+static void fix_escape_printf(const char *fmt, ...)
+{
+	va_list args;
+	char buf[2048];
+	size_t buf_pos = 0;
+
+	va_start(args, fmt);
+	for (size_t fmt_pos = 0; fmt_pos < strlen(fmt); fmt_pos++) {
+		switch (fmt[fmt_pos]) {
+		case '%': {
+			const char *s = va_arg(args, const char*);
+
+			fmt_pos++;
+			assert(fmt[fmt_pos] == 's');
+			for (size_t s_pos = 0; s_pos < strlen(s); s_pos++) {
+				switch (s[s_pos]) {
+				case '\\':
+					__fallthrough;
+				case '\"':
+					buf[buf_pos++] = '\\';
+					assert(buf_pos < sizeof(buf));
+					__fallthrough;
+				default:
+					buf[buf_pos++] = s[s_pos];
+					assert(buf_pos < sizeof(buf));
+					break;
+				}
+			}
+			break;
+		}
+		default:
+			buf[buf_pos++] = fmt[fmt_pos];
+			assert(buf_pos < sizeof(buf));
+			break;
+		}
+	}
+	va_end(args);
+	buf[buf_pos] = '\0';
+	fputs(buf, stdout);
+}
+
+static void json_print_event(void *ps, const char *pmu_name, const char *topic,
+			     const char *event_name, const char *event_alias,
+			     const char *scale_unit,
+			     bool deprecated, const char *event_type_desc,
+			     const char *desc, const char *long_desc,
+			     const char *encoding_desc,
+			     const char *metric_name, const char *metric_expr)
+{
+	struct json_print_state *print_state = ps;
+	bool need_sep = false;
+
+	printf("%s{\n", print_state->need_sep ? ",\n" : "");
+	print_state->need_sep = true;
+	if (pmu_name) {
+		fix_escape_printf("\t\"Unit\": \"%s\"", pmu_name);
+		need_sep = true;
+	}
+	if (topic) {
+		fix_escape_printf("%s\t\"Topic\": \"%s\"", need_sep ? ",\n" : "", topic);
+		need_sep = true;
+	}
+	if (event_name) {
+		fix_escape_printf("%s\t\"EventName\": \"%s\"", need_sep ? ",\n" : "", event_name);
+		need_sep = true;
+	}
+	if (event_alias && strlen(event_alias)) {
+		fix_escape_printf("%s\t\"EventAlias\": \"%s\"", need_sep ? ",\n" : "", event_alias);
+		need_sep = true;
+	}
+	if (scale_unit && strlen(scale_unit)) {
+		fix_escape_printf("%s\t\"ScaleUnit\": \"%s\"", need_sep ? ",\n" : "",
+				  scale_unit);
+		need_sep = true;
+	}
+	if (event_type_desc) {
+		fix_escape_printf("%s\t\"EventType\": \"%s\"", need_sep ? ",\n" : "",
+				  event_type_desc);
+		need_sep = true;
+	}
+	if (deprecated) {
+		fix_escape_printf("%s\t\"Deprecated\": \"%s\"", need_sep ? ",\n" : "",
+				  deprecated ? "1" : "0");
+		need_sep = true;
+	}
+	if (desc) {
+		fix_escape_printf("%s\t\"BriefDescription\": \"%s\"", need_sep ? ",\n" : "", desc);
+		need_sep = true;
+	}
+	if (long_desc) {
+		fix_escape_printf("%s\t\"PublicDescription\": \"%s\"", need_sep ? ",\n" : "",
+				  long_desc);
+		need_sep = true;
+	}
+	if (encoding_desc) {
+		fix_escape_printf("%s\t\"Encoding\": \"%s\"", need_sep ? ",\n" : "", encoding_desc);
+		need_sep = true;
+	}
+	if (metric_name) {
+		fix_escape_printf("%s\t\"MetricName\": \"%s\"", need_sep ? ",\n" : "", metric_name);
+		need_sep = true;
+	}
+	if (metric_expr) {
+		fix_escape_printf("%s\t\"MetricExpr\": \"%s\"", need_sep ? ",\n" : "", metric_expr);
+		need_sep = true;
+	}
+	printf("%s}", need_sep ? "\n" : "");
+}
+
+static void json_print_metric(void *ps __maybe_unused, const char *group,
+			      const char *name, const char *desc,
+			      const char *long_desc, const char *expr,
+			      const char *unit)
+{
+	struct json_print_state *print_state = ps;
+	bool need_sep = false;
+
+	printf("%s{\n", print_state->need_sep ? ",\n" : "");
+	print_state->need_sep = true;
+	if (group) {
+		fix_escape_printf("\t\"MetricGroup\": \"%s\"", group);
+		need_sep = true;
+	}
+	if (name) {
+		fix_escape_printf("%s\t\"MetricName\": \"%s\"", need_sep ? ",\n" : "", name);
+		need_sep = true;
+	}
+	if (expr) {
+		fix_escape_printf("%s\t\"MetricExpr\": \"%s\"", need_sep ? ",\n" : "", expr);
+		need_sep = true;
+	}
+	if (unit) {
+		fix_escape_printf("%s\t\"ScaleUnit\": \"%s\"", need_sep ? ",\n" : "", unit);
+		need_sep = true;
+	}
+	if (desc) {
+		fix_escape_printf("%s\t\"BriefDescription\": \"%s\"", need_sep ? ",\n" : "", desc);
+		need_sep = true;
+	}
+	if (long_desc) {
+		fix_escape_printf("%s\t\"PublicDescription\": \"%s\"", need_sep ? ",\n" : "",
+				  long_desc);
+		need_sep = true;
+	}
+	printf("%s}", need_sep ? "\n" : "");
+}
+
 int cmd_list(int argc, const char **argv)
 {
 	int i, ret = 0;
-	struct print_state ps = {};
+	struct print_state default_ps = {};
+	struct print_state json_ps = {};
+	void *ps = &default_ps;
 	struct print_callbacks print_cb = {
 		.print_start = default_print_start,
 		.print_end = default_print_end,
@@ -240,15 +407,17 @@ int cmd_list(int argc, const char **argv)
 	};
 	const char *hybrid_name = NULL;
 	const char *unit_name = NULL;
+	bool json = false;
 	struct option list_options[] = {
-		OPT_BOOLEAN(0, "raw-dump", &ps.name_only, "Dump raw events"),
-		OPT_BOOLEAN('d', "desc", &ps.desc,
+		OPT_BOOLEAN(0, "raw-dump", &default_ps.name_only, "Dump raw events"),
+		OPT_BOOLEAN('j', "json", &json, "JSON encode events and metrics"),
+		OPT_BOOLEAN('d', "desc", &default_ps.desc,
 			    "Print extra event descriptions. --no-desc to not print."),
-		OPT_BOOLEAN('v', "long-desc", &ps.long_desc,
+		OPT_BOOLEAN('v', "long-desc", &default_ps.long_desc,
 			    "Print longer event descriptions."),
-		OPT_BOOLEAN(0, "details", &ps.detailed,
+		OPT_BOOLEAN(0, "details", &default_ps.detailed,
 			    "Print information on the perf event names and expressions used internally by events."),
-		OPT_BOOLEAN(0, "deprecated", &ps.deprecated,
+		OPT_BOOLEAN(0, "deprecated", &default_ps.deprecated,
 			    "Print deprecated events."),
 		OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
 			   "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
@@ -272,28 +441,37 @@ int cmd_list(int argc, const char **argv)
 
 	setup_pager();
 
-	if (!ps.name_only)
+	if (!default_ps.name_only)
 		setup_pager();
 
-	ps.desc = !ps.long_desc;
-	ps.last_topic = strdup("");
-	assert(ps.last_topic);
-	ps.visited_metrics = strlist__new(NULL, NULL);
-	assert(ps.visited_metrics);
-	if (unit_name)
-		ps.pmu_glob = strdup(unit_name);
-	else if (hybrid_name) {
-		ps.pmu_glob = perf_pmu__hybrid_type_to_pmu(hybrid_name);
-		if (!ps.pmu_glob)
-			pr_warning("WARNING: hybrid cputype is not supported!\n");
+	if (json) {
+		print_cb = (struct print_callbacks){
+			.print_start = json_print_start,
+			.print_end = json_print_end,
+			.print_event = json_print_event,
+			.print_metric = json_print_metric,
+		};
+		ps = &json_ps;
+	} else {
+		default_ps.desc = !default_ps.long_desc;
+		default_ps.last_topic = strdup("");
+		assert(default_ps.last_topic);
+		default_ps.visited_metrics = strlist__new(NULL, NULL);
+		assert(default_ps.visited_metrics);
+		if (unit_name)
+			default_ps.pmu_glob = strdup(unit_name);
+		else if (hybrid_name) {
+			default_ps.pmu_glob = perf_pmu__hybrid_type_to_pmu(hybrid_name);
+			if (!default_ps.pmu_glob)
+				pr_warning("WARNING: hybrid cputype is not supported!\n");
+		}
 	}
-
 	print_cb.print_start(&ps);
 
 	if (argc == 0) {
-		ps.metrics = true;
-		ps.metricgroups = true;
-		print_events(&print_cb, &ps);
+		default_ps.metrics = true;
+		default_ps.metricgroups = true;
+		print_events(&print_cb, ps);
 		goto out;
 	}
 
@@ -301,32 +479,32 @@ int cmd_list(int argc, const char **argv)
 		char *sep, *s;
 
 		if (strcmp(argv[i], "tracepoint") == 0)
-			print_tracepoint_events(&print_cb, &ps);
+			print_tracepoint_events(&print_cb, ps);
 		else if (strcmp(argv[i], "hw") == 0 ||
 			 strcmp(argv[i], "hardware") == 0)
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_HARDWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_HARDWARE,
 					event_symbols_hw, PERF_COUNT_HW_MAX);
 		else if (strcmp(argv[i], "sw") == 0 ||
 			 strcmp(argv[i], "software") == 0) {
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_SOFTWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, &ps);
+			print_tool_events(&print_cb, ps);
 		} else if (strcmp(argv[i], "cache") == 0 ||
 			 strcmp(argv[i], "hwcache") == 0)
-			print_hwcache_events(&print_cb, &ps);
+			print_hwcache_events(&print_cb, ps);
 		else if (strcmp(argv[i], "pmu") == 0)
-			print_pmu_events(&print_cb, &ps);
+			print_pmu_events(&print_cb, ps);
 		else if (strcmp(argv[i], "sdt") == 0)
-			print_sdt_events(&print_cb, &ps);
+			print_sdt_events(&print_cb, ps);
 		else if (strcmp(argv[i], "metric") == 0 || strcmp(argv[i], "metrics") == 0) {
-			ps.metricgroups = false;
-			ps.metrics = true;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.metricgroups = false;
+			default_ps.metrics = true;
+			metricgroup__print(&print_cb, ps);
 		} else if (strcmp(argv[i], "metricgroup") == 0 ||
 			   strcmp(argv[i], "metricgroups") == 0) {
-			ps.metricgroups = true;
-			ps.metrics = false;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.metricgroups = true;
+			default_ps.metrics = false;
+			metricgroup__print(&print_cb, ps);
 		} else if ((sep = strchr(argv[i], ':')) != NULL) {
 			int sep_idx;
 
@@ -338,41 +516,41 @@ int cmd_list(int argc, const char **argv)
 			}
 
 			s[sep_idx] = '\0';
-			ps.pmu_glob = s;
-			ps.event_glob = s + sep_idx + 1;
-			print_tracepoint_events(&print_cb, &ps);
-			print_sdt_events(&print_cb, &ps);
-			ps.metrics = true;
-			ps.metricgroups = true;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.pmu_glob = s;
+			default_ps.event_glob = s + sep_idx + 1;
+			print_tracepoint_events(&print_cb, ps);
+			print_sdt_events(&print_cb, ps);
+			default_ps.metrics = true;
+			default_ps.metricgroups = true;
+			metricgroup__print(&print_cb, ps);
 			free(s);
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
 				printf("Critical: Not enough memory! Trying to continue...\n");
 				continue;
 			}
-			ps.event_glob = s;
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_HARDWARE,
+			default_ps.event_glob = s;
+			print_symbol_events(&print_cb, ps, PERF_TYPE_HARDWARE,
 					event_symbols_hw, PERF_COUNT_HW_MAX);
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_SOFTWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, &ps);
-			print_hwcache_events(&print_cb, &ps);
-			print_pmu_events(&print_cb, &ps);
-			print_tracepoint_events(&print_cb, &ps);
-			print_sdt_events(&print_cb, &ps);
-			ps.metrics = true;
-			ps.metricgroups = true;
-			metricgroup__print(&print_cb, &ps);
+			print_tool_events(&print_cb, ps);
+			print_hwcache_events(&print_cb, ps);
+			print_pmu_events(&print_cb, ps);
+			print_tracepoint_events(&print_cb, ps);
+			print_sdt_events(&print_cb, ps);
+			default_ps.metrics = true;
+			default_ps.metricgroups = true;
+			metricgroup__print(&print_cb, ps);
 			free(s);
 		}
 	}
 
 out:
-	print_cb.print_end(&ps);
-	free(ps.pmu_glob);
-	free(ps.last_topic);
-	free(ps.last_metricgroups);
-	strlist__delete(ps.visited_metrics);
+	print_cb.print_end(ps);
+	free(default_ps.pmu_glob);
+	free(default_ps.last_topic);
+	free(default_ps.last_metricgroups);
+	strlist__delete(default_ps.visited_metrics);
 	return ret;
 }
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 11:23 a.m. UTC | #2
Em Tue, Nov 15, 2022 at 10:44:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Nov 14, 2022 at 01:07:23PM -0800, Ian Rogers escreveu:
> > Output events and metrics in a json format by overriding the print
> > callbacks. Currently other command line options aren't supported and
> > metrics are repeated once per metric group.
> 
> Applied the patch with a few fixes and added this to the last cset:

There is a problem, detected using 'perf test', the last one:

[root@quaco ~]# perf test "trace + vfs_getname"
112: Check open filename arg using perf trace + vfs_getname          : FAILED!
[root@quaco ~]# perf test -v "trace + vfs_getname"
112: Check open filename arg using perf trace + vfs_getname          :
--- start ---
test child forked, pid 611667
test child finished with -1
---- end ----
Check open filename arg using perf trace + vfs_getname: FAILED!
[root@quaco ~]#

Verbose didn't help, so I looked at the shell script for that test and
the problem is here:

[root@quaco ~]# perf list syscalls:sys_enter_open*
Segmentation fault (core dumped)
^C
[root@quaco ~]#
[root@quaco ~]#
[root@quaco ~]# gdb perf
GNU gdb (GDB) Fedora 12.1-2.fc36
(gdb) run list syscalls:sys_enter_open*
Starting program: /root/bin/perf list syscalls:sys_enter_open*
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[Detaching after fork from child process 611757]

Program received signal SIGSEGV, Segmentation fault.
0x00000000004e742b in __match_glob (str=0x0, pat=0xe44900 "syscalls", ignore_space=ignore_space@entry=false, case_ins=case_ins@entry=false) at util/string.c:113
113		while (*str && *pat && *pat != '*') {
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.8-11.fc36.x86_64 cyrus-sasl-lib-2.1.27-18.fc36.x86_64 elfutils-debuginfod-client-0.187-4.fc36.x86_64 elfutils-libelf-0.187-4.fc36.x86_64 elfutils-libs-0.187-4.fc36.x86_64 glib2-2.72.3-1.fc36.x86_64 glibc-2.35-20.fc36.x86_64 keyutils-libs-1.6.1-4.fc36.x86_64 krb5-libs-1.19.2-11.fc36.x86_64 libbabeltrace-1.5.8-9.fc36.x86_64 libbrotli-1.0.9-7.fc36.x86_64 libcap-2.48-4.fc36.x86_64 libcom_err-1.46.5-2.fc36.x86_64 libcurl-7.82.0-8.fc36.x86_64 libevent-2.1.12-6.fc36.x86_64 libgcc-12.2.1-2.fc36.x86_64 libidn2-2.3.4-1.fc36.x86_64 libnghttp2-1.46.0-2.fc36.x86_64 libpsl-0.21.1-5.fc36.x86_64 libselinux-3.3-4.fc36.x86_64 libssh-0.9.6-4.fc36.x86_64 libunistring-1.0-1.fc36.x86_64 libunwind-1.6.2-2.fc36.x86_64 libuuid-2.38-1.fc36.x86_64 libxcrypt-4.4.28-1.fc36.x86_64 libzstd-1.5.2-2.fc36.x86_64 numactl-libs-2.0.14-5.fc36.x86_64 openldap-2.6.3-1.fc36.x86_64 openssl-libs-3.0.5-1.fc36.x86_64 pcre-8.45-1.fc36.1.x86_64 perl-libs-5.34.1-486.fc36.x86_64 popt-1.18-7.fc36.x86_64 python3-libs-3.10.7-1.fc36.x86_64 slang-2.3.2-11.fc36.x86_64 xz-libs-5.2.5-9.fc36.x86_64 zlib-1.2.11-33.fc36.x86_64
(gdb) bt
#0  0x00000000004e742b in __match_glob (str=0x0, pat=0xe44900 "syscalls", ignore_space=ignore_space@entry=false, case_ins=case_ins@entry=false) at util/string.c:113
#1  0x00000000004e7830 in strglobmatch (str=<optimized out>, pat=<optimized out>) at util/string.c:172
#2  0x00000000004241ea in default_print_event (ps=0x7fffffffd370, pmu_name=<optimized out>, topic=0x0, event_name=0x7fffffffc2d0 "alarmtimer:alarmtimer_cancel", event_alias=0x0, scale_unit=<optimized out>, deprecated=<optimized out>, event_type_desc=<optimized out>,
    desc=<optimized out>, long_desc=<optimized out>, encoding_desc=<optimized out>, metric_name=<optimized out>, metric_expr=<optimized out>) at builtin-list.c:107
#3  0x00000000004e3224 in print_tracepoint_events (print_cb=print_cb@entry=0x7fffffffd350, print_state=0x7fffffffd370) at util/print-events.c:94
#4  0x0000000000425342 in cmd_list (argc=<optimized out>, argv=<optimized out>) at builtin-list.c:521
#5  0x00000000004b4579 in run_builtin (p=p@entry=0xdb0490 <commands+240>, argc=argc@entry=2, argv=argv@entry=0x7fffffffdbb0) at perf.c:322
#6  0x000000000040f506 in handle_internal_command (argv=<optimized out>, argc=<optimized out>) at perf.c:376
#7  run_argv (argv=<optimized out>, argcp=<optimized out>) at perf.c:420
#8  main (argc=2, argv=0x7fffffffdbb0) at perf.c:550
(gdb)

Trying to fix this now.

Please always run 'perf test' before and after your patches and before
sending it upstream.

- Arnaldo
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 11:35 a.m. UTC | #3
Em Wed, Nov 16, 2022 at 08:23:07AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 15, 2022 at 10:44:54AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Nov 14, 2022 at 01:07:23PM -0800, Ian Rogers escreveu:
> > > Output events and metrics in a json format by overriding the print
> > > callbacks. Currently other command line options aren't supported and
> > > metrics are repeated once per metric group.
> > 
> > Applied the patch with a few fixes and added this to the last cset:
> 
> There is a problem, detected using 'perf test', the last one:
> 
> [root@quaco ~]# perf test "trace + vfs_getname"
> 112: Check open filename arg using perf trace + vfs_getname          : FAILED!
> [root@quaco ~]# perf test -v "trace + vfs_getname"
> 112: Check open filename arg using perf trace + vfs_getname          :
> --- start ---
> test child forked, pid 611667
> test child finished with -1
> ---- end ----
> Check open filename arg using perf trace + vfs_getname: FAILED!
> [root@quaco ~]#
> 
> Verbose didn't help, so I looked at the shell script for that test and
> the problem is here:
> 
> [root@quaco ~]# perf list syscalls:sys_enter_open*
> Segmentation fault (core dumped)
> ^C
> [root@quaco ~]#
> [root@quaco ~]#
> [root@quaco ~]# gdb perf
> GNU gdb (GDB) Fedora 12.1-2.fc36
> (gdb) run list syscalls:sys_enter_open*
> Starting program: /root/bin/perf list syscalls:sys_enter_open*
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> [Detaching after fork from child process 611757]
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004e742b in __match_glob (str=0x0, pat=0xe44900 "syscalls", ignore_space=ignore_space@entry=false, case_ins=case_ins@entry=false) at util/string.c:113
> 113		while (*str && *pat && *pat != '*') {
> Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.8-11.fc36.x86_64 cyrus-sasl-lib-2.1.27-18.fc36.x86_64 elfutils-debuginfod-client-0.187-4.fc36.x86_64 elfutils-libelf-0.187-4.fc36.x86_64 elfutils-libs-0.187-4.fc36.x86_64 glib2-2.72.3-1.fc36.x86_64 glibc-2.35-20.fc36.x86_64 keyutils-libs-1.6.1-4.fc36.x86_64 krb5-libs-1.19.2-11.fc36.x86_64 libbabeltrace-1.5.8-9.fc36.x86_64 libbrotli-1.0.9-7.fc36.x86_64 libcap-2.48-4.fc36.x86_64 libcom_err-1.46.5-2.fc36.x86_64 libcurl-7.82.0-8.fc36.x86_64 libevent-2.1.12-6.fc36.x86_64 libgcc-12.2.1-2.fc36.x86_64 libidn2-2.3.4-1.fc36.x86_64 libnghttp2-1.46.0-2.fc36.x86_64 libpsl-0.21.1-5.fc36.x86_64 libselinux-3.3-4.fc36.x86_64 libssh-0.9.6-4.fc36.x86_64 libunistring-1.0-1.fc36.x86_64 libunwind-1.6.2-2.fc36.x86_64 libuuid-2.38-1.fc36.x86_64 libxcrypt-4.4.28-1.fc36.x86_64 libzstd-1.5.2-2.fc36.x86_64 numactl-libs-2.0.14-5.fc36.x86_64 openldap-2.6.3-1.fc36.x86_64 openssl-libs-3.0.5-1.fc36.x86_64 pcre-8.45-1.fc36.1.x86_64 perl-libs-5.34.1-486.fc36.x86_64 popt-1.18-7.fc36.x86_64 python3-libs-3.10.7-1.fc36.x86_64 slang-2.3.2-11.fc36.x86_64 xz-libs-5.2.5-9.fc36.x86_64 zlib-1.2.11-33.fc36.x86_64
> (gdb) bt
> #0  0x00000000004e742b in __match_glob (str=0x0, pat=0xe44900 "syscalls", ignore_space=ignore_space@entry=false, case_ins=case_ins@entry=false) at util/string.c:113
> #1  0x00000000004e7830 in strglobmatch (str=<optimized out>, pat=<optimized out>) at util/string.c:172
> #2  0x00000000004241ea in default_print_event (ps=0x7fffffffd370, pmu_name=<optimized out>, topic=0x0, event_name=0x7fffffffc2d0 "alarmtimer:alarmtimer_cancel", event_alias=0x0, scale_unit=<optimized out>, deprecated=<optimized out>, event_type_desc=<optimized out>,
>     desc=<optimized out>, long_desc=<optimized out>, encoding_desc=<optimized out>, metric_name=<optimized out>, metric_expr=<optimized out>) at builtin-list.c:107
> #3  0x00000000004e3224 in print_tracepoint_events (print_cb=print_cb@entry=0x7fffffffd350, print_state=0x7fffffffd370) at util/print-events.c:94
> #4  0x0000000000425342 in cmd_list (argc=<optimized out>, argv=<optimized out>) at builtin-list.c:521
> #5  0x00000000004b4579 in run_builtin (p=p@entry=0xdb0490 <commands+240>, argc=argc@entry=2, argv=argv@entry=0x7fffffffdbb0) at perf.c:322
> #6  0x000000000040f506 in handle_internal_command (argv=<optimized out>, argc=<optimized out>) at perf.c:376
> #7  run_argv (argv=<optimized out>, argcp=<optimized out>) at perf.c:420
> #8  main (argc=2, argv=0x7fffffffdbb0) at perf.c:550
> (gdb)
> 
> Trying to fix this now.
> 
> Please always run 'perf test' before and after your patches and before
> sending it upstream.

Running as !root on a different machine I get some other interesting
info:

⬢[acme@toolbox perf]$  perf list syscalls:sys_enter_open*
double free or corruption (fasttop)
Aborted (core dumped)
⬢[acme@toolbox perf]$

That is:

        free(ps.pmu_glob);


at the end of cmd_list().

- Arnaldo
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 11:51 a.m. UTC | #4
Em Wed, Nov 16, 2022 at 08:35:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Please always run 'perf test' before and after your patches and before
> > sending it upstream.
 
> Running as !root on a different machine I get some other interesting
> info:
 
> ⬢[acme@toolbox perf]$  perf list syscalls:sys_enter_open*
> double free or corruption (fasttop)
> Aborted (core dumped)
> ⬢[acme@toolbox perf]$
> 
> That is:
> 
>         free(ps.pmu_glob);
> 
> 
> at the end of cmd_list().

This plus the change to default_ps in the subsequent patch cures the
double free, now working on the segfault.

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 12811fc40a3067cc..ce62a2fdcbd6ca61 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -329,6 +329,7 @@ int cmd_list(int argc, const char **argv)
 			metricgroup__print(&print_cb, &ps);
 		} else if ((sep = strchr(argv[i], ':')) != NULL) {
 			int sep_idx;
+			char *old_pmu_glob = ps.pmu_glob;
 
 			sep_idx = sep - argv[i];
 			s = strdup(argv[i]);
@@ -346,6 +347,7 @@ int cmd_list(int argc, const char **argv)
 			ps.metricgroups = true;
 			metricgroup__print(&print_cb, &ps);
 			free(s);
+			ps.pmu_glob = old_pmu_glob;
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
 				printf("Critical: Not enough memory! Trying to continue...\n");
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 12:41 p.m. UTC | #5
Em Wed, Nov 16, 2022 at 08:51:29AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Nov 16, 2022 at 08:35:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Please always run 'perf test' before and after your patches and before
> > > sending it upstream.
>  
> > Running as !root on a different machine I get some other interesting
> > info:
>  
> > ⬢[acme@toolbox perf]$  perf list syscalls:sys_enter_open*
> > double free or corruption (fasttop)
> > Aborted (core dumped)
> > ⬢[acme@toolbox perf]$
> > 
> > That is:
> > 
> >         free(ps.pmu_glob);
> > 
> > 
> > at the end of cmd_list().
> 
> This plus the change to default_ps in the subsequent patch cures the
> double free, now working on the segfault.

The segfault is "cured" with:


diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 3183c0351cda6cee..5eb9ed4a5cd0ad71 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -104,7 +104,7 @@ static void default_print_event(void *ps, const char *pmu_name, const char *topi
 	if (deprecated && !print_state->deprecated)
 		return;
 
-	if (print_state->pmu_glob && !strglobmatch(pmu_name, print_state->pmu_glob))
+	if (print_state->pmu_glob && pmu_name && !strglobmatch(pmu_name, print_state->pmu_glob))
 		return;
 
 	if (print_state->event_glob &&


----------------------------

But then:

[root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
  syscalls:sys_enter_open                            [Tracepoint event]
  syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
  syscalls:sys_enter_open_tree                       [Tracepoint event]
  syscalls:sys_enter_openat                          [Tracepoint event]
  syscalls:sys_enter_openat2                         [Tracepoint event]
[root@five ~]#

This stops working, looking into it.

- Arnaldo
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 1:10 p.m. UTC | #6
Em Wed, Nov 16, 2022 at 09:41:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> But then:
 
> [root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
>   syscalls:sys_enter_open                            [Tracepoint event]
>   syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
>   syscalls:sys_enter_open_tree                       [Tracepoint event]
>   syscalls:sys_enter_openat                          [Tracepoint event]
>   syscalls:sys_enter_openat2                         [Tracepoint event]
> [root@five ~]#
> 
> This stops working, looking into it.

Sidetracked with other stuff, please find what I have patched at
perf/perf-list-json-output in my tree.

I removed the last two patches and I'm testing so that I can push
perf/core with your series modulo the last two + Namhyung's 'perf list'
kit.

- Arnaldo
  
Arnaldo Carvalho de Melo Nov. 16, 2022, 3:21 p.m. UTC | #7
Em Wed, Nov 16, 2022 at 10:10:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Nov 16, 2022 at 09:41:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> > But then:
>  
> > [root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
> >   syscalls:sys_enter_open                            [Tracepoint event]
> >   syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
> >   syscalls:sys_enter_open_tree                       [Tracepoint event]
> >   syscalls:sys_enter_openat                          [Tracepoint event]
> >   syscalls:sys_enter_openat2                         [Tracepoint event]
> > [root@five ~]#
> > 
> > This stops working, looking into it.
> 
> Sidetracked with other stuff, please find what I have patched at
> perf/perf-list-json-output in my tree.
> 
> I removed the last two patches and I'm testing so that I can push
> perf/core with your series modulo the last two + Namhyung's 'perf list'
> kit.

I just saw you sent a patch on top of the previous one, will try and
combine stuff to remove failures from the bisect history.

- Arnaldo
  
Ian Rogers Nov. 16, 2022, 7:52 p.m. UTC | #8
On Wed, Nov 16, 2022 at 7:21 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Nov 16, 2022 at 10:10:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Nov 16, 2022 at 09:41:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > But then:
> >
> > > [root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
> > >   syscalls:sys_enter_open                            [Tracepoint event]
> > >   syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
> > >   syscalls:sys_enter_open_tree                       [Tracepoint event]
> > >   syscalls:sys_enter_openat                          [Tracepoint event]
> > >   syscalls:sys_enter_openat2                         [Tracepoint event]
> > > [root@five ~]#
> > >
> > > This stops working, looking into it.
> >
> > Sidetracked with other stuff, please find what I have patched at
> > perf/perf-list-json-output in my tree.
> >
> > I removed the last two patches and I'm testing so that I can push
> > perf/core with your series modulo the last two + Namhyung's 'perf list'
> > kit.
>
> I just saw you sent a patch on top of the previous one, will try and
> combine stuff to remove failures from the bisect history.
>
> - Arnaldo

So the failing test was skipping for me due to a lack of kernel
symbols, sorry for not spotting it. I find that the issue is resolved
with your fixes and:

```
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 30937e1dd82c..ad6cb5d2e1cc 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -107,7 +107,7 @@ static void default_print_event(void *ps, const
char *pmu_name, const char *topi
       if (deprecated && !print_state->deprecated)
               return;

-       if (print_state->pmu_glob && !strglobmatch(pmu_name,
print_state->pmu_glob))
+       if (print_state->pmu_glob && pmu_name &&
!strglobmatch(pmu_name, print_state->pmu_glob))
               return;

       if (print_state->event_glob &&
@@ -534,24 +534,18 @@ int cmd_list(int argc, const char **argv)
                       default_ps.metrics = false;
                       metricgroup__print(&print_cb, ps);
               } else if ((sep = strchr(argv[i], ':')) != NULL) {
-                       int sep_idx;
-
-                       sep_idx = sep - argv[i];
-                       s = strdup(argv[i]);
-                       if (s == NULL) {
+                       default_ps.event_glob = strdup(argv[i]);
+                       if (!default_ps.event_glob) {
                               ret = -1;
                               goto out;
                       }
-
-                       s[sep_idx] = '\0';
-                       default_ps.pmu_glob = s;
-                       default_ps.event_glob = s + sep_idx + 1;
                       print_tracepoint_events(&print_cb, ps);
                       print_sdt_events(&print_cb, ps);
                       default_ps.metrics = true;
                       default_ps.metricgroups = true;
                       metricgroup__print(&print_cb, ps);
-                       free(s);
+                       free(default_ps.event_glob);
+                       default_ps.event_glob = NULL;
```
I think this should be squashed into "perf list: Reorganize to use
callbacks". Some explanation, in porting the : glob case I'd assumed
the before the colon would be the PMU and the after the event. Doing
things caused tracepoint output to differ too much and so for
tracepoints the : is kept in the event name. So we can simplify the
matching to not be pmu and event, just use the event glob.

Thanks,
Ian

               } else {
                       if (asprintf(&s, "*%s*", argv[i]) < 0) {
                               printf("Critical: Not enough memory!
Trying to continue...\n");
  
Namhyung Kim Nov. 16, 2022, 11:04 p.m. UTC | #9
Hi,

On Tue, Nov 15, 2022 at 5:44 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Mon, Nov 14, 2022 at 01:07:23PM -0800, Ian Rogers escreveu:
> > Output events and metrics in a json format by overriding the print
> > callbacks. Currently other command line options aren't supported and
> > metrics are repeated once per metric group.
>
> Applied the patch with a few fixes and added this to the last cset:
>
> commit c9367a0658ebcfe8ab0bc4af2648f144c64b53a4
> Author: Ian Rogers <irogers@google.com>
> Date:   Mon Nov 14 13:07:23 2022 -0800
>
>     perf list: Add JSON output option
>
>     Output events and metrics in a JSON format by overriding the print
>     callbacks. Currently other command line options aren't supported and
>     metrics are repeated once per metric group.
>
>     Committer testing:
>
>       $ perf list cache
>
>       List of pre-defined events (to be used in -e or -M):
>
>         L1-dcache-load-misses                              [Hardware cache event]
>         L1-dcache-loads                                    [Hardware cache event]
>         L1-dcache-prefetches                               [Hardware cache event]
>         L1-icache-load-misses                              [Hardware cache event]
>         L1-icache-loads                                    [Hardware cache event]
>         branch-load-misses                                 [Hardware cache event]
>         branch-loads                                       [Hardware cache event]
>         dTLB-load-misses                                   [Hardware cache event]
>         dTLB-loads                                         [Hardware cache event]
>         iTLB-load-misses                                   [Hardware cache event]
>         iTLB-loads                                         [Hardware cache event]
>       $ perf list --json cache
>       [
>       {
>               "Unit": "cache",

It's confusing to call it 'unit', but we have it in the JSON metrics, sigh..

Thanks,
Namhyung


>               "EventName": "L1-dcache-load-misses",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "L1-dcache-loads",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "L1-dcache-prefetches",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "L1-icache-load-misses",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "L1-icache-loads",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "branch-load-misses",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "branch-loads",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "dTLB-load-misses",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "dTLB-loads",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "iTLB-load-misses",
>               "EventType": "Hardware cache event"
>       },
>       {
>               "Unit": "cache",
>               "EventName": "iTLB-loads",
>               "EventType": "Hardware cache event"
>       }
>       ]
>       $
>
>     Signed-off-by: Ian Rogers <irogers@google.com>
>     Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>     Cc: Caleb Biggers <caleb.biggers@intel.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Kajol Jain <kjain@linux.ibm.com>
>     Cc: Kan Liang <kan.liang@linux.intel.com>
>     Cc: Leo Yan <leo.yan@linaro.org>
>     Cc: Mark Rutland <mark.rutland@arm.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: Perry Taylor <perry.taylor@intel.com>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Ravi Bangoria <ravi.bangoria@amd.com>
>     Cc: Rob Herring <robh@kernel.org>
>     Cc: Sandipan Das <sandipan.das@amd.com>
>     Cc: Stephane Eranian <eranian@google.com>
>     Cc: Weilin Wang <weilin.wang@intel.com>
>     Cc: Xin Gao <gaoxin@cdjrlc.com>
>     Cc: Xing Zhengjun <zhengjun.xing@linux.intel.com>
>     Link: http://lore.kernel.org/lkml/20221114210723.2749751-11-irogers@google.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  
Arnaldo Carvalho de Melo Nov. 17, 2022, 4:31 p.m. UTC | #10
Em Wed, Nov 16, 2022 at 11:52:39AM -0800, Ian Rogers escreveu:
> On Wed, Nov 16, 2022 at 7:21 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Nov 16, 2022 at 10:10:53AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Wed, Nov 16, 2022 at 09:41:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > But then:
> > >
> > > > [root@five ~]# perf list syscalls:sys_enter_open* |& grep syscalls:
> > > >   syscalls:sys_enter_open                            [Tracepoint event]
> > > >   syscalls:sys_enter_open_by_handle_at               [Tracepoint event]
> > > >   syscalls:sys_enter_open_tree                       [Tracepoint event]
> > > >   syscalls:sys_enter_openat                          [Tracepoint event]
> > > >   syscalls:sys_enter_openat2                         [Tracepoint event]
> > > > [root@five ~]#
> > > >
> > > > This stops working, looking into it.
> > >
> > > Sidetracked with other stuff, please find what I have patched at
> > > perf/perf-list-json-output in my tree.
> > >
> > > I removed the last two patches and I'm testing so that I can push
> > > perf/core with your series modulo the last two + Namhyung's 'perf list'
> > > kit.
> >
> > I just saw you sent a patch on top of the previous one, will try and
> > combine stuff to remove failures from the bisect history.
> >
> > - Arnaldo
> 
> So the failing test was skipping for me due to a lack of kernel
> symbols, sorry for not spotting it. I find that the issue is resolved
> with your fixes and:
> 
> ```
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 30937e1dd82c..ad6cb5d2e1cc 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -107,7 +107,7 @@ static void default_print_event(void *ps, const
> char *pmu_name, const char *topi
>        if (deprecated && !print_state->deprecated)
>                return;
> 
> -       if (print_state->pmu_glob && !strglobmatch(pmu_name,
> print_state->pmu_glob))
> +       if (print_state->pmu_glob && pmu_name &&
> !strglobmatch(pmu_name, print_state->pmu_glob))
>                return;
> 
>        if (print_state->event_glob &&
> @@ -534,24 +534,18 @@ int cmd_list(int argc, const char **argv)
>                        default_ps.metrics = false;
>                        metricgroup__print(&print_cb, ps);
>                } else if ((sep = strchr(argv[i], ':')) != NULL) {
> -                       int sep_idx;
> -
> -                       sep_idx = sep - argv[i];
> -                       s = strdup(argv[i]);
> -                       if (s == NULL) {
> +                       default_ps.event_glob = strdup(argv[i]);
> +                       if (!default_ps.event_glob) {
>                                ret = -1;
>                                goto out;
>                        }
> -
> -                       s[sep_idx] = '\0';
> -                       default_ps.pmu_glob = s;
> -                       default_ps.event_glob = s + sep_idx + 1;
>                        print_tracepoint_events(&print_cb, ps);
>                        print_sdt_events(&print_cb, ps);
>                        default_ps.metrics = true;
>                        default_ps.metricgroups = true;
>                        metricgroup__print(&print_cb, ps);
> -                       free(s);
> +                       free(default_ps.event_glob);
> +                       default_ps.event_glob = NULL;
> ```
> I think this should be squashed into "perf list: Reorganize to use
> callbacks". Some explanation, in porting the : glob case I'd assumed
> the before the colon would be the PMU and the after the event. Doing
> things caused tracepoint output to differ too much and so for
> tracepoints the : is kept in the event name. So we can simplify the
> matching to not be pmu and event, just use the event glob.

Next time please send the patch, I did it manually and before the last
option I get:

[root@quaco ~]# perf list syscalls:sys_enter_open |& grep syscalls
  syscalls:sys_enter_open                            [Tracepoint event]
[root@quaco ~]# perf test 112
112: Check open filename arg using perf trace + vfs_getname          : Ok
[root@quaco ~]#

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 0c84fdb3ad37c1ad..7b63bc30665a7f56 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@
 #include "util/strlist.h"
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
+#include <linux/zalloc.h>
 #include <stdio.h>
 
 /**
@@ -328,25 +329,20 @@ int cmd_list(int argc, const char **argv)
 			ps.metrics = false;
 			metricgroup__print(&print_cb, &ps);
 		} else if ((sep = strchr(argv[i], ':')) != NULL) {
-			int sep_idx;
 			char *old_pmu_glob = ps.pmu_glob;
 
-			sep_idx = sep - argv[i];
-			s = strdup(argv[i]);
-			if (s == NULL) {
+			ps.event_glob = strdup(argv[i]);
+			if (!ps.event_glob) {
 				ret = -1;
 				goto out;
 			}
 
-			s[sep_idx] = '\0';
-			ps.pmu_glob = s;
-			ps.event_glob = s + sep_idx + 1;
 			print_tracepoint_events(&print_cb, &ps);
 			print_sdt_events(&print_cb, &ps);
 			ps.metrics = true;
 			ps.metricgroups = true;
 			metricgroup__print(&print_cb, &ps);
-			free(s);
+			zfree(&ps.event_glob);
 			ps.pmu_glob = old_pmu_glob;
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
  
Arnaldo Carvalho de Melo Nov. 17, 2022, 4:47 p.m. UTC | #11
Em Thu, Nov 17, 2022 at 01:31:53PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Nov 16, 2022 at 11:52:39AM -0800, Ian Rogers escreveu:
> > I think this should be squashed into "perf list: Reorganize to use
> > callbacks". Some explanation, in porting the : glob case I'd assumed
> > the before the colon would be the PMU and the after the event. Doing
> > things caused tracepoint output to differ too much and so for
> > tracepoints the : is kept in the event name. So we can simplify the
> > matching to not be pmu and event, just use the event glob.
 
> Next time please send the patch, I did it manually and before the last
> option I get:
 
> [root@quaco ~]# perf list syscalls:sys_enter_open |& grep syscalls
>   syscalls:sys_enter_open                            [Tracepoint event]
> [root@quaco ~]# perf test 112
> 112: Check open filename arg using perf trace + vfs_getname          : Ok
> [root@quaco ~]#

Ok, adjusted the last patch in the series, everything is in my
tmp.perf/core branch, will go to perf/core later today when all tests
gets passed.

Please check that what is at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tmp.perf/core

Is ok, its passing 'perf test' for me, including the one that was
failing:

[root@quaco ~]# perf test 112
112: Check open filename arg using perf trace + vfs_getname          : Ok
[root@quaco ~]#

- Arnaldo
  
Ian Rogers Nov. 18, 2022, 2:47 a.m. UTC | #12
On Thu, Nov 17, 2022 at 8:48 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Nov 17, 2022 at 01:31:53PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Nov 16, 2022 at 11:52:39AM -0800, Ian Rogers escreveu:
> > > I think this should be squashed into "perf list: Reorganize to use
> > > callbacks". Some explanation, in porting the : glob case I'd assumed
> > > the before the colon would be the PMU and the after the event. Doing
> > > things caused tracepoint output to differ too much and so for
> > > tracepoints the : is kept in the event name. So we can simplify the
> > > matching to not be pmu and event, just use the event glob.
>
> > Next time please send the patch, I did it manually and before the last
> > option I get:
>
> > [root@quaco ~]# perf list syscalls:sys_enter_open |& grep syscalls
> >   syscalls:sys_enter_open                            [Tracepoint event]
> > [root@quaco ~]# perf test 112
> > 112: Check open filename arg using perf trace + vfs_getname          : Ok
> > [root@quaco ~]#
>
> Ok, adjusted the last patch in the series, everything is in my
> tmp.perf/core branch, will go to perf/core later today when all tests
> gets passed.
>
> Please check that what is at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tmp.perf/core
>
> Is ok, its passing 'perf test' for me, including the one that was
> failing:
>
> [root@quaco ~]# perf test 112
> 112: Check open filename arg using perf trace + vfs_getname          : Ok
> [root@quaco ~]#
>
> - Arnaldo

Thanks! Looks good to me, I rebased the libpfm4 fix on it:
https://lore.kernel.org/lkml/20221118024607.409083-1-irogers@google.com/

Thanks,
Ian
  

Patch

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 44a819af573d..43263ca88ff7 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -43,6 +43,10 @@  Print deprecated events. By default the deprecated events are hidden.
 Print PMU events and metrics limited to the specific PMU name.
 (e.g. --unit cpu, --unit msr, --unit cpu_core, --unit cpu_atom)
 
+-j::
+--json::
+Output in json format.
+
 [[EVENT_MODIFIERS]]
 EVENT MODIFIERS
 ---------------
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 12811fc40a30..aec139f7fbb2 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@ 
 #include "util/strlist.h"
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
+#include <stdarg.h>
 #include <stdio.h>
 
 /**
@@ -228,10 +229,176 @@  static void default_print_metric(void *ps,
 	}
 }
 
+struct json_print_state {
+	/** Should a separator be printed prior to the next item? */
+	bool need_sep;
+};
+
+static void json_print_start(void *print_state __maybe_unused)
+{
+	printf("[\n");
+}
+
+static void json_print_end(void *ps)
+{
+	struct json_print_state *print_state = ps;
+
+	printf("%s]\n", print_state->need_sep ? "\n" : "");
+}
+
+static void fix_escape_printf(const char *fmt, ...)
+{
+	va_list args;
+	char buf[2048];
+	size_t buf_pos = 0;
+
+	va_start(args, fmt);
+	for (size_t fmt_pos = 0; fmt_pos < strlen(fmt); fmt_pos++) {
+		switch (fmt[fmt_pos]) {
+		case '%': {
+			const char *s = va_arg(args, const char*);
+
+			fmt_pos++;
+			assert(fmt[fmt_pos] == 's');
+			for (size_t s_pos = 0; s_pos < strlen(s); s_pos++) {
+				switch (s[s_pos]) {
+				case '\\':
+					__fallthrough;
+				case '\"':
+					buf[buf_pos++] = '\\';
+					assert(buf_pos < sizeof(buf));
+					__fallthrough;
+				default:
+					buf[buf_pos++] = s[s_pos];
+					assert(buf_pos < sizeof(buf));
+					break;
+				}
+			}
+			break;
+		}
+		default:
+			buf[buf_pos++] = fmt[fmt_pos];
+			assert(buf_pos < sizeof(buf));
+			break;
+		}
+	}
+	va_end(args);
+	buf[buf_pos] = '\0';
+	fputs(buf, stdout);
+}
+
+static void json_print_event(void *ps, const char *pmu_name, const char *topic,
+			     const char *event_name, const char *event_alias,
+			     const char *scale_unit,
+			     bool deprecated, const char *event_type_desc,
+			     const char *desc, const char *long_desc,
+			     const char *encoding_desc,
+			     const char *metric_name, const char *metric_expr)
+{
+	struct json_print_state *print_state = ps;
+	bool need_sep = false;
+
+	printf("%s{\n", print_state->need_sep ? ",\n" : "");
+	print_state->need_sep = true;
+	if (pmu_name) {
+		fix_escape_printf("\t\"Unit\": \"%s\"", pmu_name);
+		need_sep = true;
+	}
+	if (topic) {
+		fix_escape_printf("%s\t\"Topic\": \"%s\"", need_sep ? ",\n" : "", topic);
+		need_sep = true;
+	}
+	if (event_name) {
+		fix_escape_printf("%s\t\"EventName\": \"%s\"", need_sep ? ",\n" : "", event_name);
+		need_sep = true;
+	}
+	if (event_alias && strlen(event_alias)) {
+		fix_escape_printf("%s\t\"EventAlias\": \"%s\"", need_sep ? ",\n" : "", event_alias);
+		need_sep = true;
+	}
+	if (scale_unit && strlen(scale_unit)) {
+		fix_escape_printf("%s\t\"ScaleUnit\": \"%s\"", need_sep ? ",\n" : "",
+				  scale_unit);
+		need_sep = true;
+	}
+	if (event_type_desc) {
+		fix_escape_printf("%s\t\"EventType\": \"%s\"", need_sep ? ",\n" : "",
+				  event_type_desc);
+		need_sep = true;
+	}
+	if (deprecated) {
+		fix_escape_printf("%s\t\"Deprecated\": \"%s\"", need_sep ? ",\n" : "",
+				  deprecated ? "1" : "0");
+		need_sep = true;
+	}
+	if (desc) {
+		fix_escape_printf("%s\t\"BriefDescription\": \"%s\"", need_sep ? ",\n" : "", desc);
+		need_sep = true;
+	}
+	if (long_desc) {
+		fix_escape_printf("%s\t\"PublicDescription\": \"%s\"", need_sep ? ",\n" : "",
+				  long_desc);
+		need_sep = true;
+	}
+	if (encoding_desc) {
+		fix_escape_printf("%s\t\"Encoding\": \"%s\"", need_sep ? ",\n" : "", encoding_desc);
+		need_sep = true;
+	}
+	if (metric_name) {
+		fix_escape_printf("%s\t\"MetricName\": \"%s\"", need_sep ? ",\n" : "", metric_name);
+		need_sep = true;
+	}
+	if (metric_expr) {
+		fix_escape_printf("%s\t\"MetricExpr\": \"%s\"", need_sep ? ",\n" : "", metric_expr);
+		need_sep = true;
+	}
+	printf("%s}", need_sep ? "\n" : "");
+}
+
+static void json_print_metric(void *ps __maybe_unused, const char *group,
+			      const char *name, const char *desc,
+			      const char *long_desc, const char *expr,
+			      const char *unit)
+{
+	struct json_print_state *print_state = ps;
+	bool need_sep = false;
+
+	printf("%s{\n", print_state->need_sep ? ",\n" : "");
+	print_state->need_sep = true;
+	if (group) {
+		fix_escape_printf("\t\"MetricGroup\": \"%s\"", group);
+		need_sep = true;
+	}
+	if (name) {
+		fix_escape_printf("%s\t\"MetricName\": \"%s\"", need_sep ? ",\n" : "", name);
+		need_sep = true;
+	}
+	if (expr) {
+		fix_escape_printf("%s\t\"MetricExpr\": \"%s\"", need_sep ? ",\n" : "", expr);
+		need_sep = true;
+	}
+	if (unit) {
+		fix_escape_printf("%s\t\"ScaleUnit\": \"%s\"", need_sep ? ",\n" : "", unit);
+		need_sep = true;
+	}
+	if (desc) {
+		fix_escape_printf("%s\t\"BriefDescription\": \"%s\"", need_sep ? ",\n" : "", desc);
+		need_sep = true;
+	}
+	if (long_desc) {
+		fix_escape_printf("%s\t\"PublicDescription\": \"%s\"", need_sep ? ",\n" : "",
+				  long_desc);
+		need_sep = true;
+	}
+	printf("%s}", need_sep ? "\n" : "");
+}
+
 int cmd_list(int argc, const char **argv)
 {
 	int i, ret = 0;
-	struct print_state ps = {};
+	struct print_state default_ps = {};
+	struct print_state json_ps = {};
+	void *ps = &default_ps;
 	struct print_callbacks print_cb = {
 		.print_start = default_print_start,
 		.print_end = default_print_end,
@@ -240,15 +407,17 @@  int cmd_list(int argc, const char **argv)
 	};
 	const char *hybrid_name = NULL;
 	const char *unit_name = NULL;
+	bool json = false;
 	struct option list_options[] = {
-		OPT_BOOLEAN(0, "raw-dump", &ps.name_only, "Dump raw events"),
-		OPT_BOOLEAN('d', "desc", &ps.desc,
+		OPT_BOOLEAN(0, "raw-dump", &default_ps.name_only, "Dump raw events"),
+		OPT_BOOLEAN('j', "json", &json, "JSON encode events and metrics"),
+		OPT_BOOLEAN('d', "desc", &default_ps.desc,
 			    "Print extra event descriptions. --no-desc to not print."),
-		OPT_BOOLEAN('v', "long-desc", &ps.long_desc,
+		OPT_BOOLEAN('v', "long-desc", &default_ps.long_desc,
 			    "Print longer event descriptions."),
-		OPT_BOOLEAN(0, "details", &ps.detailed,
+		OPT_BOOLEAN(0, "details", &default_ps.detailed,
 			    "Print information on the perf event names and expressions used internally by events."),
-		OPT_BOOLEAN(0, "deprecated", &ps.deprecated,
+		OPT_BOOLEAN(0, "deprecated", &default_ps.deprecated,
 			    "Print deprecated events."),
 		OPT_STRING(0, "cputype", &hybrid_name, "hybrid cpu type",
 			   "Limit PMU or metric printing to the given hybrid PMU (e.g. core or atom)."),
@@ -272,28 +441,37 @@  int cmd_list(int argc, const char **argv)
 
 	setup_pager();
 
-	if (!ps.name_only)
+	if (!default_ps.name_only)
 		setup_pager();
 
-	ps.desc = !ps.long_desc;
-	ps.last_topic = strdup("");
-	assert(ps.last_topic);
-	ps.visited_metrics = strlist__new(NULL, NULL);
-	assert(ps.visited_metrics);
-	if (unit_name)
-		ps.pmu_glob = strdup(unit_name);
-	else if (hybrid_name) {
-		ps.pmu_glob = perf_pmu__hybrid_type_to_pmu(hybrid_name);
-		if (!ps.pmu_glob)
-			pr_warning("WARNING: hybrid cputype is not supported!\n");
+	if (json) {
+		print_cb = (struct print_callbacks){
+			.print_start = json_print_start,
+			.print_end = json_print_end,
+			.print_event = json_print_event,
+			.print_metric = json_print_metric,
+		};
+		ps = &json_ps;
+	} else {
+		default_ps.desc = !default_ps.long_desc;
+		default_ps.last_topic = strdup("");
+		assert(default_ps.last_topic);
+		default_ps.visited_metrics = strlist__new(NULL, NULL);
+		assert(default_ps.visited_metrics);
+		if (unit_name)
+			default_ps.pmu_glob = strdup(unit_name);
+		else if (hybrid_name) {
+			default_ps.pmu_glob = perf_pmu__hybrid_type_to_pmu(hybrid_name);
+			if (!default_ps.pmu_glob)
+				pr_warning("WARNING: hybrid cputype is not supported!\n");
+		}
 	}
-
 	print_cb.print_start(&ps);
 
 	if (argc == 0) {
-		ps.metrics = true;
-		ps.metricgroups = true;
-		print_events(&print_cb, &ps);
+		default_ps.metrics = true;
+		default_ps.metricgroups = true;
+		print_events(&print_cb, ps);
 		goto out;
 	}
 
@@ -301,32 +479,32 @@  int cmd_list(int argc, const char **argv)
 		char *sep, *s;
 
 		if (strcmp(argv[i], "tracepoint") == 0)
-			print_tracepoint_events(&print_cb, &ps);
+			print_tracepoint_events(&print_cb, ps);
 		else if (strcmp(argv[i], "hw") == 0 ||
 			 strcmp(argv[i], "hardware") == 0)
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_HARDWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_HARDWARE,
 					event_symbols_hw, PERF_COUNT_HW_MAX);
 		else if (strcmp(argv[i], "sw") == 0 ||
 			 strcmp(argv[i], "software") == 0) {
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_SOFTWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, &ps);
+			print_tool_events(&print_cb, ps);
 		} else if (strcmp(argv[i], "cache") == 0 ||
 			 strcmp(argv[i], "hwcache") == 0)
-			print_hwcache_events(&print_cb, &ps);
+			print_hwcache_events(&print_cb, ps);
 		else if (strcmp(argv[i], "pmu") == 0)
-			print_pmu_events(&print_cb, &ps);
+			print_pmu_events(&print_cb, ps);
 		else if (strcmp(argv[i], "sdt") == 0)
-			print_sdt_events(&print_cb, &ps);
+			print_sdt_events(&print_cb, ps);
 		else if (strcmp(argv[i], "metric") == 0 || strcmp(argv[i], "metrics") == 0) {
-			ps.metricgroups = false;
-			ps.metrics = true;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.metricgroups = false;
+			default_ps.metrics = true;
+			metricgroup__print(&print_cb, ps);
 		} else if (strcmp(argv[i], "metricgroup") == 0 ||
 			   strcmp(argv[i], "metricgroups") == 0) {
-			ps.metricgroups = true;
-			ps.metrics = false;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.metricgroups = true;
+			default_ps.metrics = false;
+			metricgroup__print(&print_cb, ps);
 		} else if ((sep = strchr(argv[i], ':')) != NULL) {
 			int sep_idx;
 
@@ -338,41 +516,41 @@  int cmd_list(int argc, const char **argv)
 			}
 
 			s[sep_idx] = '\0';
-			ps.pmu_glob = s;
-			ps.event_glob = s + sep_idx + 1;
-			print_tracepoint_events(&print_cb, &ps);
-			print_sdt_events(&print_cb, &ps);
-			ps.metrics = true;
-			ps.metricgroups = true;
-			metricgroup__print(&print_cb, &ps);
+			default_ps.pmu_glob = s;
+			default_ps.event_glob = s + sep_idx + 1;
+			print_tracepoint_events(&print_cb, ps);
+			print_sdt_events(&print_cb, ps);
+			default_ps.metrics = true;
+			default_ps.metricgroups = true;
+			metricgroup__print(&print_cb, ps);
 			free(s);
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
 				printf("Critical: Not enough memory! Trying to continue...\n");
 				continue;
 			}
-			ps.event_glob = s;
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_HARDWARE,
+			default_ps.event_glob = s;
+			print_symbol_events(&print_cb, ps, PERF_TYPE_HARDWARE,
 					event_symbols_hw, PERF_COUNT_HW_MAX);
-			print_symbol_events(&print_cb, &ps, PERF_TYPE_SOFTWARE,
+			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, &ps);
-			print_hwcache_events(&print_cb, &ps);
-			print_pmu_events(&print_cb, &ps);
-			print_tracepoint_events(&print_cb, &ps);
-			print_sdt_events(&print_cb, &ps);
-			ps.metrics = true;
-			ps.metricgroups = true;
-			metricgroup__print(&print_cb, &ps);
+			print_tool_events(&print_cb, ps);
+			print_hwcache_events(&print_cb, ps);
+			print_pmu_events(&print_cb, ps);
+			print_tracepoint_events(&print_cb, ps);
+			print_sdt_events(&print_cb, ps);
+			default_ps.metrics = true;
+			default_ps.metricgroups = true;
+			metricgroup__print(&print_cb, ps);
 			free(s);
 		}
 	}
 
 out:
-	print_cb.print_end(&ps);
-	free(ps.pmu_glob);
-	free(ps.last_topic);
-	free(ps.last_metricgroups);
-	strlist__delete(ps.visited_metrics);
+	print_cb.print_end(ps);
+	free(default_ps.pmu_glob);
+	free(default_ps.last_topic);
+	free(default_ps.last_metricgroups);
+	strlist__delete(default_ps.visited_metrics);
 	return ret;
 }