[PATCH v4 0/6] Avoid parsing tracepoint format just for id

[PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Ian Rogers]

The tracepoint format isn't needed to open an event, just the id for
the config value. Refactor the use of evsel->tp_format to use an
accessor that will lazily construct its value. In evsel__newtp_idx
read the id so the config value can be set up/used.

This allows tracepoints to be used without libtraceevent in a number
of tests. Other functionality is enabled without libtracevent, such as
mapping a tracepoint id back to its name. There may be some
performance benefit to code using tracepoints but not using the format
information.

v4. Rebase due to conflict with 9ac98662dbd3 ("perf: event: Remove deadcode")
v3. Whitespace changes, Arnaldo.
v2. Add additional error checking/handling in evsel__tp_format.

Ian Rogers (6):
  tool api fs: Correctly encode errno for read/write open failures
  perf trace-event: Constify print arguments
  perf trace-event: Always build trace-event-info.c
  perf evsel: Add/use accessor for tp_format
  perf evsel: Allow evsel__newtp without libtraceevent
  perf tests: Enable tests disabled due to tracepoint parsing

 tools/lib/api/fs/fs.c                         |   6 +-
 tools/perf/builtin-kmem.c                     |  12 +-
 tools/perf/builtin-kwork.c                    |   3 +-
 tools/perf/builtin-record.c                   |   2 -
 tools/perf/builtin-script.c                   |   9 +-
 tools/perf/builtin-trace.c                    |  79 +++++++++----
 tools/perf/tests/Build                        |   6 +-
 tools/perf/tests/builtin-test.c               |   2 -
 tools/perf/tests/parse-events.c               |  25 +---
 tools/perf/util/Build                         |   2 +-
 tools/perf/util/data-convert-bt.c             |  10 +-
 tools/perf/util/data-convert-json.c           |   8 +-
 tools/perf/util/evsel.c                       | 110 +++++++++++++-----
 tools/perf/util/evsel.h                       |   9 +-
 tools/perf/util/evsel_fprintf.c               |   4 +-
 tools/perf/util/parse-events.c                |  16 +--
 tools/perf/util/perf_event_attr_fprintf.c     |   4 -
 .../util/scripting-engines/trace-event-perl.c |   3 +-
 .../scripting-engines/trace-event-python.c    |   3 +-
 tools/perf/util/sort.c                        |  33 ++++--
 tools/perf/util/trace-event-parse.c           |   2 +-
 tools/perf/util/trace-event-scripting.c       |  10 +-
 tools/perf/util/trace-event.h                 |   2 +-
 23 files changed, 214 insertions(+), 146 deletions(-)

-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 1/6] tool api fs: Correctly encode errno for read/write open failures

[Ian Rogers]

Switch from returning -1 to -errno so that callers can determine types
of failure.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/api/fs/fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 337fde770e45..edec23406dbc 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -296,7 +296,7 @@ int filename__read_int(const char *filename, int *value)
 	int fd = open(filename, O_RDONLY), err = -1;
 
 	if (fd < 0)
-		return -1;
+		return -errno;
 
 	if (read(fd, line, sizeof(line)) > 0) {
 		*value = atoi(line);
@@ -314,7 +314,7 @@ static int filename__read_ull_base(const char *filename,
 	int fd = open(filename, O_RDONLY), err = -1;
 
 	if (fd < 0)
-		return -1;
+		return -errno;
 
 	if (read(fd, line, sizeof(line)) > 0) {
 		*value = strtoull(line, NULL, base);
@@ -372,7 +372,7 @@ int filename__write_int(const char *filename, int value)
 	char buf[64];
 
 	if (fd < 0)
-		return err;
+		return -errno;
 
 	sprintf(buf, "%d", value);
 	if (write(fd, buf, sizeof(buf)) == sizeof(buf))
-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 2/6] perf trace-event: Constify print arguments

[Ian Rogers]

Capture that these functions don't mutate their input.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/trace-event-parse.c | 2 +-
 tools/perf/util/trace-event.h       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index d97830cdbd7e..9f218ba787d8 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -99,7 +99,7 @@ unsigned long long read_size(struct tep_event *event, void *ptr, int size)
 	return tep_read_number(event->tep, ptr, size);
 }
 
-void event_format__fprintf(struct tep_event *event,
+void event_format__fprintf(const struct tep_event *event,
 			   int cpu, void *data, int size, FILE *fp)
 {
 	struct tep_record record;
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index 0e5133f1b910..4292977e1cbf 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -39,7 +39,7 @@ trace_event__tp_format(const char *sys, const char *name);
 
 struct tep_event *trace_event__tp_format_id(int id);
 
-void event_format__fprintf(struct tep_event *event,
+void event_format__fprintf(const struct tep_event *event,
 			   int cpu, void *data, int size, FILE *fp);
 
 int parse_ftrace_file(struct tep_handle *pevent, char *buf, unsigned long size);
-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 3/6] perf trace-event: Always build trace-event-info.c

[Ian Rogers]

trace-event-info.c has no libtraceevent depdendencies, always build it
and use it in builtin-record and perf_event_attr printing.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-record.c               | 2 --
 tools/perf/util/Build                     | 2 +-
 tools/perf/util/perf_event_attr_fprintf.c | 4 ----
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f83252472921..0b637cea4850 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1748,10 +1748,8 @@ static void record__init_features(struct record *rec)
 	if (rec->no_buildid)
 		perf_header__clear_feat(&session->header, HEADER_BUILD_ID);
 
-#ifdef HAVE_LIBTRACEEVENT
 	if (!have_tracepoints(&rec->evlist->core.entries))
 		perf_header__clear_feat(&session->header, HEADER_TRACING_DATA);
-#endif
 
 	if (!rec->opts.branch_stack)
 		perf_header__clear_feat(&session->header, HEADER_BRANCH_STACK);
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 1eedead5f2f2..609c69995c51 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -85,7 +85,7 @@ perf-util-y += pmu-flex.o
 perf-util-y += pmu-bison.o
 perf-util-y += tool_pmu.o
 perf-util-y += svghelper.o
-perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
+perf-util-y += trace-event-info.o
 perf-util-y += trace-event-scripting.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-parse.o
diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
index 59fbbba79697..a73c74b99a3b 100644
--- a/tools/perf/util/perf_event_attr_fprintf.c
+++ b/tools/perf/util/perf_event_attr_fprintf.c
@@ -212,7 +212,6 @@ static void __p_config_hw_cache_id(char *buf, size_t size, u64 value)
 	}
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static void __p_config_tracepoint_id(char *buf, size_t size, u64 value)
 {
 	char *str = tracepoint_id_to_name(value);
@@ -220,7 +219,6 @@ static void __p_config_tracepoint_id(char *buf, size_t size, u64 value)
 	print_id_hex(str);
 	free(str);
 }
-#endif
 
 static void __p_config_id(struct perf_pmu *pmu, char *buf, size_t size, u32 type, u64 value)
 {
@@ -238,9 +236,7 @@ static void __p_config_id(struct perf_pmu *pmu, char *buf, size_t size, u32 type
 	case PERF_TYPE_HW_CACHE:
 		return __p_config_hw_cache_id(buf, size, value);
 	case PERF_TYPE_TRACEPOINT:
-#ifdef HAVE_LIBTRACEEVENT
 		return __p_config_tracepoint_id(buf, size, value);
-#endif
 	case PERF_TYPE_RAW:
 	case PERF_TYPE_BREAKPOINT:
 	default:
-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 4/6] perf evsel: Add/use accessor for tp_format

[Ian Rogers]

Add an accessor function for tp_format. Rather than search+replace
uses try to use a variable and reuse it. Add additional NULL checks
when accessing/using the value. Make sure the PTR_ERR is nulled out on
error path in evsel__newtp_idx.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-kmem.c                     | 12 +--
 tools/perf/builtin-kwork.c                    |  3 +-
 tools/perf/builtin-script.c                   |  9 ++-
 tools/perf/builtin-trace.c                    | 79 +++++++++++++------
 tools/perf/util/data-convert-bt.c             | 10 ++-
 tools/perf/util/data-convert-json.c           |  8 +-
 tools/perf/util/evsel.c                       |  9 ++-
 tools/perf/util/evsel.h                       |  5 ++
 tools/perf/util/evsel_fprintf.c               |  4 +-
 .../util/scripting-engines/trace-event-perl.c |  3 +-
 .../scripting-engines/trace-event-python.c    |  3 +-
 tools/perf/util/sort.c                        | 33 +++++---
 tools/perf/util/trace-event-scripting.c       | 10 ++-
 13 files changed, 128 insertions(+), 60 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index a756147e2eec..0ffbb0681fd9 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -761,6 +761,7 @@ static int parse_gfp_flags(struct evsel *evsel, struct perf_sample *sample,
 	};
 	struct trace_seq seq;
 	char *str, *pos = NULL;
+	const struct tep_event *tp_format;
 
 	if (nr_gfps) {
 		struct gfp_flag key = {
@@ -772,8 +773,9 @@ static int parse_gfp_flags(struct evsel *evsel, struct perf_sample *sample,
 	}
 
 	trace_seq_init(&seq);
-	tep_print_event(evsel->tp_format->tep,
-			&seq, &record, "%s", TEP_PRINT_INFO);
+	tp_format = evsel__tp_format(evsel);
+	if (tp_format)
+		tep_print_event(tp_format->tep, &seq, &record, "%s", TEP_PRINT_INFO);
 
 	str = strtok_r(seq.buffer, " ", &pos);
 	while (str) {
@@ -2012,13 +2014,13 @@ int cmd_kmem(int argc, const char **argv)
 
 	if (kmem_page) {
 		struct evsel *evsel = evlist__find_tracepoint_by_name(session->evlist, "kmem:mm_page_alloc");
+		const struct tep_event *tp_format = evsel ? evsel__tp_format(evsel) : NULL;
 
-		if (evsel == NULL) {
+		if (tp_format == NULL) {
 			pr_err(errmsg, "page", "page");
 			goto out_delete;
 		}
-
-		kmem_page_size = tep_get_page_size(evsel->tp_format->tep);
+		kmem_page_size = tep_get_page_size(tp_format->tep);
 		symbol_conf.use_callchain = true;
 	}
 
diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index c1daf82c9b92..e564da5293b5 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -1103,7 +1103,8 @@ static char *evsel__softirq_name(struct evsel *evsel, u64 num)
 	char *name = NULL;
 	bool found = false;
 	struct tep_print_flag_sym *sym = NULL;
-	struct tep_print_arg *args = evsel->tp_format->print_fmt.args;
+	const struct tep_event *tp_format = evsel__tp_format(evsel);
+	struct tep_print_arg *args = tp_format ? tp_format->print_fmt.args : NULL;
 
 	if ((args == NULL) || (args->next == NULL))
 		return NULL;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 6b6d4472db6e..0acdb98314f8 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2289,8 +2289,13 @@ static void process_event(struct perf_script *script,
 	}
 #ifdef HAVE_LIBTRACEEVENT
 	if (PRINT_FIELD(TRACE) && sample->raw_data) {
-		event_format__fprintf(evsel->tp_format, sample->cpu,
-				      sample->raw_data, sample->raw_size, fp);
+		const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+		if (tp_format) {
+			event_format__fprintf(tp_format, sample->cpu,
+					      sample->raw_data, sample->raw_size,
+					      fp);
+		}
 	}
 #endif
 	if (attr->type == PERF_TYPE_SYNTH && PRINT_FIELD(SYNTH))
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index a661fbd870e7..35214e76c721 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -389,7 +389,12 @@ static struct syscall_arg_fmt *evsel__syscall_arg_fmt(struct evsel *evsel)
 	}
 
 	if (et->fmt == NULL) {
-		et->fmt = calloc(evsel->tp_format->format.nr_fields, sizeof(struct syscall_arg_fmt));
+		const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+		if (tp_format == NULL)
+			goto out_delete;
+
+		et->fmt = calloc(tp_format->format.nr_fields, sizeof(struct syscall_arg_fmt));
 		if (et->fmt == NULL)
 			goto out_delete;
 	}
@@ -2154,8 +2159,12 @@ static int evsel__init_tp_arg_scnprintf(struct evsel *evsel, bool *use_btf)
 	struct syscall_arg_fmt *fmt = evsel__syscall_arg_fmt(evsel);
 
 	if (fmt != NULL) {
-		syscall_arg_fmt__init_array(fmt, evsel->tp_format->format.fields, use_btf);
-		return 0;
+		const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+		if (tp_format) {
+			syscall_arg_fmt__init_array(fmt, tp_format->format.fields, use_btf);
+			return 0;
+		}
 	}
 
 	return -ENOMEM;
@@ -3026,7 +3035,8 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct evsel *evsel,
 {
 	char bf[2048];
 	size_t size = sizeof(bf);
-	struct tep_format_field *field = evsel->tp_format->format.fields;
+	const struct tep_event *tp_format = evsel__tp_format(evsel);
+	struct tep_format_field *field = tp_format ? tp_format->format.fields : NULL;
 	struct syscall_arg_fmt *arg = __evsel__syscall_arg_fmt(evsel);
 	size_t printed = 0, btf_printed;
 	unsigned long val;
@@ -3149,11 +3159,13 @@ static int trace__event_handler(struct trace *trace, struct evsel *evsel,
 
 	if (evsel__is_bpf_output(evsel)) {
 		bpf_output__fprintf(trace, sample);
-	} else if (evsel->tp_format) {
-		if (strncmp(evsel->tp_format->name, "sys_enter_", 10) ||
-		    trace__fprintf_sys_enter(trace, evsel, sample)) {
+	} else {
+		const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+		if (tp_format && (strncmp(tp_format->name, "sys_enter_", 10) ||
+				  trace__fprintf_sys_enter(trace, evsel, sample))) {
 			if (trace->libtraceevent_print) {
-				event_format__fprintf(evsel->tp_format, sample->cpu,
+				event_format__fprintf(tp_format, sample->cpu,
 						      sample->raw_data, sample->raw_size,
 						      trace->output);
 			} else {
@@ -4081,17 +4093,23 @@ static int ordered_events__deliver_event(struct ordered_events *oe,
 static struct syscall_arg_fmt *evsel__find_syscall_arg_fmt_by_name(struct evsel *evsel, char *arg,
 								   char **type)
 {
-	struct tep_format_field *field;
 	struct syscall_arg_fmt *fmt = __evsel__syscall_arg_fmt(evsel);
+	const struct tep_event *tp_format;
+
+	if (!fmt)
+		return NULL;
 
-	if (evsel->tp_format == NULL || fmt == NULL)
+	tp_format = evsel__tp_format(evsel);
+	if (!tp_format)
 		return NULL;
 
-	for (field = evsel->tp_format->format.fields; field; field = field->next, ++fmt)
+	for (const struct tep_format_field *field = tp_format->format.fields; field;
+	     field = field->next, ++fmt) {
 		if (strcmp(field->name, arg) == 0) {
 			*type = field->type;
 			return fmt;
 		}
+	}
 
 	return NULL;
 }
@@ -4844,13 +4862,18 @@ static void evsel__set_syscall_arg_fmt(struct evsel *evsel, const char *name)
 		const struct syscall_fmt *scfmt = syscall_fmt__find(name);
 
 		if (scfmt) {
-			int skip = 0;
+			const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+			if (tp_format) {
+				int skip = 0;
 
-			if (strcmp(evsel->tp_format->format.fields->name, "__syscall_nr") == 0 ||
-			    strcmp(evsel->tp_format->format.fields->name, "nr") == 0)
-				++skip;
+				if (strcmp(tp_format->format.fields->name, "__syscall_nr") == 0 ||
+				    strcmp(tp_format->format.fields->name, "nr") == 0)
+					++skip;
 
-			memcpy(fmt + skip, scfmt->arg, (evsel->tp_format->format.nr_fields - skip) * sizeof(*fmt));
+				memcpy(fmt + skip, scfmt->arg,
+				       (tp_format->format.nr_fields - skip) * sizeof(*fmt));
+			}
 		}
 	}
 }
@@ -4860,10 +4883,16 @@ static int evlist__set_syscall_tp_fields(struct evlist *evlist, bool *use_btf)
 	struct evsel *evsel;
 
 	evlist__for_each_entry(evlist, evsel) {
-		if (evsel->priv || !evsel->tp_format)
+		const struct tep_event *tp_format;
+
+		if (evsel->priv)
+			continue;
+
+		tp_format = evsel__tp_format(evsel);
+		if (!tp_format)
 			continue;
 
-		if (strcmp(evsel->tp_format->system, "syscalls")) {
+		if (strcmp(tp_format->system, "syscalls")) {
 			evsel__init_tp_arg_scnprintf(evsel, use_btf);
 			continue;
 		}
@@ -4871,20 +4900,24 @@ static int evlist__set_syscall_tp_fields(struct evlist *evlist, bool *use_btf)
 		if (evsel__init_syscall_tp(evsel))
 			return -1;
 
-		if (!strncmp(evsel->tp_format->name, "sys_enter_", 10)) {
+		if (!strncmp(tp_format->name, "sys_enter_", 10)) {
 			struct syscall_tp *sc = __evsel__syscall_tp(evsel);
 
 			if (__tp_field__init_ptr(&sc->args, sc->id.offset + sizeof(u64)))
 				return -1;
 
-			evsel__set_syscall_arg_fmt(evsel, evsel->tp_format->name + sizeof("sys_enter_") - 1);
-		} else if (!strncmp(evsel->tp_format->name, "sys_exit_", 9)) {
+			evsel__set_syscall_arg_fmt(evsel,
+						   tp_format->name + sizeof("sys_enter_") - 1);
+		} else if (!strncmp(tp_format->name, "sys_exit_", 9)) {
 			struct syscall_tp *sc = __evsel__syscall_tp(evsel);
 
-			if (__tp_field__init_uint(&sc->ret, sizeof(u64), sc->id.offset + sizeof(u64), evsel->needs_swap))
+			if (__tp_field__init_uint(&sc->ret, sizeof(u64),
+						  sc->id.offset + sizeof(u64),
+						  evsel->needs_swap))
 				return -1;
 
-			evsel__set_syscall_arg_fmt(evsel, evsel->tp_format->name + sizeof("sys_exit_") - 1);
+			evsel__set_syscall_arg_fmt(evsel,
+						   tp_format->name + sizeof("sys_exit_") - 1);
 		}
 	}
 
diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index 021e9b1d5cc5..d022d3c2955e 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -426,8 +426,9 @@ static int add_tracepoint_values(struct ctf_writer *cw,
 				 struct evsel *evsel,
 				 struct perf_sample *sample)
 {
-	struct tep_format_field *common_fields = evsel->tp_format->format.common_fields;
-	struct tep_format_field *fields        = evsel->tp_format->format.fields;
+	const struct tep_event *tp_format = evsel__tp_format(evsel);
+	struct tep_format_field *common_fields = tp_format->format.common_fields;
+	struct tep_format_field *fields        = tp_format->format.fields;
 	int ret;
 
 	ret = add_tracepoint_fields_values(cw, event_class, event,
@@ -1064,8 +1065,9 @@ static int add_tracepoint_types(struct ctf_writer *cw,
 				struct evsel *evsel,
 				struct bt_ctf_event_class *class)
 {
-	struct tep_format_field *common_fields = evsel->tp_format->format.common_fields;
-	struct tep_format_field *fields        = evsel->tp_format->format.fields;
+	const struct tep_event *tp_format = evsel__tp_format(evsel);
+	struct tep_format_field *common_fields = tp_format ? tp_format->format.common_fields : NULL;
+	struct tep_format_field *fields        = tp_format ? tp_format->format.fields : NULL;
 	int ret;
 
 	ret = add_tracepoint_fields_types(cw, common_fields, class);
diff --git a/tools/perf/util/data-convert-json.c b/tools/perf/util/data-convert-json.c
index 20bfb0884e9e..eb068f38ca81 100644
--- a/tools/perf/util/data-convert-json.c
+++ b/tools/perf/util/data-convert-json.c
@@ -230,12 +230,12 @@ static int process_sample_event(const struct perf_tool *tool,
 
 #ifdef HAVE_LIBTRACEEVENT
 	if (sample->raw_data) {
-		int i;
-		struct tep_format_field **fields;
+		struct tep_event *tp_format = evsel__tp_format(evsel);
+		struct tep_format_field **fields = tp_format ? tep_event_fields(tp_format) : NULL;
 
-		fields = tep_event_fields(evsel->tp_format);
 		if (fields) {
-			i = 0;
+			int i = 0;
+
 			while (fields[i]) {
 				struct trace_seq s;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f780e30aa259..a95db7e900d5 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -570,6 +570,7 @@ struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool
 			evsel->tp_format = trace_event__tp_format(sys, name);
 			if (IS_ERR(evsel->tp_format)) {
 				err = PTR_ERR(evsel->tp_format);
+				evsel->tp_format = NULL;
 				goto out_free;
 			}
 			attr.config = evsel->tp_format->id;
@@ -3209,12 +3210,16 @@ u16 evsel__id_hdr_size(const struct evsel *evsel)
 #ifdef HAVE_LIBTRACEEVENT
 struct tep_format_field *evsel__field(struct evsel *evsel, const char *name)
 {
-	return tep_find_field(evsel->tp_format, name);
+	struct tep_event *tp_format = evsel__tp_format(evsel);
+
+	return tp_format ? tep_find_field(tp_format, name) : NULL;
 }
 
 struct tep_format_field *evsel__common_field(struct evsel *evsel, const char *name)
 {
-	return tep_find_common_field(evsel->tp_format, name);
+	struct tep_event *tp_format = evsel__tp_format(evsel);
+
+	return tp_format ? tep_find_common_field(tp_format, name) : NULL;
 }
 
 void *evsel__rawptr(struct evsel *evsel, struct perf_sample *sample, const char *name)
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 04934a7af174..c3e53d320bf5 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -257,6 +257,11 @@ static inline struct evsel *evsel__newtp(const char *sys, const char *name)
 {
 	return evsel__newtp_idx(sys, name, 0, true);
 }
+
+static inline struct tep_event *evsel__tp_format(struct evsel *evsel)
+{
+	return evsel->tp_format;
+}
 #endif
 
 #ifdef HAVE_LIBTRACEEVENT
diff --git a/tools/perf/util/evsel_fprintf.c b/tools/perf/util/evsel_fprintf.c
index c2c0500d5da9..54c5702f8c00 100644
--- a/tools/perf/util/evsel_fprintf.c
+++ b/tools/perf/util/evsel_fprintf.c
@@ -81,13 +81,15 @@ int evsel__fprintf(struct evsel *evsel, struct perf_attr_details *details, FILE
 #ifdef HAVE_LIBTRACEEVENT
 	if (details->trace_fields) {
 		struct tep_format_field *field;
+		const struct tep_event *tp_format;
 
 		if (evsel->core.attr.type != PERF_TYPE_TRACEPOINT) {
 			printed += comma_fprintf(fp, &first, " (not a tracepoint)");
 			goto out;
 		}
 
-		field = evsel->tp_format->format.fields;
+		tp_format = evsel__tp_format(evsel);
+		field = tp_format ? tp_format->format.fields : NULL;
 		if (field == NULL) {
 			printed += comma_fprintf(fp, &first, " (no trace field)");
 			goto out;
diff --git a/tools/perf/util/scripting-engines/trace-event-perl.c b/tools/perf/util/scripting-engines/trace-event-perl.c
index e16257d5ab2c..0d63b77b5140 100644
--- a/tools/perf/util/scripting-engines/trace-event-perl.c
+++ b/tools/perf/util/scripting-engines/trace-event-perl.c
@@ -344,7 +344,7 @@ static void perl_process_tracepoint(struct perf_sample *sample,
 				    struct addr_location *al)
 {
 	struct thread *thread = al->thread;
-	struct tep_event *event = evsel->tp_format;
+	struct tep_event *event;
 	struct tep_format_field *field;
 	static char handler[256];
 	unsigned long long val;
@@ -362,6 +362,7 @@ static void perl_process_tracepoint(struct perf_sample *sample,
 	if (evsel->core.attr.type != PERF_TYPE_TRACEPOINT)
 		return;
 
+	event = evsel__tp_format(evsel);
 	if (!event) {
 		pr_debug("ug! no event found for type %" PRIu64, (u64)evsel->core.attr.config);
 		return;
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index d7183134b669..77f1ab2d3eac 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -946,7 +946,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 				      struct addr_location *al,
 				      struct addr_location *addr_al)
 {
-	struct tep_event *event = evsel->tp_format;
+	struct tep_event *event;
 	PyObject *handler, *context, *t, *obj = NULL, *callchain;
 	PyObject *dict = NULL, *all_entries_dict = NULL;
 	static char handler_name[256];
@@ -963,6 +963,7 @@ static void python_process_tracepoint(struct perf_sample *sample,
 
 	bitmap_zero(events_defined, TRACE_EVENT_TYPE_MAX);
 
+	event = evsel__tp_format(evsel);
 	if (!event) {
 		snprintf(handler_name, sizeof(handler_name),
 			 "ug! no event found for type %" PRIu64, (u64)evsel->core.attr.config);
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 168c488f0178..710839ec3f99 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1038,17 +1038,19 @@ static char *get_trace_output(struct hist_entry *he)
 		.data = he->raw_data,
 		.size = he->raw_size,
 	};
+	struct tep_event *tp_format;
 
 	evsel = hists_to_evsel(he->hists);
 
 	trace_seq_init(&seq);
-	if (symbol_conf.raw_trace) {
-		tep_print_fields(&seq, he->raw_data, he->raw_size,
-				 evsel->tp_format);
-	} else {
-		tep_print_event(evsel->tp_format->tep,
-				&seq, &rec, "%s", TEP_PRINT_INFO);
+	tp_format = evsel__tp_format(evsel);
+	if (tp_format) {
+		if (symbol_conf.raw_trace)
+			tep_print_fields(&seq, he->raw_data, he->raw_size, tp_format);
+		else
+			tep_print_event(tp_format->tep, &seq, &rec, "%s", TEP_PRINT_INFO);
 	}
+
 	/*
 	 * Trim the buffer, it starts at 4KB and we're not going to
 	 * add anything more to this buffer.
@@ -3293,9 +3295,8 @@ static int __dynamic_dimension__add(struct evsel *evsel,
 static int add_evsel_fields(struct evsel *evsel, bool raw_trace, int level)
 {
 	int ret;
-	struct tep_format_field *field;
-
-	field = evsel->tp_format->format.fields;
+	struct tep_event *tp_format = evsel__tp_format(evsel);
+	struct tep_format_field *field = tp_format ? tp_format->format.fields : NULL;
 	while (field) {
 		ret = __dynamic_dimension__add(evsel, field, raw_trace, level);
 		if (ret < 0)
@@ -3328,13 +3329,19 @@ static int add_all_matching_fields(struct evlist *evlist,
 {
 	int ret = -ESRCH;
 	struct evsel *evsel;
-	struct tep_format_field *field;
 
 	evlist__for_each_entry(evlist, evsel) {
+		struct tep_event *tp_format;
+		struct tep_format_field *field;
+
 		if (evsel->core.attr.type != PERF_TYPE_TRACEPOINT)
 			continue;
 
-		field = tep_find_any_field(evsel->tp_format, field_name);
+		tp_format = evsel__tp_format(evsel);
+		if (tp_format == NULL)
+			continue;
+
+		field = tep_find_any_field(tp_format, field_name);
 		if (field == NULL)
 			continue;
 
@@ -3416,7 +3423,9 @@ static int add_dynamic_entry(struct evlist *evlist, const char *tok,
 	if (!strcmp(field_name, "*")) {
 		ret = add_evsel_fields(evsel, raw_trace, level);
 	} else {
-		struct tep_format_field *field = tep_find_any_field(evsel->tp_format, field_name);
+		struct tep_event *tp_format = evsel__tp_format(evsel);
+		struct tep_format_field *field =
+			tp_format ? tep_find_any_field(tp_format, field_name) : NULL;
 
 		if (field == NULL) {
 			pr_debug("Cannot find event field for %s.%s\n",
diff --git a/tools/perf/util/trace-event-scripting.c b/tools/perf/util/trace-event-scripting.c
index bd0000300c77..bab893587863 100644
--- a/tools/perf/util/trace-event-scripting.c
+++ b/tools/perf/util/trace-event-scripting.c
@@ -28,12 +28,14 @@ void scripting_context__update(struct scripting_context *c,
 			       struct addr_location *al,
 			       struct addr_location *addr_al)
 {
-	c->event_data = sample->raw_data;
-	c->pevent = NULL;
 #ifdef HAVE_LIBTRACEEVENT
-	if (evsel->tp_format)
-		c->pevent = evsel->tp_format->tep;
+	const struct tep_event *tp_format = evsel__tp_format(evsel);
+
+	c->pevent = tp_format ? tp_format->tep : NULL;
+#else
+	c->pevent = NULL;
 #endif
+	c->event_data = sample->raw_data;
 	c->event = event;
 	c->sample = sample;
 	c->evsel = evsel;
-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 5/6] perf evsel: Allow evsel__newtp without libtraceevent

[Ian Rogers]

Switch from reading the tracepoint format to reading the id directly
for the evsel config. This avoids the need to initialize
libtraceevent, plugins, etc. It is sufficient for many tracepoint
commands to work like:
$ perf stat -e sched:sched_switch true

To populate evsel->tp_format, do lazy initialization using
libtraceevent in the evsel__tp_format function (the sys and name are
saved in evsel__newtp_idx for this purpose). Reading the id should be
indicative of the format failing to load, but if not an error is
reported in evsel__tp_format. This could happen for a tracepoint with
a format that fails to parse.

As tracepoints can be parsed without libtraceevent with this, remove
the associated #ifdefs in parse-events.c.

By only lazily parsing the tracepoint format information it is hoped
this will help improve the performance of code using tracepoints but
not the format information. It also cuts down on the build and ifdef
logic.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/evsel.c        | 103 ++++++++++++++++++++++++---------
 tools/perf/util/evsel.h        |  14 ++---
 tools/perf/util/parse-events.c |  16 +----
 3 files changed, 82 insertions(+), 51 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a95db7e900d5..56e19e32a852 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -543,54 +543,101 @@ struct evsel *evsel__clone(struct evsel *orig)
 	return NULL;
 }
 
+static int trace_event__id(const char *sys, const char *name)
+{
+	char *tp_dir = get_events_file(sys);
+	char path[PATH_MAX];
+	int id, err;
+
+	if (!tp_dir)
+		return -1;
+
+	scnprintf(path, PATH_MAX, "%s/%s/id", tp_dir, name);
+	put_events_file(tp_dir);
+	err = filename__read_int(path, &id);
+	if (err)
+		return err;
+
+	return id;
+}
+
 /*
  * Returns pointer with encoded error via <linux/err.h> interface.
  */
-#ifdef HAVE_LIBTRACEEVENT
 struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format)
 {
+	struct perf_event_attr attr = {
+		.type	       = PERF_TYPE_TRACEPOINT,
+		.sample_type   = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME |
+				PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD),
+	};
 	struct evsel *evsel = zalloc(perf_evsel__object.size);
-	int err = -ENOMEM;
+	int err = -ENOMEM, id = -1;
 
-	if (evsel == NULL) {
+	if (evsel == NULL)
 		goto out_err;
-	} else {
-		struct perf_event_attr attr = {
-			.type	       = PERF_TYPE_TRACEPOINT,
-			.sample_type   = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME |
-					  PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD),
-		};
 
-		if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
-			goto out_free;
 
-		event_attr_init(&attr);
+	if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
+		goto out_free;
 
-		if (format) {
-			evsel->tp_format = trace_event__tp_format(sys, name);
-			if (IS_ERR(evsel->tp_format)) {
-				err = PTR_ERR(evsel->tp_format);
-				evsel->tp_format = NULL;
-				goto out_free;
-			}
-			attr.config = evsel->tp_format->id;
-		} else {
-			attr.config = (__u64) -1;
-		}
+#ifdef HAVE_LIBTRACEEVENT
+	evsel->tp_sys = strdup(sys);
+	if (!evsel->tp_sys)
+		goto out_free;
 
+	evsel->tp_name = strdup(name);
+	if (!evsel->tp_name)
+		goto out_free;
+#endif
 
-		attr.sample_period = 1;
-		evsel__init(evsel, &attr, idx);
-	}
+	event_attr_init(&attr);
 
+	if (format) {
+		id = trace_event__id(sys, name);
+		if (id < 0) {
+			err = id;
+			goto out_free;
+		}
+	}
+	attr.config = (__u64)id;
+	attr.sample_period = 1;
+	evsel__init(evsel, &attr, idx);
 	return evsel;
 
 out_free:
 	zfree(&evsel->name);
+#ifdef HAVE_LIBTRACEEVENT
+	zfree(&evsel->tp_sys);
+	zfree(&evsel->tp_name);
+#endif
 	free(evsel);
 out_err:
 	return ERR_PTR(err);
 }
+
+#ifdef HAVE_LIBTRACEEVENT
+struct tep_event *evsel__tp_format(struct evsel *evsel)
+{
+	struct tep_event *tp_format = evsel->tp_format;
+
+	if (tp_format)
+		return tp_format;
+
+	if (evsel->core.attr.type != PERF_TYPE_TRACEPOINT)
+		return NULL;
+
+	tp_format = trace_event__tp_format(evsel->tp_sys, evsel->tp_name);
+	if (IS_ERR(tp_format)) {
+		int err = -PTR_ERR(evsel->tp_format);
+
+		pr_err("Error getting tracepoint format '%s' '%s'(%d)\n",
+			evsel__name(evsel), strerror(err), err);
+		return NULL;
+	}
+	evsel->tp_format = tp_format;
+	return evsel->tp_format;
+}
 #endif
 
 const char *const evsel__hw_names[PERF_COUNT_HW_MAX] = {
@@ -1587,6 +1634,10 @@ void evsel__exit(struct evsel *evsel)
 	perf_thread_map__put(evsel->core.threads);
 	zfree(&evsel->group_name);
 	zfree(&evsel->name);
+#ifdef HAVE_LIBTRACEEVENT
+	zfree(&evsel->tp_sys);
+	zfree(&evsel->tp_name);
+#endif
 	zfree(&evsel->filter);
 	zfree(&evsel->group_pmu_name);
 	zfree(&evsel->unit);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index c3e53d320bf5..93b6244ec302 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -59,6 +59,8 @@ struct evsel {
 		char			*group_name;
 		const char		*group_pmu_name;
 #ifdef HAVE_LIBTRACEEVENT
+		char			*tp_sys;
+		char			*tp_name;
 		struct tep_event	*tp_format;
 #endif
 		char			*filter;
@@ -247,25 +249,17 @@ int copy_config_terms(struct list_head *dst, struct list_head *src);
 void free_config_terms(struct list_head *config_terms);
 
 
-#ifdef HAVE_LIBTRACEEVENT
-struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
-
 /*
  * Returns pointer with encoded error via <linux/err.h> interface.
  */
+struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
 static inline struct evsel *evsel__newtp(const char *sys, const char *name)
 {
 	return evsel__newtp_idx(sys, name, 0, true);
 }
 
-static inline struct tep_event *evsel__tp_format(struct evsel *evsel)
-{
-	return evsel->tp_format;
-}
-#endif
-
 #ifdef HAVE_LIBTRACEEVENT
-struct tep_event *event_format__new(const char *sys, const char *name);
+struct tep_event *evsel__tp_format(struct evsel *evsel);
 #endif
 
 void evsel__init(struct evsel *evsel, struct perf_event_attr *attr, int idx);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index afeb8d815bbf..7fc1c36ef2a4 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -489,7 +489,6 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 	return found_supported ? 0 : -EINVAL;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static void tracepoint_error(struct parse_events_error *e, int err,
 			     const char *sys, const char *name, int column)
 {
@@ -644,7 +643,6 @@ static int add_tracepoint_multi_sys(struct parse_events_state *parse_state,
 	closedir(events_dir);
 	return ret;
 }
-#endif /* HAVE_LIBTRACEEVENT */
 
 size_t default_breakpoint_len(void)
 {
@@ -1066,7 +1064,6 @@ static int config_term_pmu(struct perf_event_attr *attr,
 	return config_term_common(attr, term, err);
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static int config_term_tracepoint(struct perf_event_attr *attr,
 				  struct parse_events_term *term,
 				  struct parse_events_error *err)
@@ -1111,7 +1108,6 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
 
 	return 0;
 }
-#endif
 
 static int config_attr(struct perf_event_attr *attr,
 		       const struct parse_events_terms *head,
@@ -1303,7 +1299,7 @@ int parse_events_add_tracepoint(struct parse_events_state *parse_state,
 				struct parse_events_terms *head_config, void *loc_)
 {
 	YYLTYPE *loc = loc_;
-#ifdef HAVE_LIBTRACEEVENT
+
 	if (head_config) {
 		struct perf_event_attr attr;
 
@@ -1318,16 +1314,6 @@ int parse_events_add_tracepoint(struct parse_events_state *parse_state,
 	else
 		return add_tracepoint_event(parse_state, list, sys, event,
 					    err, head_config, loc);
-#else
-	(void)parse_state;
-	(void)list;
-	(void)sys;
-	(void)event;
-	(void)head_config;
-	parse_events_error__handle(err, loc->first_column, strdup("unsupported tracepoint"),
-				strdup("libtraceevent is necessary for tracepoint support"));
-	return -1;
-#endif
 }
 
 static int __parse_events_add_numeric(struct parse_events_state *parse_state,
-- 
2.47.0.277.g8800431eea-goog

[PATCH v4 6/6] perf tests: Enable tests disabled due to tracepoint parsing

[Ian Rogers]

Tracepoint parsing required libtraceevent but no longer does. Remove
the Build logic and #ifdefs that caused the tests not to be run. Test
code that directly uses libtraceevent is still guarded.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/Build          |  6 +++---
 tools/perf/tests/builtin-test.c |  2 --
 tools/perf/tests/parse-events.c | 25 +------------------------
 3 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 01ed9335db4d..f26a668a96df 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -5,10 +5,10 @@ perf-test-y += tests-scripts.o
 perf-test-y += parse-events.o
 perf-test-y += dso-data.o
 perf-test-y += vmlinux-kallsyms.o
-perf-test-$(CONFIG_LIBTRACEEVENT) += openat-syscall.o
-perf-test-$(CONFIG_LIBTRACEEVENT) += openat-syscall-all-cpus.o
+perf-test-y += openat-syscall.o
+perf-test-y += openat-syscall-all-cpus.o
 perf-test-$(CONFIG_LIBTRACEEVENT) += openat-syscall-tp-fields.o
-perf-test-$(CONFIG_LIBTRACEEVENT) += mmap-basic.o
+perf-test-y += mmap-basic.o
 perf-test-y += perf-record.o
 perf-test-y += evsel-roundtrip-name.o
 perf-test-$(CONFIG_LIBTRACEEVENT) += evsel-tp-sched.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index d2cabaa8ad92..4c3b622130a9 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -60,11 +60,9 @@ static struct test_suite *arch_tests[] = {
 
 static struct test_suite *generic_tests[] = {
 	&suite__vmlinux_matches_kallsyms,
-#ifdef HAVE_LIBTRACEEVENT
 	&suite__openat_syscall_event,
 	&suite__openat_syscall_event_on_all_cpus,
 	&suite__basic_mmap,
-#endif
 	&suite__mem,
 	&suite__parse_events,
 	&suite__expr,
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 82a19674a38f..5ec2e5607987 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -54,8 +54,6 @@ static bool test_perf_config(const struct perf_evsel *evsel, __u64 expected_conf
 	return (evsel->attr.config & PERF_HW_EVENT_MASK) == expected_config;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
-
 #if defined(__s390x__)
 /* Return true if kvm module is available and loaded. Test this
  * and return success when trace point kvm_s390_create_vm
@@ -112,7 +110,6 @@ static int test__checkevent_tracepoint_multi(struct evlist *evlist)
 	}
 	return TEST_OK;
 }
-#endif /* HAVE_LIBTRACEEVENT */
 
 static int test__checkevent_raw(struct evlist *evlist)
 {
@@ -311,7 +308,6 @@ static int test__checkevent_breakpoint_rw(struct evlist *evlist)
 	return TEST_OK;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static int test__checkevent_tracepoint_modifier(struct evlist *evlist)
 {
 	struct evsel *evsel = evlist__first(evlist);
@@ -340,7 +336,6 @@ test__checkevent_tracepoint_multi_modifier(struct evlist *evlist)
 
 	return test__checkevent_tracepoint_multi(evlist);
 }
-#endif /* HAVE_LIBTRACEEVENT */
 
 static int test__checkevent_raw_modifier(struct evlist *evlist)
 {
@@ -629,7 +624,6 @@ static int test__checkevent_pmu(struct evlist *evlist)
 	return TEST_OK;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static int test__checkevent_list(struct evlist *evlist)
 {
 	struct evsel *evsel = evlist__first(evlist);
@@ -671,7 +665,6 @@ static int test__checkevent_list(struct evlist *evlist)
 
 	return TEST_OK;
 }
-#endif
 
 static int test__checkevent_pmu_name(struct evlist *evlist)
 {
@@ -971,7 +964,6 @@ static int test__group2(struct evlist *evlist)
 	return TEST_OK;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static int test__group3(struct evlist *evlist __maybe_unused)
 {
 	struct evsel *evsel, *group1_leader = NULL, *group2_leader = NULL;
@@ -1078,7 +1070,6 @@ static int test__group3(struct evlist *evlist __maybe_unused)
 	}
 	return TEST_OK;
 }
-#endif
 
 static int test__group4(struct evlist *evlist __maybe_unused)
 {
@@ -1813,7 +1804,6 @@ static int test__term_equal_legacy(struct evlist *evlist)
 	return TEST_OK;
 }
 
-#ifdef HAVE_LIBTRACEEVENT
 static int count_tracepoints(void)
 {
 	struct dirent *events_ent;
@@ -1867,7 +1857,6 @@ static int test__all_tracepoints(struct evlist *evlist)
 
 	return test__checkevent_tracepoint_multi(evlist);
 }
-#endif /* HAVE_LIBTRACEVENT */
 
 struct evlist_test {
 	const char *name;
@@ -1876,7 +1865,6 @@ struct evlist_test {
 };
 
 static const struct evlist_test test__events[] = {
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name  = "syscalls:sys_enter_openat",
 		.check = test__checkevent_tracepoint,
@@ -1887,7 +1875,6 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_tracepoint_multi,
 		/* 1 */
 	},
-#endif
 	{
 		.name  = "r1a",
 		.check = test__checkevent_raw,
@@ -1938,7 +1925,6 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_breakpoint_w,
 		/* 1 */
 	},
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name  = "syscalls:sys_enter_openat:k",
 		.check = test__checkevent_tracepoint_modifier,
@@ -1949,7 +1935,6 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_tracepoint_multi_modifier,
 		/* 3 */
 	},
-#endif
 	{
 		.name  = "r1a:kp",
 		.check = test__checkevent_raw_modifier,
@@ -1995,13 +1980,11 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_breakpoint_w_modifier,
 		/* 2 */
 	},
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name  = "r1,syscalls:sys_enter_openat:k,1:1:hp",
 		.check = test__checkevent_list,
 		/* 3 */
 	},
-#endif
 	{
 		.name  = "instructions:G",
 		.check = test__checkevent_exclude_host_modifier,
@@ -2032,13 +2015,11 @@ static const struct evlist_test test__events[] = {
 		.check = test__group2,
 		/* 9 */
 	},
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name  = "group1{syscalls:sys_enter_openat:H,cycles:kppp},group2{cycles,1:3}:G,instructions:u",
 		.check = test__group3,
 		/* 0 */
 	},
-#endif
 	{
 		.name  = "{cycles:u,instructions:kp}:p",
 		.check = test__group4,
@@ -2049,13 +2030,11 @@ static const struct evlist_test test__events[] = {
 		.check = test__group5,
 		/* 2 */
 	},
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name  = "*:*",
 		.check = test__all_tracepoints,
 		/* 3 */
 	},
-#endif
 	{
 		.name  = "{cycles,cache-misses:G}:H",
 		.check = test__group_gh1,
@@ -2111,7 +2090,7 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_breakpoint_len_rw_modifier,
 		/* 4 */
 	},
-#if defined(__s390x__) && defined(HAVE_LIBTRACEEVENT)
+#if defined(__s390x__)
 	{
 		.name  = "kvm-s390:kvm_s390_create_vm",
 		.check = test__checkevent_tracepoint,
@@ -2265,13 +2244,11 @@ static const struct evlist_test test__events[] = {
 		.check = test__checkevent_breakpoint_2_events,
 		/* 3 */
 	},
-#ifdef HAVE_LIBTRACEEVENT
 	{
 		.name = "9p:9p_client_req",
 		.check = test__checkevent_tracepoint,
 		/* 4 */
 	},
-#endif
 };
 
 static const struct evlist_test test__events_pmu[] = {
-- 
2.47.0.277.g8800431eea-goog

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Namhyung Kim]

On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> The tracepoint format isn't needed to open an event, just the id for
> the config value. Refactor the use of evsel->tp_format to use an
> accessor that will lazily construct its value. In evsel__newtp_idx
> read the id so the config value can be set up/used.
> 
> This allows tracepoints to be used without libtraceevent in a number
> of tests. Other functionality is enabled without libtracevent, such as
> mapping a tracepoint id back to its name. There may be some
> performance benefit to code using tracepoints but not using the format
> information.
> 
> v4. Rebase due to conflict with 9ac98662dbd3 ("perf: event: Remove deadcode")
> v3. Whitespace changes, Arnaldo.
> v2. Add additional error checking/handling in evsel__tp_format.
> 
> Ian Rogers (6):
>   tool api fs: Correctly encode errno for read/write open failures
>   perf trace-event: Constify print arguments
>   perf trace-event: Always build trace-event-info.c
>   perf evsel: Add/use accessor for tp_format
>   perf evsel: Allow evsel__newtp without libtraceevent
>   perf tests: Enable tests disabled due to tracepoint parsing

After applying this series, I'm seeing some test failures.  But I don't
understand why it affects non-tracepoint events though.

  $ sudo ./perf test -v pipe
  --- start ---
  test child forked, pid 3036123
   1bde35-1bdecc l noploop
  perf does have symbol 'noploop'
  
  Record+report pipe test
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.210 MB - ]
  [ perf record: Woken up 2 times to write data ]
  [ perf record: Captured and wrote 0.517 MB - ]
  [ perf record: Woken up 2 times to write data ]
  [ perf record: Captured and wrote 0.516 MB - ]
  Record+report pipe test [Success]
  
  Inject -B build-ids test
  0xa5c [0x17a4]: failed to process type: 80
  Error:
  failed to process sample
  Inject build-ids test [Failed - cannot find noploop function in pipe #1]
  
  Inject -b build-ids test
  0xa5c [0x17a4]: failed to process type: 80
  Error:
  failed to process sample
  Inject build-ids test [Failed - cannot find noploop function in pipe #1]
  
  Inject --buildid-all build-ids test
  0xa5c [0x17a4]: failed to process type: 80
  Error:
  failed to process sample
  Inject build-ids test [Failed - cannot find noploop function in pipe #1]
  
  Inject --mmap2-buildid-all build-ids test
  0xa5c [0x17a4]: failed to process type: 80
  Error:
  failed to process sample
  Inject build-ids test [Failed - cannot find noploop function in pipe #1]
  ---- end(-1) ----
   84: perf pipe recording and injection test                          : FAILED!
  
  $ sudo ./perf test -v Zstd
  --- start ---
  test child forked, pid 3036097
  Collecting compressed record file:
  500+0 records in
  500+0 records out
  256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
  Checking compressed events stats:
  Couldn't decompress data
  0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
  Error:
  failed to process sample
  ---- end(-1) ----
   86: Zstd perf.data compression/decompression                        : FAILED!

Thanks,
Namhyung


> 
>  tools/lib/api/fs/fs.c                         |   6 +-
>  tools/perf/builtin-kmem.c                     |  12 +-
>  tools/perf/builtin-kwork.c                    |   3 +-
>  tools/perf/builtin-record.c                   |   2 -
>  tools/perf/builtin-script.c                   |   9 +-
>  tools/perf/builtin-trace.c                    |  79 +++++++++----
>  tools/perf/tests/Build                        |   6 +-
>  tools/perf/tests/builtin-test.c               |   2 -
>  tools/perf/tests/parse-events.c               |  25 +---
>  tools/perf/util/Build                         |   2 +-
>  tools/perf/util/data-convert-bt.c             |  10 +-
>  tools/perf/util/data-convert-json.c           |   8 +-
>  tools/perf/util/evsel.c                       | 110 +++++++++++++-----
>  tools/perf/util/evsel.h                       |   9 +-
>  tools/perf/util/evsel_fprintf.c               |   4 +-
>  tools/perf/util/parse-events.c                |  16 +--
>  tools/perf/util/perf_event_attr_fprintf.c     |   4 -
>  .../util/scripting-engines/trace-event-perl.c |   3 +-
>  .../scripting-engines/trace-event-python.c    |   3 +-
>  tools/perf/util/sort.c                        |  33 ++++--
>  tools/perf/util/trace-event-parse.c           |   2 +-
>  tools/perf/util/trace-event-scripting.c       |  10 +-
>  tools/perf/util/trace-event.h                 |   2 +-
>  23 files changed, 214 insertions(+), 146 deletions(-)
> 
> -- 
> 2.47.0.277.g8800431eea-goog
> 

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Ian Rogers]

On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > Ian Rogers (6):
> >   tool api fs: Correctly encode errno for read/write open failures
> >   perf trace-event: Constify print arguments
> >   perf trace-event: Always build trace-event-info.c
> >   perf evsel: Add/use accessor for tp_format
> >   perf evsel: Allow evsel__newtp without libtraceevent
> >   perf tests: Enable tests disabled due to tracepoint parsing
>
> After applying this series, I'm seeing some test failures.  But I don't
> understand why it affects non-tracepoint events though.
>
>   $ sudo ./perf test -v pipe
>   --- start ---
>   test child forked, pid 3036123
>    1bde35-1bdecc l noploop
>   perf does have symbol 'noploop'
>
>   Record+report pipe test
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.210 MB - ]
>   [ perf record: Woken up 2 times to write data ]
>   [ perf record: Captured and wrote 0.517 MB - ]
>   [ perf record: Woken up 2 times to write data ]
>   [ perf record: Captured and wrote 0.516 MB - ]
>   Record+report pipe test [Success]
>
>   Inject -B build-ids test
>   0xa5c [0x17a4]: failed to process type: 80
>   Error:
>   failed to process sample
>   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
>
>   Inject -b build-ids test
>   0xa5c [0x17a4]: failed to process type: 80
>   Error:
>   failed to process sample
>   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
>
>   Inject --buildid-all build-ids test
>   0xa5c [0x17a4]: failed to process type: 80
>   Error:
>   failed to process sample
>   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
>
>   Inject --mmap2-buildid-all build-ids test
>   0xa5c [0x17a4]: failed to process type: 80
>   Error:
>   failed to process sample
>   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
>   ---- end(-1) ----
>    84: perf pipe recording and injection test                          : FAILED!
>
>   $ sudo ./perf test -v Zstd
>   --- start ---
>   test child forked, pid 3036097
>   Collecting compressed record file:
>   500+0 records in
>   500+0 records out
>   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
>   Checking compressed events stats:
>   Couldn't decompress data
>   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
>   Error:
>   failed to process sample
>   ---- end(-1) ----
>    86: Zstd perf.data compression/decompression                        : FAILED!
>
> Thanks,
> Namhyung

I'm not able to reproduce:
```
$ git log --oneline |head
a59bca6eb0a6 perf test: Add a runs-per-test flag
0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
7f57057c7884 perf evsel: Add/use accessor for tp_format
c27d357d2d4c perf trace-event: Always build trace-event-info.c
20bf7a2cd68a perf trace-event: Constify print arguments
f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
...
$ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
84: perf pipe recording and injection test                          : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
86: Zstd perf.data compression/decompression                        : Ok
```
Similarly not as root or if runs-per-test is 100.

Agreed that the changes are for tracepoints and these tests aren't for
tracepoints, so an interaction wouldn't be expected. If you have a
reliable reproduction perhaps you can bisect it.

Thanks,
Ian

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Namhyung Kim]

On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > Ian Rogers (6):
> > >   tool api fs: Correctly encode errno for read/write open failures
> > >   perf trace-event: Constify print arguments
> > >   perf trace-event: Always build trace-event-info.c
> > >   perf evsel: Add/use accessor for tp_format
> > >   perf evsel: Allow evsel__newtp without libtraceevent
> > >   perf tests: Enable tests disabled due to tracepoint parsing
> >
> > After applying this series, I'm seeing some test failures.  But I don't
> > understand why it affects non-tracepoint events though.
> >
> >   $ sudo ./perf test -v pipe
> >   --- start ---
> >   test child forked, pid 3036123
> >    1bde35-1bdecc l noploop
> >   perf does have symbol 'noploop'
> >
> >   Record+report pipe test
> >   [ perf record: Woken up 1 times to write data ]
> >   [ perf record: Captured and wrote 0.210 MB - ]
> >   [ perf record: Woken up 2 times to write data ]
> >   [ perf record: Captured and wrote 0.517 MB - ]
> >   [ perf record: Woken up 2 times to write data ]
> >   [ perf record: Captured and wrote 0.516 MB - ]
> >   Record+report pipe test [Success]
> >
> >   Inject -B build-ids test
> >   0xa5c [0x17a4]: failed to process type: 80
> >   Error:
> >   failed to process sample
> >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> >
> >   Inject -b build-ids test
> >   0xa5c [0x17a4]: failed to process type: 80
> >   Error:
> >   failed to process sample
> >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> >
> >   Inject --buildid-all build-ids test
> >   0xa5c [0x17a4]: failed to process type: 80
> >   Error:
> >   failed to process sample
> >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> >
> >   Inject --mmap2-buildid-all build-ids test
> >   0xa5c [0x17a4]: failed to process type: 80
> >   Error:
> >   failed to process sample
> >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> >   ---- end(-1) ----
> >    84: perf pipe recording and injection test                          : FAILED!
> >
> >   $ sudo ./perf test -v Zstd
> >   --- start ---
> >   test child forked, pid 3036097
> >   Collecting compressed record file:
> >   500+0 records in
> >   500+0 records out
> >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> >   [ perf record: Woken up 1 times to write data ]
> >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> >   Checking compressed events stats:
> >   Couldn't decompress data
> >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> >   Error:
> >   failed to process sample
> >   ---- end(-1) ----
> >    86: Zstd perf.data compression/decompression                        : FAILED!
> >
> > Thanks,
> > Namhyung
> 
> I'm not able to reproduce:
> ```
> $ git log --oneline |head
> a59bca6eb0a6 perf test: Add a runs-per-test flag
> 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> 7f57057c7884 perf evsel: Add/use accessor for tp_format
> c27d357d2d4c perf trace-event: Always build trace-event-info.c
> 20bf7a2cd68a perf trace-event: Constify print arguments
> f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> ...
> $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 84: perf pipe recording and injection test                          : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> 86: Zstd perf.data compression/decompression                        : Ok
> ```
> Similarly not as root or if runs-per-test is 100.
> 
> Agreed that the changes are for tracepoints and these tests aren't for
> tracepoints, so an interaction wouldn't be expected. If you have a
> reliable reproduction perhaps you can bisect it.

it says:

9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
Author: Ian Rogers <irogers@google.com>
Date:   Fri Nov 8 10:47:46 2024 -0800

    tool api fs: Correctly encode errno for read/write open failures
    
    Switch from returning -1 to -errno so that callers can determine types
    of failure.
    
    Signed-off-by: Ian Rogers <irogers@google.com>
    Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

 tools/lib/api/fs/fs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Thanks,
Namhyung

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Ian Rogers]

On Sat, Nov 9, 2024 at 9:04 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> > On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > > Ian Rogers (6):
> > > >   tool api fs: Correctly encode errno for read/write open failures
> > > >   perf trace-event: Constify print arguments
> > > >   perf trace-event: Always build trace-event-info.c
> > > >   perf evsel: Add/use accessor for tp_format
> > > >   perf evsel: Allow evsel__newtp without libtraceevent
> > > >   perf tests: Enable tests disabled due to tracepoint parsing
> > >
> > > After applying this series, I'm seeing some test failures.  But I don't
> > > understand why it affects non-tracepoint events though.
> > >
> > >   $ sudo ./perf test -v pipe
> > >   --- start ---
> > >   test child forked, pid 3036123
> > >    1bde35-1bdecc l noploop
> > >   perf does have symbol 'noploop'
> > >
> > >   Record+report pipe test
> > >   [ perf record: Woken up 1 times to write data ]
> > >   [ perf record: Captured and wrote 0.210 MB - ]
> > >   [ perf record: Woken up 2 times to write data ]
> > >   [ perf record: Captured and wrote 0.517 MB - ]
> > >   [ perf record: Woken up 2 times to write data ]
> > >   [ perf record: Captured and wrote 0.516 MB - ]
> > >   Record+report pipe test [Success]
> > >
> > >   Inject -B build-ids test
> > >   0xa5c [0x17a4]: failed to process type: 80
> > >   Error:
> > >   failed to process sample
> > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > >
> > >   Inject -b build-ids test
> > >   0xa5c [0x17a4]: failed to process type: 80
> > >   Error:
> > >   failed to process sample
> > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > >
> > >   Inject --buildid-all build-ids test
> > >   0xa5c [0x17a4]: failed to process type: 80
> > >   Error:
> > >   failed to process sample
> > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > >
> > >   Inject --mmap2-buildid-all build-ids test
> > >   0xa5c [0x17a4]: failed to process type: 80
> > >   Error:
> > >   failed to process sample
> > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > >   ---- end(-1) ----
> > >    84: perf pipe recording and injection test                          : FAILED!
> > >
> > >   $ sudo ./perf test -v Zstd
> > >   --- start ---
> > >   test child forked, pid 3036097
> > >   Collecting compressed record file:
> > >   500+0 records in
> > >   500+0 records out
> > >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> > >   [ perf record: Woken up 1 times to write data ]
> > >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> > >   Checking compressed events stats:
> > >   Couldn't decompress data
> > >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> > >   Error:
> > >   failed to process sample
> > >   ---- end(-1) ----
> > >    86: Zstd perf.data compression/decompression                        : FAILED!
> > >
> > > Thanks,
> > > Namhyung
> >
> > I'm not able to reproduce:
> > ```
> > $ git log --oneline |head
> > a59bca6eb0a6 perf test: Add a runs-per-test flag
> > 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> > 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> > 7f57057c7884 perf evsel: Add/use accessor for tp_format
> > c27d357d2d4c perf trace-event: Always build trace-event-info.c
> > 20bf7a2cd68a perf trace-event: Constify print arguments
> > f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> > ...
> > $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 84: perf pipe recording and injection test                          : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > 86: Zstd perf.data compression/decompression                        : Ok
> > ```
> > Similarly not as root or if runs-per-test is 100.
> >
> > Agreed that the changes are for tracepoints and these tests aren't for
> > tracepoints, so an interaction wouldn't be expected. If you have a
> > reliable reproduction perhaps you can bisect it.
>
> it says:
>
> 9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
> commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
> Author: Ian Rogers <irogers@google.com>
> Date:   Fri Nov 8 10:47:46 2024 -0800
>
>     tool api fs: Correctly encode errno for read/write open failures
>
>     Switch from returning -1 to -errno so that callers can determine types
>     of failure.
>
>     Signed-off-by: Ian Rogers <irogers@google.com>
>     Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
>  tools/lib/api/fs/fs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

So I tried to eye-ball/grep all callers to spot assumptions on the
return value like:
```
err = ...__read_int
if (err == -1)
```
Didn't spot anything.

It seems in the test log the record is failing so I ran this under
gdb, set breakpoints on the 3 modified functions and then looked up
the call stack to spot bad return value assumptions. Everything looks
good.
I then tried inject and report, the only file read by these functions
is /proc/sys/kernel/perf_event_paranoid as part of symbol
initialization (nit, this should probably be read lazily and the
restriction should really come from the perf.data file, not the
running system) and those calls look good.

The change is small and not critical for the series. It improves the
error message when reading the tracepoint id fails. So we could move
forward with the rest of the series, but that could be annoying for
tracepoint users.

If I had a reproducer I'd revert the 1 line change on each function to
find out which is causing the regression. Once you have that then you
can binary search to find the bad call by using some global counter
where the first 'n' calls use the new return value and the later use
the old value. You can then vary 'n' to binary search and find the bad
caller.

Is there any chance you can help diagnose this or help me to find the
reproducer?

Thanks,
Ian

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Namhyung Kim]

On Wed, Nov 13, 2024 at 10:06:13AM -0800, Ian Rogers wrote:
> On Sat, Nov 9, 2024 at 9:04 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> > > On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > > > Ian Rogers (6):
> > > > >   tool api fs: Correctly encode errno for read/write open failures
> > > > >   perf trace-event: Constify print arguments
> > > > >   perf trace-event: Always build trace-event-info.c
> > > > >   perf evsel: Add/use accessor for tp_format
> > > > >   perf evsel: Allow evsel__newtp without libtraceevent
> > > > >   perf tests: Enable tests disabled due to tracepoint parsing
> > > >
> > > > After applying this series, I'm seeing some test failures.  But I don't
> > > > understand why it affects non-tracepoint events though.
> > > >
> > > >   $ sudo ./perf test -v pipe
> > > >   --- start ---
> > > >   test child forked, pid 3036123
> > > >    1bde35-1bdecc l noploop
> > > >   perf does have symbol 'noploop'
> > > >
> > > >   Record+report pipe test
> > > >   [ perf record: Woken up 1 times to write data ]
> > > >   [ perf record: Captured and wrote 0.210 MB - ]
> > > >   [ perf record: Woken up 2 times to write data ]
> > > >   [ perf record: Captured and wrote 0.517 MB - ]
> > > >   [ perf record: Woken up 2 times to write data ]
> > > >   [ perf record: Captured and wrote 0.516 MB - ]
> > > >   Record+report pipe test [Success]
> > > >
> > > >   Inject -B build-ids test
> > > >   0xa5c [0x17a4]: failed to process type: 80
> > > >   Error:
> > > >   failed to process sample
> > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > >
> > > >   Inject -b build-ids test
> > > >   0xa5c [0x17a4]: failed to process type: 80
> > > >   Error:
> > > >   failed to process sample
> > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > >
> > > >   Inject --buildid-all build-ids test
> > > >   0xa5c [0x17a4]: failed to process type: 80
> > > >   Error:
> > > >   failed to process sample
> > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > >
> > > >   Inject --mmap2-buildid-all build-ids test
> > > >   0xa5c [0x17a4]: failed to process type: 80
> > > >   Error:
> > > >   failed to process sample
> > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > >   ---- end(-1) ----
> > > >    84: perf pipe recording and injection test                          : FAILED!
> > > >
> > > >   $ sudo ./perf test -v Zstd
> > > >   --- start ---
> > > >   test child forked, pid 3036097
> > > >   Collecting compressed record file:
> > > >   500+0 records in
> > > >   500+0 records out
> > > >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> > > >   [ perf record: Woken up 1 times to write data ]
> > > >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> > > >   Checking compressed events stats:
> > > >   Couldn't decompress data
> > > >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> > > >   Error:
> > > >   failed to process sample
> > > >   ---- end(-1) ----
> > > >    86: Zstd perf.data compression/decompression                        : FAILED!
> > > >
> > > > Thanks,
> > > > Namhyung
> > >
> > > I'm not able to reproduce:
> > > ```
> > > $ git log --oneline |head
> > > a59bca6eb0a6 perf test: Add a runs-per-test flag
> > > 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> > > 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> > > 7f57057c7884 perf evsel: Add/use accessor for tp_format
> > > c27d357d2d4c perf trace-event: Always build trace-event-info.c
> > > 20bf7a2cd68a perf trace-event: Constify print arguments
> > > f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> > > ...
> > > $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 84: perf pipe recording and injection test                          : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > 86: Zstd perf.data compression/decompression                        : Ok
> > > ```
> > > Similarly not as root or if runs-per-test is 100.
> > >
> > > Agreed that the changes are for tracepoints and these tests aren't for
> > > tracepoints, so an interaction wouldn't be expected. If you have a
> > > reliable reproduction perhaps you can bisect it.
> >
> > it says:
> >
> > 9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
> > commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
> > Author: Ian Rogers <irogers@google.com>
> > Date:   Fri Nov 8 10:47:46 2024 -0800
> >
> >     tool api fs: Correctly encode errno for read/write open failures
> >
> >     Switch from returning -1 to -errno so that callers can determine types
> >     of failure.
> >
> >     Signed-off-by: Ian Rogers <irogers@google.com>
> >     Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> >  tools/lib/api/fs/fs.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> So I tried to eye-ball/grep all callers to spot assumptions on the
> return value like:
> ```
> err = ...__read_int
> if (err == -1)
> ```
> Didn't spot anything.
> 
> It seems in the test log the record is failing so I ran this under
> gdb, set breakpoints on the 3 modified functions and then looked up
> the call stack to spot bad return value assumptions. Everything looks
> good.
> I then tried inject and report, the only file read by these functions
> is /proc/sys/kernel/perf_event_paranoid as part of symbol
> initialization (nit, this should probably be read lazily and the
> restriction should really come from the perf.data file, not the
> running system) and those calls look good.
> 
> The change is small and not critical for the series. It improves the
> error message when reading the tracepoint id fails. So we could move
> forward with the rest of the series, but that could be annoying for
> tracepoint users.
> 
> If I had a reproducer I'd revert the 1 line change on each function to
> find out which is causing the regression. Once you have that then you
> can binary search to find the bad call by using some global counter
> where the first 'n' calls use the new return value and the later use
> the old value. You can then vary 'n' to binary search and find the bad
> caller.
> 
> Is there any chance you can help diagnose this or help me to find the
> reproducer?

I think this depends on the system configuration.  I've debugged it
failed when it gets cpu topology:

  ...
  read int failed: /sys/devices/system/cpu/cpu112/topology/core_id (errno=2)
  read int failed: /sys/devices/system/cpu/cpu112/topology/physical_package_id (errno=2)
  read int failed: /sys/devices/system/cpu/cpu112/topology/die_id (errno=2)
  ...

Maybe it's because # online CPUs != # possible CPUs.

  $ cat /sys/devices/system/cpu/online 
  0-63
  $ cat /sys/devices/system/cpu/possible 
  0-127

There's a code like cpu__get_socket_id() to use the return value of
sysfs__read_int() directly.  And it saves the value to aggr_cpu_id which
requires exact match like in aggr_cpu_id__equal().

Thanks,
Namhyung

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Ian Rogers]

On Mon, Nov 18, 2024 at 10:31 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Nov 13, 2024 at 10:06:13AM -0800, Ian Rogers wrote:
> > On Sat, Nov 9, 2024 at 9:04 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> > > > On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > > > > Ian Rogers (6):
> > > > > >   tool api fs: Correctly encode errno for read/write open failures
> > > > > >   perf trace-event: Constify print arguments
> > > > > >   perf trace-event: Always build trace-event-info.c
> > > > > >   perf evsel: Add/use accessor for tp_format
> > > > > >   perf evsel: Allow evsel__newtp without libtraceevent
> > > > > >   perf tests: Enable tests disabled due to tracepoint parsing
> > > > >
> > > > > After applying this series, I'm seeing some test failures.  But I don't
> > > > > understand why it affects non-tracepoint events though.
> > > > >
> > > > >   $ sudo ./perf test -v pipe
> > > > >   --- start ---
> > > > >   test child forked, pid 3036123
> > > > >    1bde35-1bdecc l noploop
> > > > >   perf does have symbol 'noploop'
> > > > >
> > > > >   Record+report pipe test
> > > > >   [ perf record: Woken up 1 times to write data ]
> > > > >   [ perf record: Captured and wrote 0.210 MB - ]
> > > > >   [ perf record: Woken up 2 times to write data ]
> > > > >   [ perf record: Captured and wrote 0.517 MB - ]
> > > > >   [ perf record: Woken up 2 times to write data ]
> > > > >   [ perf record: Captured and wrote 0.516 MB - ]
> > > > >   Record+report pipe test [Success]
> > > > >
> > > > >   Inject -B build-ids test
> > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > >   Error:
> > > > >   failed to process sample
> > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > >
> > > > >   Inject -b build-ids test
> > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > >   Error:
> > > > >   failed to process sample
> > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > >
> > > > >   Inject --buildid-all build-ids test
> > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > >   Error:
> > > > >   failed to process sample
> > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > >
> > > > >   Inject --mmap2-buildid-all build-ids test
> > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > >   Error:
> > > > >   failed to process sample
> > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > >   ---- end(-1) ----
> > > > >    84: perf pipe recording and injection test                          : FAILED!
> > > > >
> > > > >   $ sudo ./perf test -v Zstd
> > > > >   --- start ---
> > > > >   test child forked, pid 3036097
> > > > >   Collecting compressed record file:
> > > > >   500+0 records in
> > > > >   500+0 records out
> > > > >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> > > > >   [ perf record: Woken up 1 times to write data ]
> > > > >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> > > > >   Checking compressed events stats:
> > > > >   Couldn't decompress data
> > > > >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> > > > >   Error:
> > > > >   failed to process sample
> > > > >   ---- end(-1) ----
> > > > >    86: Zstd perf.data compression/decompression                        : FAILED!
> > > > >
> > > > > Thanks,
> > > > > Namhyung
> > > >
> > > > I'm not able to reproduce:
> > > > ```
> > > > $ git log --oneline |head
> > > > a59bca6eb0a6 perf test: Add a runs-per-test flag
> > > > 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> > > > 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> > > > 7f57057c7884 perf evsel: Add/use accessor for tp_format
> > > > c27d357d2d4c perf trace-event: Always build trace-event-info.c
> > > > 20bf7a2cd68a perf trace-event: Constify print arguments
> > > > f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> > > > ...
> > > > $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 84: perf pipe recording and injection test                          : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > ```
> > > > Similarly not as root or if runs-per-test is 100.
> > > >
> > > > Agreed that the changes are for tracepoints and these tests aren't for
> > > > tracepoints, so an interaction wouldn't be expected. If you have a
> > > > reliable reproduction perhaps you can bisect it.
> > >
> > > it says:
> > >
> > > 9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
> > > commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
> > > Author: Ian Rogers <irogers@google.com>
> > > Date:   Fri Nov 8 10:47:46 2024 -0800
> > >
> > >     tool api fs: Correctly encode errno for read/write open failures
> > >
> > >     Switch from returning -1 to -errno so that callers can determine types
> > >     of failure.
> > >
> > >     Signed-off-by: Ian Rogers <irogers@google.com>
> > >     Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >
> > >  tools/lib/api/fs/fs.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > So I tried to eye-ball/grep all callers to spot assumptions on the
> > return value like:
> > ```
> > err = ...__read_int
> > if (err == -1)
> > ```
> > Didn't spot anything.
> >
> > It seems in the test log the record is failing so I ran this under
> > gdb, set breakpoints on the 3 modified functions and then looked up
> > the call stack to spot bad return value assumptions. Everything looks
> > good.
> > I then tried inject and report, the only file read by these functions
> > is /proc/sys/kernel/perf_event_paranoid as part of symbol
> > initialization (nit, this should probably be read lazily and the
> > restriction should really come from the perf.data file, not the
> > running system) and those calls look good.
> >
> > The change is small and not critical for the series. It improves the
> > error message when reading the tracepoint id fails. So we could move
> > forward with the rest of the series, but that could be annoying for
> > tracepoint users.
> >
> > If I had a reproducer I'd revert the 1 line change on each function to
> > find out which is causing the regression. Once you have that then you
> > can binary search to find the bad call by using some global counter
> > where the first 'n' calls use the new return value and the later use
> > the old value. You can then vary 'n' to binary search and find the bad
> > caller.
> >
> > Is there any chance you can help diagnose this or help me to find the
> > reproducer?
>
> I think this depends on the system configuration.  I've debugged it
> failed when it gets cpu topology:
>
>   ...
>   read int failed: /sys/devices/system/cpu/cpu112/topology/core_id (errno=2)
>   read int failed: /sys/devices/system/cpu/cpu112/topology/physical_package_id (errno=2)
>   read int failed: /sys/devices/system/cpu/cpu112/topology/die_id (errno=2)
>   ...
>
> Maybe it's because # online CPUs != # possible CPUs.
>
>   $ cat /sys/devices/system/cpu/online
>   0-63
>   $ cat /sys/devices/system/cpu/possible
>   0-127
>
> There's a code like cpu__get_socket_id() to use the return value of
> sysfs__read_int() directly.  And it saves the value to aggr_cpu_id which
> requires exact match like in aggr_cpu_id__equal().

So this is a latent bug. Are you working on the fix or asking me to do
it? I'm not sure why we should fail to describe the topology for
offline cores, but if this is a kernel restriction we should probably
purge those logical CPUs from the topology.

Thanks,
Ian

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Namhyung Kim]

On Mon, Nov 18, 2024 at 11:35:19AM -0800, Ian Rogers wrote:
> On Mon, Nov 18, 2024 at 10:31 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, Nov 13, 2024 at 10:06:13AM -0800, Ian Rogers wrote:
> > > On Sat, Nov 9, 2024 at 9:04 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> > > > > On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > > > > > Ian Rogers (6):
> > > > > > >   tool api fs: Correctly encode errno for read/write open failures
> > > > > > >   perf trace-event: Constify print arguments
> > > > > > >   perf trace-event: Always build trace-event-info.c
> > > > > > >   perf evsel: Add/use accessor for tp_format
> > > > > > >   perf evsel: Allow evsel__newtp without libtraceevent
> > > > > > >   perf tests: Enable tests disabled due to tracepoint parsing
> > > > > >
> > > > > > After applying this series, I'm seeing some test failures.  But I don't
> > > > > > understand why it affects non-tracepoint events though.
> > > > > >
> > > > > >   $ sudo ./perf test -v pipe
> > > > > >   --- start ---
> > > > > >   test child forked, pid 3036123
> > > > > >    1bde35-1bdecc l noploop
> > > > > >   perf does have symbol 'noploop'
> > > > > >
> > > > > >   Record+report pipe test
> > > > > >   [ perf record: Woken up 1 times to write data ]
> > > > > >   [ perf record: Captured and wrote 0.210 MB - ]
> > > > > >   [ perf record: Woken up 2 times to write data ]
> > > > > >   [ perf record: Captured and wrote 0.517 MB - ]
> > > > > >   [ perf record: Woken up 2 times to write data ]
> > > > > >   [ perf record: Captured and wrote 0.516 MB - ]
> > > > > >   Record+report pipe test [Success]
> > > > > >
> > > > > >   Inject -B build-ids test
> > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > >   Error:
> > > > > >   failed to process sample
> > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > >
> > > > > >   Inject -b build-ids test
> > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > >   Error:
> > > > > >   failed to process sample
> > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > >
> > > > > >   Inject --buildid-all build-ids test
> > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > >   Error:
> > > > > >   failed to process sample
> > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > >
> > > > > >   Inject --mmap2-buildid-all build-ids test
> > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > >   Error:
> > > > > >   failed to process sample
> > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > >   ---- end(-1) ----
> > > > > >    84: perf pipe recording and injection test                          : FAILED!
> > > > > >
> > > > > >   $ sudo ./perf test -v Zstd
> > > > > >   --- start ---
> > > > > >   test child forked, pid 3036097
> > > > > >   Collecting compressed record file:
> > > > > >   500+0 records in
> > > > > >   500+0 records out
> > > > > >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> > > > > >   [ perf record: Woken up 1 times to write data ]
> > > > > >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> > > > > >   Checking compressed events stats:
> > > > > >   Couldn't decompress data
> > > > > >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> > > > > >   Error:
> > > > > >   failed to process sample
> > > > > >   ---- end(-1) ----
> > > > > >    86: Zstd perf.data compression/decompression                        : FAILED!
> > > > > >
> > > > > > Thanks,
> > > > > > Namhyung
> > > > >
> > > > > I'm not able to reproduce:
> > > > > ```
> > > > > $ git log --oneline |head
> > > > > a59bca6eb0a6 perf test: Add a runs-per-test flag
> > > > > 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> > > > > 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> > > > > 7f57057c7884 perf evsel: Add/use accessor for tp_format
> > > > > c27d357d2d4c perf trace-event: Always build trace-event-info.c
> > > > > 20bf7a2cd68a perf trace-event: Constify print arguments
> > > > > f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> > > > > ...
> > > > > $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > ```
> > > > > Similarly not as root or if runs-per-test is 100.
> > > > >
> > > > > Agreed that the changes are for tracepoints and these tests aren't for
> > > > > tracepoints, so an interaction wouldn't be expected. If you have a
> > > > > reliable reproduction perhaps you can bisect it.
> > > >
> > > > it says:
> > > >
> > > > 9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
> > > > commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
> > > > Author: Ian Rogers <irogers@google.com>
> > > > Date:   Fri Nov 8 10:47:46 2024 -0800
> > > >
> > > >     tool api fs: Correctly encode errno for read/write open failures
> > > >
> > > >     Switch from returning -1 to -errno so that callers can determine types
> > > >     of failure.
> > > >
> > > >     Signed-off-by: Ian Rogers <irogers@google.com>
> > > >     Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > >
> > > >  tools/lib/api/fs/fs.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > So I tried to eye-ball/grep all callers to spot assumptions on the
> > > return value like:
> > > ```
> > > err = ...__read_int
> > > if (err == -1)
> > > ```
> > > Didn't spot anything.
> > >
> > > It seems in the test log the record is failing so I ran this under
> > > gdb, set breakpoints on the 3 modified functions and then looked up
> > > the call stack to spot bad return value assumptions. Everything looks
> > > good.
> > > I then tried inject and report, the only file read by these functions
> > > is /proc/sys/kernel/perf_event_paranoid as part of symbol
> > > initialization (nit, this should probably be read lazily and the
> > > restriction should really come from the perf.data file, not the
> > > running system) and those calls look good.
> > >
> > > The change is small and not critical for the series. It improves the
> > > error message when reading the tracepoint id fails. So we could move
> > > forward with the rest of the series, but that could be annoying for
> > > tracepoint users.
> > >
> > > If I had a reproducer I'd revert the 1 line change on each function to
> > > find out which is causing the regression. Once you have that then you
> > > can binary search to find the bad call by using some global counter
> > > where the first 'n' calls use the new return value and the later use
> > > the old value. You can then vary 'n' to binary search and find the bad
> > > caller.
> > >
> > > Is there any chance you can help diagnose this or help me to find the
> > > reproducer?
> >
> > I think this depends on the system configuration.  I've debugged it
> > failed when it gets cpu topology:
> >
> >   ...
> >   read int failed: /sys/devices/system/cpu/cpu112/topology/core_id (errno=2)
> >   read int failed: /sys/devices/system/cpu/cpu112/topology/physical_package_id (errno=2)
> >   read int failed: /sys/devices/system/cpu/cpu112/topology/die_id (errno=2)
> >   ...
> >
> > Maybe it's because # online CPUs != # possible CPUs.
> >
> >   $ cat /sys/devices/system/cpu/online
> >   0-63
> >   $ cat /sys/devices/system/cpu/possible
> >   0-127
> >
> > There's a code like cpu__get_socket_id() to use the return value of
> > sysfs__read_int() directly.  And it saves the value to aggr_cpu_id which
> > requires exact match like in aggr_cpu_id__equal().
> 
> So this is a latent bug. Are you working on the fix or asking me to do
> it? I'm not sure why we should fail to describe the topology for
> offline cores, but if this is a kernel restriction we should probably
> purge those logical CPUs from the topology.

It doesn't have the topology directory for the offline CPUs.

  $ ls /sys/devices/system/cpu/cpu112/
  acpi_cppc  cpufreq  cpuidle  crash_notes  crash_notes_size  driver  firmware_node  hotplug  node0  online  power  subsystem  uevent

  $ ls /sys/devices/system/cpu/cpu112/topology
  ls: cannot access '/sys/devices/system/cpu/cpu112/topology': No such file or directory

Anyway, I'm not sure if it's really needed for this change.  Maybe you
can drop the patch 1 from the series for now and tackle it later.

Thanks,
Namhyung

Re: [PATCH v4 0/6] Avoid parsing tracepoint format just for id

[Ian Rogers]

On Mon, Nov 18, 2024 at 1:57 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Mon, Nov 18, 2024 at 11:35:19AM -0800, Ian Rogers wrote:
> > On Mon, Nov 18, 2024 at 10:31 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Wed, Nov 13, 2024 at 10:06:13AM -0800, Ian Rogers wrote:
> > > > On Sat, Nov 9, 2024 at 9:04 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > >
> > > > > On Sat, Nov 09, 2024 at 08:26:20AM -0800, Ian Rogers wrote:
> > > > > > On Fri, Nov 8, 2024 at 10:45 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > > > > > On Fri, Nov 08, 2024 at 10:47:45AM -0800, Ian Rogers wrote:
> > > > > > > > Ian Rogers (6):
> > > > > > > >   tool api fs: Correctly encode errno for read/write open failures
> > > > > > > >   perf trace-event: Constify print arguments
> > > > > > > >   perf trace-event: Always build trace-event-info.c
> > > > > > > >   perf evsel: Add/use accessor for tp_format
> > > > > > > >   perf evsel: Allow evsel__newtp without libtraceevent
> > > > > > > >   perf tests: Enable tests disabled due to tracepoint parsing
> > > > > > >
> > > > > > > After applying this series, I'm seeing some test failures.  But I don't
> > > > > > > understand why it affects non-tracepoint events though.
> > > > > > >
> > > > > > >   $ sudo ./perf test -v pipe
> > > > > > >   --- start ---
> > > > > > >   test child forked, pid 3036123
> > > > > > >    1bde35-1bdecc l noploop
> > > > > > >   perf does have symbol 'noploop'
> > > > > > >
> > > > > > >   Record+report pipe test
> > > > > > >   [ perf record: Woken up 1 times to write data ]
> > > > > > >   [ perf record: Captured and wrote 0.210 MB - ]
> > > > > > >   [ perf record: Woken up 2 times to write data ]
> > > > > > >   [ perf record: Captured and wrote 0.517 MB - ]
> > > > > > >   [ perf record: Woken up 2 times to write data ]
> > > > > > >   [ perf record: Captured and wrote 0.516 MB - ]
> > > > > > >   Record+report pipe test [Success]
> > > > > > >
> > > > > > >   Inject -B build-ids test
> > > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > > >   Error:
> > > > > > >   failed to process sample
> > > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > > >
> > > > > > >   Inject -b build-ids test
> > > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > > >   Error:
> > > > > > >   failed to process sample
> > > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > > >
> > > > > > >   Inject --buildid-all build-ids test
> > > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > > >   Error:
> > > > > > >   failed to process sample
> > > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > > >
> > > > > > >   Inject --mmap2-buildid-all build-ids test
> > > > > > >   0xa5c [0x17a4]: failed to process type: 80
> > > > > > >   Error:
> > > > > > >   failed to process sample
> > > > > > >   Inject build-ids test [Failed - cannot find noploop function in pipe #1]
> > > > > > >   ---- end(-1) ----
> > > > > > >    84: perf pipe recording and injection test                          : FAILED!
> > > > > > >
> > > > > > >   $ sudo ./perf test -v Zstd
> > > > > > >   --- start ---
> > > > > > >   test child forked, pid 3036097
> > > > > > >   Collecting compressed record file:
> > > > > > >   500+0 records in
> > > > > > >   500+0 records out
> > > > > > >   256000 bytes (256 kB, 250 KiB) copied, 0.00169127 s, 151 MB/s
> > > > > > >   [ perf record: Woken up 1 times to write data ]
> > > > > > >   [ perf record: Captured and wrote 0.032 MB /tmp/perf.data.KBo, compressed (original 0.004 MB, ratio is 3.324) ]
> > > > > > >   Checking compressed events stats:
> > > > > > >   Couldn't decompress data
> > > > > > >   0x7ca8 [0x4f2]: failed to process type: 81 [Operation not permitted]
> > > > > > >   Error:
> > > > > > >   failed to process sample
> > > > > > >   ---- end(-1) ----
> > > > > > >    86: Zstd perf.data compression/decompression                        : FAILED!
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Namhyung
> > > > > >
> > > > > > I'm not able to reproduce:
> > > > > > ```
> > > > > > $ git log --oneline |head
> > > > > > a59bca6eb0a6 perf test: Add a runs-per-test flag
> > > > > > 0d0c002eb45c perf tests: Enable tests disabled due to tracepoint parsing
> > > > > > 4b8f5c9dfbda perf evsel: Allow evsel__newtp without libtraceevent
> > > > > > 7f57057c7884 perf evsel: Add/use accessor for tp_format
> > > > > > c27d357d2d4c perf trace-event: Always build trace-event-info.c
> > > > > > 20bf7a2cd68a perf trace-event: Constify print arguments
> > > > > > f18b07ee2af1 tool api fs: Correctly encode errno for read/write open failures
> > > > > > ...
> > > > > > $ sudo /tmp/perf/perf test -r 10 Zstd pipe -v
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 84: perf pipe recording and injection test                          : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > 86: Zstd perf.data compression/decompression                        : Ok
> > > > > > ```
> > > > > > Similarly not as root or if runs-per-test is 100.
> > > > > >
> > > > > > Agreed that the changes are for tracepoints and these tests aren't for
> > > > > > tracepoints, so an interaction wouldn't be expected. If you have a
> > > > > > reliable reproduction perhaps you can bisect it.
> > > > >
> > > > > it says:
> > > > >
> > > > > 9c10de391840a35ab095b65e9a5c4fad0ac52068 is the first bad commit
> > > > > commit 9c10de391840a35ab095b65e9a5c4fad0ac52068 (HEAD)
> > > > > Author: Ian Rogers <irogers@google.com>
> > > > > Date:   Fri Nov 8 10:47:46 2024 -0800
> > > > >
> > > > >     tool api fs: Correctly encode errno for read/write open failures
> > > > >
> > > > >     Switch from returning -1 to -errno so that callers can determine types
> > > > >     of failure.
> > > > >
> > > > >     Signed-off-by: Ian Rogers <irogers@google.com>
> > > > >     Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > > >
> > > > >  tools/lib/api/fs/fs.c | 6 +++---
> > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > So I tried to eye-ball/grep all callers to spot assumptions on the
> > > > return value like:
> > > > ```
> > > > err = ...__read_int
> > > > if (err == -1)
> > > > ```
> > > > Didn't spot anything.
> > > >
> > > > It seems in the test log the record is failing so I ran this under
> > > > gdb, set breakpoints on the 3 modified functions and then looked up
> > > > the call stack to spot bad return value assumptions. Everything looks
> > > > good.
> > > > I then tried inject and report, the only file read by these functions
> > > > is /proc/sys/kernel/perf_event_paranoid as part of symbol
> > > > initialization (nit, this should probably be read lazily and the
> > > > restriction should really come from the perf.data file, not the
> > > > running system) and those calls look good.
> > > >
> > > > The change is small and not critical for the series. It improves the
> > > > error message when reading the tracepoint id fails. So we could move
> > > > forward with the rest of the series, but that could be annoying for
> > > > tracepoint users.
> > > >
> > > > If I had a reproducer I'd revert the 1 line change on each function to
> > > > find out which is causing the regression. Once you have that then you
> > > > can binary search to find the bad call by using some global counter
> > > > where the first 'n' calls use the new return value and the later use
> > > > the old value. You can then vary 'n' to binary search and find the bad
> > > > caller.
> > > >
> > > > Is there any chance you can help diagnose this or help me to find the
> > > > reproducer?
> > >
> > > I think this depends on the system configuration.  I've debugged it
> > > failed when it gets cpu topology:
> > >
> > >   ...
> > >   read int failed: /sys/devices/system/cpu/cpu112/topology/core_id (errno=2)
> > >   read int failed: /sys/devices/system/cpu/cpu112/topology/physical_package_id (errno=2)
> > >   read int failed: /sys/devices/system/cpu/cpu112/topology/die_id (errno=2)
> > >   ...
> > >
> > > Maybe it's because # online CPUs != # possible CPUs.
> > >
> > >   $ cat /sys/devices/system/cpu/online
> > >   0-63
> > >   $ cat /sys/devices/system/cpu/possible
> > >   0-127
> > >
> > > There's a code like cpu__get_socket_id() to use the return value of
> > > sysfs__read_int() directly.  And it saves the value to aggr_cpu_id which
> > > requires exact match like in aggr_cpu_id__equal().
> >
> > So this is a latent bug. Are you working on the fix or asking me to do
> > it? I'm not sure why we should fail to describe the topology for
> > offline cores, but if this is a kernel restriction we should probably
> > purge those logical CPUs from the topology.
>
> It doesn't have the topology directory for the offline CPUs.
>
>   $ ls /sys/devices/system/cpu/cpu112/
>   acpi_cppc  cpufreq  cpuidle  crash_notes  crash_notes_size  driver  firmware_node  hotplug  node0  online  power  subsystem  uevent
>
>   $ ls /sys/devices/system/cpu/cpu112/topology
>   ls: cannot access '/sys/devices/system/cpu/cpu112/topology': No such file or directory
>
> Anyway, I'm not sure if it's really needed for this change.  Maybe you
> can drop the patch 1 from the series for now and tackle it later.

So this would be a regression. Currently in add_tracepoint the calls look like:

add_tracepoint
|->evsel__new_tp_idx
  |-> tp_format
    |->filename__read_str

The filename__read_str error is passed in add_tracepoint for errors to
tracepoint_error that handles EACCES, ENOENT and default as separate
cases:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c#n466
The code changes filename__read_str to filename__read_int but if that
can only return -1 on error then that'll make all tracepoint open
errors look like EPERM.

Based on your instructions I was able to reproduce the problem and
have a fix to just force the error value to always be -1. I'll add
that to the series and send as v5.

Thanks,
Ian