[PATCH v1 00/17] perf: Small fixes and new tests

[PATCH v1 00/17] perf: Small fixes and new tests

[Ian Rogers]

Pushing for additional test coverage exposed minor fixes, a missing
timechart option, and a number of reference counting issues. Fix the
issues then add the new tests. The new tests cover c2c, additional
build-id tests, perf top, timechart, kallsyms, perf script with
 --dlfilter, evlist and kvm.

Ian Rogers (17):
  perf kvm: Fix debug assertion
  perf timechart: Add record support for output perf.data path
  perf symbol-elf: Add missing puts on error path
  perf symbol: Add missed dso__put
  perf probe-event: Ensure probe event nsinfo is always cleared
  perf hist: In init, ensure mem_info is put on error paths
  perf mem-events: Don't leak online CPU map
  perf jitdump: Fix missed dso__put
  perf c2c: Clean up some defensive gets and make asan clean
  perf tests c2c: Add a basic c2c
  perf tests buildid: Add purge and remove testing
  perf tests top: Add basic perf top coverage test
  perf tests timechart: Add a perf timechart test
  perf tests kallsyms: Add basic kallsyms test
  perf tests script dlfilter: Add a dlfilter test
  perf tests evlist: Add basic evlist test
  perf test kvm: Add some basic perf kvm test coverage

 tools/perf/builtin-c2c.c                  |  36 ++--
 tools/perf/builtin-kvm.c                  |   2 +-
 tools/perf/builtin-timechart.c            |  15 +-
 tools/perf/tests/shell/buildid.sh         | 203 +++++++++++++++++++---
 tools/perf/tests/shell/c2c.sh             |  62 +++++++
 tools/perf/tests/shell/evlist.sh          |  79 +++++++++
 tools/perf/tests/shell/kallsyms.sh        |  56 ++++++
 tools/perf/tests/shell/kvm.sh             | 154 ++++++++++++++++
 tools/perf/tests/shell/script_dlfilter.sh | 107 ++++++++++++
 tools/perf/tests/shell/timechart.sh       |  67 +++++++
 tools/perf/tests/shell/top.sh             |  74 ++++++++
 tools/perf/util/hist.c                    |   6 +-
 tools/perf/util/jitdump.c                 |   2 +
 tools/perf/util/mem-events.c              |   5 +-
 tools/perf/util/probe-event.c             |  12 +-
 tools/perf/util/symbol-elf.c              |   5 +-
 tools/perf/util/symbol.c                  |   1 +
 17 files changed, 819 insertions(+), 67 deletions(-)
 create mode 100755 tools/perf/tests/shell/c2c.sh
 create mode 100755 tools/perf/tests/shell/evlist.sh
 create mode 100755 tools/perf/tests/shell/kallsyms.sh
 create mode 100755 tools/perf/tests/shell/kvm.sh
 create mode 100755 tools/perf/tests/shell/script_dlfilter.sh
 create mode 100755 tools/perf/tests/shell/timechart.sh
 create mode 100755 tools/perf/tests/shell/top.sh

-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 01/17] perf kvm: Fix debug assertion

[Ian Rogers]

There are 2 slots left for kvm_add_default_arch_event, fix the
assertion so that debug builds don't fail the assert and to agree with
the comment.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index f0f285763f19..c61369d54dd9 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -2014,7 +2014,7 @@ static int __cmd_record(const char *file_name, int argc, const char **argv)
 	for (j = 1; j < argc; j++, i++)
 		rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
 
-	BUG_ON(i != rec_argc);
+	BUG_ON(i + 2 != rec_argc);
 
 	ret = kvm_add_default_arch_event(&i, rec_argv);
 	if (ret)
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 02/17] perf timechart: Add record support for output perf.data path

[Ian Rogers]

The '-o' option exists for the SVG creation but not for `perf
timechart record`. Add to better allow testing.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-timechart.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 22050c640dfa..f8b49d69e9a5 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1651,7 +1651,7 @@ static int __cmd_timechart(struct timechart *tchart, const char *output_name)
 	return ret;
 }
 
-static int timechart__io_record(int argc, const char **argv)
+static int timechart__io_record(int argc, const char **argv, const char *output_data)
 {
 	unsigned int rec_argc, i;
 	const char **rec_argv;
@@ -1659,7 +1659,7 @@ static int timechart__io_record(int argc, const char **argv)
 	char *filter = NULL;
 
 	const char * const common_args[] = {
-		"record", "-a", "-R", "-c", "1",
+		"record", "-a", "-R", "-c", "1", "-o", output_data,
 	};
 	unsigned int common_args_nr = ARRAY_SIZE(common_args);
 
@@ -1786,7 +1786,8 @@ static int timechart__io_record(int argc, const char **argv)
 }
 
 
-static int timechart__record(struct timechart *tchart, int argc, const char **argv)
+static int timechart__record(struct timechart *tchart, int argc, const char **argv,
+			     const char *output_data)
 {
 	unsigned int rec_argc, i, j;
 	const char **rec_argv;
@@ -1794,7 +1795,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
 	unsigned int record_elems;
 
 	const char * const common_args[] = {
-		"record", "-a", "-R", "-c", "1",
+		"record", "-a", "-R", "-c", "1", "-o", output_data,
 	};
 	unsigned int common_args_nr = ARRAY_SIZE(common_args);
 
@@ -1934,6 +1935,7 @@ int cmd_timechart(int argc, const char **argv)
 		.merge_dist = 1000,
 	};
 	const char *output_name = "output.svg";
+	const char *output_record_data = "perf.data";
 	const struct option timechart_common_options[] = {
 	OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
 	OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only, "output processes data only"),
@@ -1976,6 +1978,7 @@ int cmd_timechart(int argc, const char **argv)
 	OPT_BOOLEAN('I', "io-only", &tchart.io_only,
 		    "record only IO data"),
 	OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
+	OPT_STRING('o', "output", &output_record_data, "file", "output data file name"),
 	OPT_PARENT(timechart_common_options),
 	};
 	const char * const timechart_record_usage[] = {
@@ -2024,9 +2027,9 @@ int cmd_timechart(int argc, const char **argv)
 		}
 
 		if (tchart.io_only)
-			ret = timechart__io_record(argc, argv);
+			ret = timechart__io_record(argc, argv, output_record_data);
 		else
-			ret = timechart__record(&tchart, argc, argv);
+			ret = timechart__record(&tchart, argc, argv, output_record_data);
 		goto out;
 	} else if (argc)
 		usage_with_options(timechart_usage, timechart_options);
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 03/17] perf symbol-elf: Add missing puts on error path

[Ian Rogers]

In dso__process_kernel_symbol if inserting a map fails, probably
ENOMEM, then the reference count puts were missing on the dso and map.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/symbol-elf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9e820599bab3..4c76e680b13d 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1446,8 +1446,11 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 			map__set_mapping_type(curr_map, MAPPING_TYPE__IDENTITY);
 		}
 		dso__set_symtab_type(curr_dso, dso__symtab_type(dso));
-		if (maps__insert(kmaps, curr_map))
+		if (maps__insert(kmaps, curr_map)) {
+			dso__put(curr_dso);
+			map__put(curr_map);
 			return -1;
+		}
 		dsos__add(&maps__machine(kmaps)->dsos, curr_dso);
 		dso__set_loaded(curr_dso);
 		dso__put(*curr_dsop);
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 04/17] perf symbol: Add missed dso__put

[Ian Rogers]

Add missing dso__put for the dso created in maps__split_kallsyms.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/symbol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index cc26b7bf302b..067754b5b667 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -985,6 +985,7 @@ static int maps__split_kallsyms(struct maps *kmaps, struct dso *dso, u64 delta,
 				dso__put(ndso);
 				return -1;
 			}
+			dso__put(ndso);
 			++kernel_range;
 		} else if (delta) {
 			/* Kernel was relocated at boot time */
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 05/17] perf probe-event: Ensure probe event nsinfo is always cleared

[Ian Rogers]

Move nsinfo__zput from cleanup_perf_probe_events to
clear_perf_probe_event so it is always executed. Clean up
clear_perf_probe_event to not call nsinfo__zput and use the pev
variable to avoid repeated array accesses.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/probe-event.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 6ab2eb551b6c..710e4620923e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2419,6 +2419,7 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 	}
 	pev->nargs = 0;
 	zfree(&pev->args);
+	nsinfo__zput(pev->nsi);
 }
 
 #define strdup_or_goto(str, label)	\
@@ -3767,12 +3768,11 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	/* Loop 3: cleanup and free trace events  */
 	for (i = 0; i < npevs; i++) {
 		pev = &pevs[i];
-		for (j = 0; j < pevs[i].ntevs; j++)
-			clear_probe_trace_event(&pevs[i].tevs[j]);
-		zfree(&pevs[i].tevs);
-		pevs[i].ntevs = 0;
-		nsinfo__zput(pev->nsi);
-		clear_perf_probe_event(&pevs[i]);
+		for (j = 0; j < pev->ntevs; j++)
+			clear_probe_trace_event(&pev->tevs[j]);
+		zfree(&pev->tevs);
+		pev->ntevs = 0;
+		clear_perf_probe_event(pev);
 	}
 }
 
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 06/17] perf hist: In init, ensure mem_info is put on error paths

[Ian Rogers]

Rather than exit the internal map_symbols directly, put the mem-info
that does this and also lowers the reference count on the mem-info
itself otherwise the mem-info is being leaked.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/hist.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 64ff427040c3..ef4b569f7df4 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
 		map_symbol__exit(&he->branch_info->to.ms);
 		zfree(&he->branch_info);
 	}
-	if (he->mem_info) {
-		map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
-		map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
-	}
+	if (he->mem_info)
+		mem_info__zput(he->mem_info);
 err:
 	map_symbol__exit(&he->ms);
 	zfree(&he->stat_acc);
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 07/17] perf mem-events: Don't leak online CPU map

[Ian Rogers]

Reference count checking found the online CPU map was being gotten but
not put. Add in the missing put.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/mem-events.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 80b3069427bc..0b49fce251fc 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -303,12 +303,15 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr, char **eve
 	}
 
 	if (cpu_map) {
-		if (!perf_cpu_map__equal(cpu_map, cpu_map__online())) {
+		struct perf_cpu_map *online = cpu_map__online();
+
+		if (!perf_cpu_map__equal(cpu_map, online)) {
 			char buf[200];
 
 			cpu_map__snprint(cpu_map, buf, sizeof(buf));
 			pr_warning("Memory events are enabled on a subset of CPUs: %s\n", buf);
 		}
+		perf_cpu_map__put(online);
 		perf_cpu_map__put(cpu_map);
 	}
 
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 08/17] perf jitdump: Fix missed dso__put

[Ian Rogers]

Reference count checking caught a missing dso__put following a
machine__findnew_dso_id.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/jitdump.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index b062b1f234b6..27a2ea1b000b 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -546,6 +546,8 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
 
 		if (dso)
 			dso__set_hit(dso);
+
+		dso__put(dso);
 	}
 out:
 	perf_sample__exit(&sample);
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 09/17] perf c2c: Clean up some defensive gets and make asan clean

[Ian Rogers]

To deal with histogram code that had missing gets the c2c code had
some defensive gets. Those other issues were cleaned up by the
reference count checker, clean them up for the c2c command here.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-c2c.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 14c3823f8fed..d390ae4e3ec8 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -322,7 +322,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 	struct c2c_stats stats = { .nr_entries = 0, };
 	struct hist_entry *he;
 	struct addr_location al;
-	struct mem_info *mi, *mi_dup;
+	struct mem_info *mi = NULL;
 	struct callchain_cursor *cursor;
 	int ret;
 
@@ -349,20 +349,15 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 		goto out;
 	}
 
-	/*
-	 * The mi object is released in hists__add_entry_ops,
-	 * if it gets sorted out into existing data, so we need
-	 * to take the copy now.
-	 */
-	mi_dup = mem_info__get(mi);
-
 	c2c_decode_stats(&stats, mi);
 
 	he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
 				  &al, NULL, NULL, mi, NULL,
 				  sample, true);
-	if (he == NULL)
-		goto free_mi;
+	if (he == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	c2c_add_stats(&c2c_he->stats, &stats);
@@ -393,17 +388,19 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 		int cpu = sample->cpu == (unsigned int) -1 ? 0 : sample->cpu;
 		int node = c2c.cpu2node[cpu];
 
-		mi = mi_dup;
-
 		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2, machine->env);
-		if (!c2c_hists)
-			goto free_mi;
+		if (!c2c_hists) {
+			ret = -ENOMEM;
+			goto out;
+		}
 
 		he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
 					  &al, NULL, NULL, mi, NULL,
 					  sample, true);
-		if (he == NULL)
-			goto free_mi;
+		if (he == NULL) {
+			ret = -ENOMEM;
+			goto out;
+		}
 
 		c2c_he = container_of(he, struct c2c_hist_entry, he);
 		c2c_add_stats(&c2c_he->stats, &stats);
@@ -421,14 +418,9 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 	}
 
 out:
+	mem_info__put(mi);
 	addr_location__exit(&al);
 	return ret;
-
-free_mi:
-	mem_info__put(mi_dup);
-	mem_info__put(mi);
-	ret = -ENOMEM;
-	goto out;
 }
 
 static const char * const c2c_usage[] = {
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 10/17] perf tests c2c: Add a basic c2c

[Ian Rogers]

Add basic c2c record and report testing to gain some coverage.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/c2c.sh | 62 +++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100755 tools/perf/tests/shell/c2c.sh

diff --git a/tools/perf/tests/shell/c2c.sh b/tools/perf/tests/shell/c2c.sh
new file mode 100755
index 000000000000..2471d44595c3
--- /dev/null
+++ b/tools/perf/tests/shell/c2c.sh
@@ -0,0 +1,62 @@
+#!/bin/bash
+# perf c2c tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+perfdata=$(mktemp /tmp/__perf_c2c_test.perf.data.XXXXX)
+
+cleanup() {
+	rm -f "${perfdata}"
+	rm -f "${perfdata}".old
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+check_c2c_support() {
+	# Check if perf c2c record works.
+	if ! perf c2c record -o "${perfdata}" -- true > /dev/null 2>&1 ; then
+		return 1
+	fi
+	return 0
+}
+
+test_c2c_record_report() {
+	echo "c2c record and report test"
+	if ! check_c2c_support ; then
+		echo "c2c record and report test [Skipped: perf c2c record failed (possibly missing hardware support)]"
+		err=2
+		return
+	fi
+
+	# Run a workload that does some memory operations.
+	if ! perf c2c record -o "${perfdata}" -- perf test -w datasym 1 > /dev/null 2>&1 ; then
+		echo "c2c record and report test [Skipped: perf c2c record failed during workload]"
+		return
+	fi
+
+	if ! perf c2c report -i "${perfdata}" --stdio > /dev/null 2>&1 ; then
+		echo "c2c record and report test [Failed: report failed]"
+		err=1
+		return
+	fi
+
+	if ! perf c2c report -i "${perfdata}" -N > /dev/null 2>&1 ; then
+		echo "c2c record and report test [Failed: report -N failed]"
+		err=1
+		return
+	fi
+
+	echo "c2c record and report test [Success]"
+}
+
+test_c2c_record_report
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 11/17] perf tests buildid: Add purge and remove testing

[Ian Rogers]

Add testing for the purge and remove commands. Use the noploop
workload rather than just a return to avoid missing samples in the
workload in perf record. Tidy up the cleanup code to cleanup when
signals happen.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/buildid.sh | 203 ++++++++++++++++++++++++++----
 1 file changed, 177 insertions(+), 26 deletions(-)

diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
index d2eb213da01d..102808cca9db 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -36,16 +36,69 @@ if [ ${run_pe} -eq 1 ]; then
 	unset WAYLAND_DISPLAY
 fi
 
-ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
-ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
+build_id_dir=
+ex_source=$(mktemp /tmp/perf_buildid_test.ex.XXX.c)
+ex_md5=$(mktemp /tmp/perf_buildid_test.ex.MD5.XXX)
+ex_sha1=$(mktemp /tmp/perf_buildid_test.ex.SHA1.XXX)
 ex_pe=$(dirname $0)/../pe-file.exe
+data=$(mktemp /tmp/perf_buildid_test.data.XXX)
+log_out=$(mktemp /tmp/perf_buildid_test.log.out.XXX)
+log_err=$(mktemp /tmp/perf_buildid_test.log.err.XXX)
 
-echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
-echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
+cleanup() {
+	rm -f ${ex_source} ${ex_md5} ${ex_sha1} ${data} ${log_out} ${log_err}
+	if [ ${run_pe} -eq 1 ]; then
+		rm -r ${wineprefix}
+	fi
+        if [ -d ${build_id_dir} ]; then
+		rm -rf ${build_id_dir}
+        fi
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+# Test program based on the noploop workload.
+cat <<EOF > ${ex_source}
+#include <stdlib.h>
+#include <signal.h>
+#include <unistd.h>
 
+static volatile sig_atomic_t done;
+
+static void sighandler(int sig)
+{
+	(void)sig;
+	done = 1;
+}
+
+int main(int argc, const char **argv)
+{
+	int sec = 1;
+
+	if (argc > 1)
+		sec = atoi(argv[1]);
+
+	signal(SIGINT, sighandler);
+	signal(SIGALRM, sighandler);
+	alarm(sec);
+
+	while (!done)
+		continue;
+
+	return 0;
+}
+EOF
+cc -Wl,--build-id=sha1 ${ex_source} -o ${ex_sha1} -x c -
+cc -Wl,--build-id=md5 ${ex_source} -o ${ex_md5} -x c -
 echo "test binaries: ${ex_sha1} ${ex_md5} ${ex_pe}"
 
-check()
+get_build_id()
 {
 	case $1 in
 	*.exe)
@@ -64,6 +117,15 @@ check()
 		id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
 		;;
 	esac
+	echo ${id}
+}
+
+check()
+{
+	file=$1
+	perf_data=$2
+
+	id=$(get_build_id $file)
 	echo "build id: ${id}"
 
 	id_file=${id#??}
@@ -76,45 +138,53 @@ check()
 		exit 1
 	fi
 
-	file=${build_id_dir}/.build-id/$id_dir/`readlink ${link}`/elf
-	echo "file: ${file}"
+	cached_file=${build_id_dir}/.build-id/$id_dir/`readlink ${link}`/elf
+	echo "file: ${cached_file}"
 
 	# Check for file permission of original file
 	# in case of pe-file.exe file
 	echo $1 | grep ".exe"
 	if [ $? -eq 0 ]; then
-		if [ -x $1 ] && [ ! -x $file ]; then
-			echo "failed: file ${file} executable does not exist"
+		if [ -x $1 ] && [ ! -x $cached_file ]; then
+			echo "failed: file ${cached_file} executable does not exist"
 			exit 1
 		fi
 
-		if [ ! -x $file ] && [ ! -e $file ]; then
-			echo "failed: file ${file} does not exist"
+		if [ ! -x $cached_file ] && [ ! -e $cached_file ]; then
+			echo "failed: file ${cached_file} does not exist"
 			exit 1
 		fi
-	elif [ ! -x $file ]; then
-		echo "failed: file ${file} does not exist"
+	elif [ ! -x $cached_file ]; then
+		echo "failed: file ${cached_file} does not exist"
 		exit 1
 	fi
 
-	diff ${file} ${1}
+	diff ${cached_file} ${1}
 	if [ $? -ne 0 ]; then
-		echo "failed: ${file} do not match"
+		echo "failed: ${cached_file} do not match"
 		exit 1
 	fi
 
-	${perf} buildid-cache -l | grep ${id}
+	${perf} buildid-cache -l | grep -q ${id}
 	if [ $? -ne 0 ]; then
 		echo "failed: ${id} is not reported by \"perf buildid-cache -l\""
 		exit 1
 	fi
 
+	if [ -n "${perf_data}" ]; then
+		${perf} buildid-list -i ${perf_data} | grep -q ${id}
+		if [ $? -ne 0 ]; then
+			echo "failed: ${id} is not reported by \"perf buildid-list -i ${perf_data}\""
+			exit 1
+		fi
+	fi
+
 	echo "OK for ${1}"
 }
 
 test_add()
 {
-	build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+	build_id_dir=$(mktemp -d /tmp/perf_buildid_test.debug.XXX)
 	perf="perf --buildid-dir ${build_id_dir}"
 
 	${perf} buildid-cache -v -a ${1}
@@ -128,12 +198,88 @@ test_add()
 	rm -rf ${build_id_dir}
 }
 
+test_remove()
+{
+	build_id_dir=$(mktemp -d /tmp/perf_buildid_test.debug.XXX)
+	perf="perf --buildid-dir ${build_id_dir}"
+
+	${perf} buildid-cache -v -a ${1}
+	if [ $? -ne 0 ]; then
+		echo "failed: add ${1} to build id cache"
+		exit 1
+	fi
+
+	id=$(get_build_id ${1})
+	if ! ${perf} buildid-cache -l | grep -q ${id}; then
+		echo "failed: ${id} not in cache"
+		exit 1
+	fi
+
+	${perf} buildid-cache -v -r ${1}
+	if [ $? -ne 0 ]; then
+		echo "failed: remove ${id} from build id cache"
+		exit 1
+	fi
+
+	if ${perf} buildid-cache -l | grep -q ${id}; then
+		echo "failed: ${id} still in cache after remove"
+		exit 1
+	fi
+
+	echo "remove: OK"
+	rm -rf ${build_id_dir}
+}
+
+test_purge()
+{
+	build_id_dir=$(mktemp -d /tmp/perf_buildid_test.debug.XXX)
+	perf="perf --buildid-dir ${build_id_dir}"
+
+	id1=$(get_build_id ${ex_sha1})
+	${perf} buildid-cache -v -a ${ex_sha1}
+	if ! ${perf} buildid-cache -l | grep -q ${id1}; then
+		echo "failed: ${id1} not in cache"
+		exit 1
+	fi
+
+	id2=$(get_build_id ${ex_md5})
+	${perf} buildid-cache -v -a ${ex_md5}
+	if ! ${perf} buildid-cache -l | grep -q ${id2}; then
+		echo "failed: ${id2} not in cache"
+		exit 1
+	fi
+
+	# Purge by path
+	${perf} buildid-cache -v -p ${ex_sha1}
+	if [ $? -ne 0 ]; then
+		echo "failed: purge build id cache of ${ex_sha1}"
+		exit 1
+	fi
+
+	${perf} buildid-cache -v -p ${ex_md5}
+	if [ $? -ne 0 ]; then
+		echo "failed: purge build id cache of ${ex_md5}"
+		exit 1
+	fi
+
+	# Verify both are gone
+	if ${perf} buildid-cache -l | grep -q ${id1}; then
+		echo "failed: ${id1} still in cache after purge"
+		exit 1
+	fi
+
+	if ${perf} buildid-cache -l | grep -q ${id2}; then
+		echo "failed: ${id2} still in cache after purge"
+		exit 1
+	fi
+
+	echo "purge: OK"
+	rm -rf ${build_id_dir}
+}
+
 test_record()
 {
-	data=$(mktemp /tmp/perf.data.XXX)
-	build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
-	log_out=$(mktemp /tmp/perf.log.out.XXX)
-	log_err=$(mktemp /tmp/perf.log.err.XXX)
+	build_id_dir=$(mktemp -d /tmp/perf_buildid_test.debug.XXX)
 	perf="perf --buildid-dir ${build_id_dir}"
 
 	echo "running: perf record $*"
@@ -145,7 +291,7 @@ test_record()
 	fi
 
 	args="$*"
-	check ${args##* }
+	check ${args##* } ${data}
 
 	rm -f ${log_out} ${log_err}
 	rm -rf ${build_id_dir}
@@ -166,10 +312,15 @@ if [ ${run_pe} -eq 1 ]; then
 	test_record wine ${ex_pe}
 fi
 
-# cleanup
-rm ${ex_sha1} ${ex_md5}
-if [ ${run_pe} -eq 1 ]; then
-	rm -r ${wineprefix}
+# remove binaries manually via perf buildid-cache -r
+test_remove ${ex_sha1}
+test_remove ${ex_md5}
+if [ ${add_pe} -eq 1 ]; then
+	test_remove ${ex_pe}
 fi
 
+# purge binaries manually via perf buildid-cache -p
+test_purge
+
+cleanup
 exit 0
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 12/17] perf tests top: Add basic perf top coverage test

[Ian Rogers]

The test starts a backgroup thloop workload and monitors it using
cpu-clock ensuring test_loop appears in the output.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/top.sh | 74 +++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100755 tools/perf/tests/shell/top.sh

diff --git a/tools/perf/tests/shell/top.sh b/tools/perf/tests/shell/top.sh
new file mode 100755
index 000000000000..768ebcf7a89c
--- /dev/null
+++ b/tools/perf/tests/shell/top.sh
@@ -0,0 +1,74 @@
+#!/bin/bash
+# perf top tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+log_file=$(mktemp /tmp/perf.top.log.XXXXX)
+
+cleanup() {
+	rm -f "${log_file}"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+test_basic_perf_top() {
+	echo "Basic perf top test"
+
+	# Start a workload that spins to generate samples
+	# thloop runs for the specified number of seconds
+	perf test -w thloop 20 &
+	PID=$!
+
+	# Allow it to start
+	sleep 0.1
+
+	# Run perf top for 5 seconds, monitoring that PID
+	# Use --stdio to avoid TUI and redirect output
+	# Use -d 1 to avoid flooding output
+	# Use -e cpu-clock to ensure we get samples
+	# Use sleep to keep stdin open but silent, preventing EOF loop or interactive spam
+	if ! sleep 10 | timeout 5s perf top --stdio -d 1 -e cpu-clock -p $PID > "${log_file}" 2>&1; then
+		retval=$?
+		if [ $retval -ne 124 ] && [ $retval -ne 0 ]; then
+			echo "Basic perf top test [Failed: perf top failed to start or run (ret=$retval)]"
+			head -n 50 "${log_file}"
+			kill $PID
+			wait $PID 2>/dev/null || true
+			err=1
+			return
+		fi
+	fi
+
+	kill $PID
+	wait $PID 2>/dev/null || true
+
+	# Check for some sample data (percentage)
+	if ! grep -E -q "[0-9]+\.[0-9]+%" "${log_file}"; then
+		echo "Basic perf top test [Failed: no sample percentage found]"
+		head -n 50 "${log_file}"
+		err=1
+		return
+	fi
+
+	# Check for the symbol
+	if ! grep -q "test_loop" "${log_file}"; then
+		echo "Basic perf top test [Failed: test_loop symbol not found]"
+		head -n 50 "${log_file}"
+		err=1
+		return
+	fi
+
+	echo "Basic perf top test [Success]"
+}
+
+test_basic_perf_top
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 13/17] perf tests timechart: Add a perf timechart test

[Ian Rogers]

Basic coverage for `perf timechart` doing a record and then a basic
sanity test of the generated SVG file.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/timechart.sh | 67 +++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100755 tools/perf/tests/shell/timechart.sh

diff --git a/tools/perf/tests/shell/timechart.sh b/tools/perf/tests/shell/timechart.sh
new file mode 100755
index 000000000000..b14b3472c284
--- /dev/null
+++ b/tools/perf/tests/shell/timechart.sh
@@ -0,0 +1,67 @@
+#!/bin/bash
+# perf timechart tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+perfdata=$(mktemp /tmp/__perf_timechart_test.perf.data.XXXXX)
+output=$(mktemp /tmp/__perf_timechart_test.output.XXXXX.svg)
+
+cleanup() {
+	rm -f "${perfdata}"
+	rm -f "${output}"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+test_timechart() {
+	echo "Basic perf timechart test"
+
+	# Try to record timechart data.
+	# perf timechart record uses system-wide recording and specific tracepoints.
+	# If it fails (e.g. permissions, missing tracepoints), skip the test.
+	if ! perf timechart record -o "${perfdata}" true > /dev/null 2>&1; then
+		echo "Basic perf timechart test [Skipped: perf timechart record failed (permissions/events?)]"
+		return
+	fi
+
+	# Generate the timechart
+	if ! perf timechart -i "${perfdata}" -o "${output}" > /dev/null 2>&1; then
+		echo "Basic perf timechart test [Failed: perf timechart command failed]"
+		err=1
+		return
+	fi
+
+	# Check if output file exists and is not empty
+	if [ ! -s "${output}" ]; then
+		echo "Basic perf timechart test [Failed: output file is empty or missing]"
+		err=1
+		return
+	fi
+
+	# Check if it looks like an SVG
+	if ! grep -q "svg" "${output}"; then
+		echo "Basic perf timechart test [Failed: output doesn't look like SVG]"
+		err=1
+		return
+	fi
+
+	echo "Basic perf timechart test [Success]"
+}
+
+if ! perf check feature -q libtraceevent ; then
+	echo "perf timechart is not supported. Skip."
+        cleanup
+	exit 2
+fi
+
+test_timechart
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 14/17] perf tests kallsyms: Add basic kallsyms test

[Ian Rogers]

Add test that kallsyms finds a well known symbol and fails for
another.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/kallsyms.sh | 56 ++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100755 tools/perf/tests/shell/kallsyms.sh

diff --git a/tools/perf/tests/shell/kallsyms.sh b/tools/perf/tests/shell/kallsyms.sh
new file mode 100755
index 000000000000..d0eb99753f47
--- /dev/null
+++ b/tools/perf/tests/shell/kallsyms.sh
@@ -0,0 +1,56 @@
+#!/bin/bash
+# perf kallsyms tests
+# SPDX-License-Identifier: GPL-2.0
+
+err=0
+
+test_kallsyms() {
+	echo "Basic perf kallsyms test"
+
+	# Check if /proc/kallsyms is readable
+	if [ ! -r /proc/kallsyms ]; then
+		echo "Basic perf kallsyms test [Skipped: /proc/kallsyms not readable]"
+		err=2
+		return
+	fi
+
+	# Use a symbol that is definitely a function and present in all kernels, e.g. schedule
+	symbol="schedule"
+
+	# Run perf kallsyms
+	# It prints "address symbol_name"
+	output=$(perf kallsyms $symbol 2>&1)
+	ret=$?
+
+	if [ $ret -ne 0 ] || [ -z "$output" ]; then
+		# If empty or failed, it might be due to permissions (kptr_restrict)
+		# Check if we can grep the symbol from /proc/kallsyms directly
+		if grep -q "$symbol" /proc/kallsyms 2>/dev/null; then
+			# If it's in /proc/kallsyms but perf kallsyms returned empty/error,
+			# it likely means perf couldn't parse it or access it correctly (e.g. kptr_restrict=2).
+			echo "Basic perf kallsyms test [Skipped: $symbol found in /proc/kallsyms but perf kallsyms failed (output: '$output')]"
+			err=2
+			return
+		else
+			echo "Basic perf kallsyms test [Skipped: $symbol not found in /proc/kallsyms]"
+			err=2
+			return
+		fi
+	fi
+
+	if echo "$output" | grep -q "not found"; then
+		echo "Basic perf kallsyms test [Failed: output '$output' does not contain $symbol]"
+		err=1
+		return
+	fi
+
+	if perf kallsyms ErlingHaaland | grep -vq "not found"; then
+		echo "Basic perf kallsyms test [Failed: ErlingHaaland found in the output]"
+		err=1
+		return
+	fi
+	echo "Basic perf kallsyms test [Success]"
+}
+
+test_kallsyms
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 15/17] perf tests script dlfilter: Add a dlfilter test

[Ian Rogers]

Compile a simple dlfilter and make sure it remove samples from
everything other than a test_loop.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/script_dlfilter.sh | 107 ++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100755 tools/perf/tests/shell/script_dlfilter.sh

diff --git a/tools/perf/tests/shell/script_dlfilter.sh b/tools/perf/tests/shell/script_dlfilter.sh
new file mode 100755
index 000000000000..45c97d4a7d5f
--- /dev/null
+++ b/tools/perf/tests/shell/script_dlfilter.sh
@@ -0,0 +1,107 @@
+#!/bin/bash
+# perf script --dlfilter tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+shelldir=$(dirname "$0")
+# shellcheck source=lib/setup_python.sh
+. "${shelldir}"/lib/setup_python.sh
+
+# skip if there's no compiler
+if ! [ -x "$(command -v cc)" ]; then
+	echo "failed: no compiler, install gcc"
+	exit 2
+fi
+
+err=0
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+dlfilter_c=$(mktemp /tmp/__perf_test.dlfilter.test.c.XXXXX)
+dlfilter_so=$(mktemp /tmp/__perf_test.dlfilter.so.XXXXX)
+
+cleanup() {
+	rm -f "${perfdata}"
+	rm -f "${dlfilter_c}"
+	rm -f "${dlfilter_so}"
+	rm -f "${dlfilter_so}.o"
+
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+cat <<EOF > "${dlfilter_c}"
+#include <perf/perf_dlfilter.h>
+#include <string.h>
+#include <stdio.h>
+
+struct perf_dlfilter_fns perf_dlfilter_fns;
+
+int filter_event(void *data, const struct perf_dlfilter_sample *sample, void *ctx)
+{
+	const struct perf_dlfilter_al *al;
+
+	if (!sample->ip)
+		return 0;
+
+	al = perf_dlfilter_fns.resolve_ip(ctx);
+	if (!al || !al->sym || strcmp(al->sym, "test_loop"))
+		return 1;
+
+	return 0;
+}
+EOF
+
+test_dlfilter() {
+	echo "Basic --dlfilter test"
+	# Generate perf.data file
+	if ! perf record -o "${perfdata}" perf test -w thloop 1 2> /dev/null
+	then
+		echo "Basic --dlfilter test [Failed record]"
+		err=1
+		return
+	fi
+
+	# Build the dlfilter
+	if ! cc -c -I tools/perf/include -fpic -x c "${dlfilter_c}" -o "${dlfilter_so}.o"
+	then
+		echo "Basic --dlfilter test [Failed to build dlfilter object]"
+		err=1
+		return
+	fi
+
+	if ! cc -shared -o "${dlfilter_so}" "${dlfilter_so}.o"
+	then
+		echo "Basic --dlfilter test [Failed to link dlfilter shared object]"
+		err=1
+		return
+	fi
+
+	# Check that the output contains "test_loop" and nothing else
+	if ! perf script -i "${perfdata}" --dlfilter "${dlfilter_so}" | grep -q "test_loop"
+	then
+		echo "Basic --dlfilter test [Failed missing output]"
+		err=1
+		return
+	fi
+
+	# The filter should filter out everything except test_loop, so ensure no other symbols are present
+	# This is a simple check; we could be more rigorous
+	if perf script -i "${perfdata}" --dlfilter "${dlfilter_so}" | grep -v "test_loop" | grep -q "perf"
+	then
+		echo "Basic --dlfilter test [Failed filtering]"
+		err=1
+		return
+	fi
+
+	echo "Basic --dlfilter test [Success]"
+}
+
+test_dlfilter
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 16/17] perf tests evlist: Add basic evlist test

[Ian Rogers]

Add test that evlist reports expected events from perf record.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/evlist.sh | 79 ++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100755 tools/perf/tests/shell/evlist.sh

diff --git a/tools/perf/tests/shell/evlist.sh b/tools/perf/tests/shell/evlist.sh
new file mode 100755
index 000000000000..140f099e75c1
--- /dev/null
+++ b/tools/perf/tests/shell/evlist.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+# perf evlist tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+
+cleanup() {
+	rm -f "${perfdata}"
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+test_evlist_simple() {
+	echo "Simple evlist test"
+	if ! perf record -e cycles -o "${perfdata}" true 2> /dev/null
+	then
+		echo "Simple evlist [Failed record]"
+		err=1
+		return
+	fi
+	if ! perf evlist -i "${perfdata}" | grep -q "cycles"
+	then
+		echo "Simple evlist [Failed to list event]"
+		err=1
+		return
+	fi
+	echo "Simple evlist test [Success]"
+}
+
+test_evlist_group() {
+	echo "Group evlist test"
+	if ! perf record -e "{cycles,instructions}" -o "${perfdata}" true 2> /dev/null
+	then
+		echo "Group evlist [Skipped event group recording failed]"
+		return
+	fi
+
+	if ! perf evlist -i "${perfdata}" -g | grep -q "{.*cycles.*,.*instructions.*}"
+	then
+		echo "Group evlist [Failed to list event group]"
+		err=1
+		return
+	fi
+	echo "Group evlist test [Success]"
+}
+
+test_evlist_verbose() {
+	echo "Event configuration evlist test"
+	if ! perf record -e cycles -o "${perfdata}" true 2> /dev/null
+	then
+		echo "Event configuration evlist [Failed record]"
+		err=1
+		return
+	fi
+
+	if ! perf evlist -i "${perfdata}" -v | grep -q "config:"
+	then
+		echo "Event configuration evlist [Failed to list verbose info]"
+		err=1
+		return
+	fi
+	echo "Event configuration evlist test [Success]"
+}
+
+test_evlist_simple
+test_evlist_group
+test_evlist_verbose
+
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

[PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Ian Rogers]

Setup qemu with KVM then run kvm stat and some host
recording/reporting/build-id tests.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)
 create mode 100755 tools/perf/tests/shell/kvm.sh

diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
new file mode 100755
index 000000000000..2fafde1a29cc
--- /dev/null
+++ b/tools/perf/tests/shell/kvm.sh
@@ -0,0 +1,154 @@
+#!/bin/bash
+# perf kvm tests
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+err=0
+perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
+qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
+
+cleanup() {
+	rm -f "${perfdata}"
+	if [ -f "${qemu_pid_file}" ]; then
+		if [ -s "${qemu_pid_file}" ]; then
+			qemu_pid=$(cat "${qemu_pid_file}")
+			if [ -n "${qemu_pid}" ]; then
+				kill "${qemu_pid}" 2>/dev/null || true
+			fi
+		fi
+		rm -f "${qemu_pid_file}"
+	fi
+	trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+trap trap_cleanup EXIT TERM INT
+
+skip() {
+	echo "Skip: $1"
+	cleanup
+	exit 2
+}
+
+test_kvm_stat() {
+	echo "Testing perf kvm stat"
+
+	echo "Recording kvm events for pid ${qemu_pid}..."
+	if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
+		echo "Failed to record kvm events"
+		err=1
+		return
+	fi
+
+	echo "Reporting kvm events..."
+	if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
+		echo "Failed to find VM-EXIT in report"
+		perf kvm -i "${perfdata}" stat report 2>&1
+		err=1
+		return
+	fi
+
+	echo "perf kvm stat test [Success]"
+}
+
+test_kvm_record_report() {
+	echo "Testing perf kvm record/report"
+
+	echo "Recording kvm profile for pid ${qemu_pid}..."
+	# Use --host to avoid needing guest symbols/mounts for this simple test
+	# We just want to verify the command runs and produces data
+	# We run in background and kill it because 'perf kvm record' appends options
+	# after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
+	perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
+	rec_pid=$!
+	sleep 1
+	kill -INT "${rec_pid}"
+	wait "${rec_pid}" || true
+
+	echo "Reporting kvm profile..."
+	# Check for some standard output from report
+	if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
+		echo "Failed to report kvm profile"
+		perf kvm -i "${perfdata}" report --stdio 2>&1
+		err=1
+		return
+	fi
+
+	echo "perf kvm record/report test [Success]"
+}
+
+test_kvm_buildid_list() {
+	echo "Testing perf kvm buildid-list"
+
+	# We reuse the perf.data from the previous record test
+	if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
+		echo "Failed to list buildids"
+		perf kvm --host -i "${perfdata}" buildid-list 2>&1
+		err=1
+		return
+	fi
+
+	echo "perf kvm buildid-list test [Success]"
+}
+
+setup_qemu() {
+	# Find qemu
+	if [ "$(uname -m)" = "x86_64" ]; then
+		qemu="qemu-system-x86_64"
+	elif [ "$(uname -m)" = "aarch64" ]; then
+		qemu="qemu-system-aarch64"
+	elif [ "$(uname -m)" = "s390x" ]; then
+		qemu="qemu-system-s390x"
+	elif [ "$(uname -m)" = "ppc64le" ]; then
+		qemu="qemu-system-ppc64"
+	else
+		qemu="qemu-system-$(uname -m)"
+	fi
+
+	if ! which -s "$qemu"; then
+		skip "$qemu not found"
+	fi
+
+	if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
+		skip "/dev/kvm not accessible"
+	fi
+
+	if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
+		skip "No permission to record kvm events"
+	fi
+
+	echo "Starting $qemu..."
+	# Start qemu in background, detached, with pidfile
+	# We use -display none -daemonize and a monitor to keep it alive/controllable if needed
+	# We don't need a real kernel, just KVM active.
+	if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then
+		echo "Failed to start qemu"
+		err=1
+		return
+	fi
+
+	# Wait a bit for qemu to start
+	sleep 1
+	qemu_pid=$(cat "${qemu_pid_file}")
+
+	if ! kill -0 "${qemu_pid}" 2>/dev/null; then
+		echo "Qemu process failed to stay alive"
+		err=1
+		return
+	fi
+}
+
+setup_qemu
+if [ $err -eq 0 ]; then
+	test_kvm_stat
+	test_kvm_record_report
+	test_kvm_buildid_list
+fi
+
+cleanup
+exit $err
-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH v1 01/17] perf kvm: Fix debug assertion

[Namhyung Kim]

On Sat, Nov 22, 2025 at 12:19:13AM -0800, Ian Rogers wrote:
> There are 2 slots left for kvm_add_default_arch_event, fix the
> assertion so that debug builds don't fail the assert and to agree with
> the comment.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

Fixes: 45ff39f6e70aa55d0 ("perf tools kvm: Fix the potential out of range memory access issue")

Thanks,
Namhyung

> ---
>  tools/perf/builtin-kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index f0f285763f19..c61369d54dd9 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -2014,7 +2014,7 @@ static int __cmd_record(const char *file_name, int argc, const char **argv)
>  	for (j = 1; j < argc; j++, i++)
>  		rec_argv[i] = STRDUP_FAIL_EXIT(argv[j]);
>  
> -	BUG_ON(i != rec_argc);
> +	BUG_ON(i + 2 != rec_argc);
>  
>  	ret = kvm_add_default_arch_event(&i, rec_argv);
>  	if (ret)
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 02/17] perf timechart: Add record support for output perf.data path

[Namhyung Kim]

On Sat, Nov 22, 2025 at 12:19:14AM -0800, Ian Rogers wrote:
> The '-o' option exists for the SVG creation but not for `perf
> timechart record`. Add to better allow testing.

Can you please update the documentation as well?

Thanks,
Namhyung

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-timechart.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 22050c640dfa..f8b49d69e9a5 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1651,7 +1651,7 @@ static int __cmd_timechart(struct timechart *tchart, const char *output_name)
>  	return ret;
>  }
>  
> -static int timechart__io_record(int argc, const char **argv)
> +static int timechart__io_record(int argc, const char **argv, const char *output_data)
>  {
>  	unsigned int rec_argc, i;
>  	const char **rec_argv;
> @@ -1659,7 +1659,7 @@ static int timechart__io_record(int argc, const char **argv)
>  	char *filter = NULL;
>  
>  	const char * const common_args[] = {
> -		"record", "-a", "-R", "-c", "1",
> +		"record", "-a", "-R", "-c", "1", "-o", output_data,
>  	};
>  	unsigned int common_args_nr = ARRAY_SIZE(common_args);
>  
> @@ -1786,7 +1786,8 @@ static int timechart__io_record(int argc, const char **argv)
>  }
>  
>  
> -static int timechart__record(struct timechart *tchart, int argc, const char **argv)
> +static int timechart__record(struct timechart *tchart, int argc, const char **argv,
> +			     const char *output_data)
>  {
>  	unsigned int rec_argc, i, j;
>  	const char **rec_argv;
> @@ -1794,7 +1795,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
>  	unsigned int record_elems;
>  
>  	const char * const common_args[] = {
> -		"record", "-a", "-R", "-c", "1",
> +		"record", "-a", "-R", "-c", "1", "-o", output_data,
>  	};
>  	unsigned int common_args_nr = ARRAY_SIZE(common_args);
>  
> @@ -1934,6 +1935,7 @@ int cmd_timechart(int argc, const char **argv)
>  		.merge_dist = 1000,
>  	};
>  	const char *output_name = "output.svg";
> +	const char *output_record_data = "perf.data";
>  	const struct option timechart_common_options[] = {
>  	OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
>  	OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only, "output processes data only"),
> @@ -1976,6 +1978,7 @@ int cmd_timechart(int argc, const char **argv)
>  	OPT_BOOLEAN('I', "io-only", &tchart.io_only,
>  		    "record only IO data"),
>  	OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
> +	OPT_STRING('o', "output", &output_record_data, "file", "output data file name"),
>  	OPT_PARENT(timechart_common_options),
>  	};
>  	const char * const timechart_record_usage[] = {
> @@ -2024,9 +2027,9 @@ int cmd_timechart(int argc, const char **argv)
>  		}
>  
>  		if (tchart.io_only)
> -			ret = timechart__io_record(argc, argv);
> +			ret = timechart__io_record(argc, argv, output_record_data);
>  		else
> -			ret = timechart__record(&tchart, argc, argv);
> +			ret = timechart__record(&tchart, argc, argv, output_record_data);
>  		goto out;
>  	} else if (argc)
>  		usage_with_options(timechart_usage, timechart_options);
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 05/17] perf probe-event: Ensure probe event nsinfo is always cleared

[Namhyung Kim]

On Sat, Nov 22, 2025 at 12:19:17AM -0800, Ian Rogers wrote:
> Move nsinfo__zput from cleanup_perf_probe_events to
> clear_perf_probe_event so it is always executed. Clean up
> clear_perf_probe_event to not call nsinfo__zput and use the pev

Do you mean clear_perf_probe_events?


> variable to avoid repeated array accesses.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/probe-event.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 6ab2eb551b6c..710e4620923e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2419,6 +2419,7 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
>  	}
>  	pev->nargs = 0;
>  	zfree(&pev->args);
> +	nsinfo__zput(pev->nsi);

I'm not sure pev->nsi is always set properly.  At least it seems to miss
in the perf_probe_event__copy().

Thanks,
Namhyung


>  }
>  
>  #define strdup_or_goto(str, label)	\
> @@ -3767,12 +3768,11 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  	/* Loop 3: cleanup and free trace events  */
>  	for (i = 0; i < npevs; i++) {
>  		pev = &pevs[i];
> -		for (j = 0; j < pevs[i].ntevs; j++)
> -			clear_probe_trace_event(&pevs[i].tevs[j]);
> -		zfree(&pevs[i].tevs);
> -		pevs[i].ntevs = 0;
> -		nsinfo__zput(pev->nsi);
> -		clear_perf_probe_event(&pevs[i]);
> +		for (j = 0; j < pev->ntevs; j++)
> +			clear_probe_trace_event(&pev->tevs[j]);
> +		zfree(&pev->tevs);
> +		pev->ntevs = 0;
> +		clear_perf_probe_event(pev);
>  	}
>  }
>  
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 16/17] perf tests evlist: Add basic evlist test

[Namhyung Kim]

On Sat, Nov 22, 2025 at 12:19:28AM -0800, Ian Rogers wrote:
> Add test that evlist reports expected events from perf record.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/evlist.sh | 79 ++++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100755 tools/perf/tests/shell/evlist.sh
> 
> diff --git a/tools/perf/tests/shell/evlist.sh b/tools/perf/tests/shell/evlist.sh
> new file mode 100755
> index 000000000000..140f099e75c1
> --- /dev/null
> +++ b/tools/perf/tests/shell/evlist.sh
> @@ -0,0 +1,79 @@
> +#!/bin/bash
> +# perf evlist tests
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +err=0
> +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> +
> +cleanup() {
> +	rm -f "${perfdata}"
> +	trap - EXIT TERM INT
> +}
> +
> +trap_cleanup() {
> +	echo "Unexpected signal in ${FUNCNAME[1]}"
> +	cleanup
> +	exit 1
> +}
> +trap trap_cleanup EXIT TERM INT
> +
> +test_evlist_simple() {
> +	echo "Simple evlist test"
> +	if ! perf record -e cycles -o "${perfdata}" true 2> /dev/null

I think we need to use a software event to make sure it can run on any
platforms.

Thanks,
Namhyung


> +	then
> +		echo "Simple evlist [Failed record]"
> +		err=1
> +		return
> +	fi
> +	if ! perf evlist -i "${perfdata}" | grep -q "cycles"
> +	then
> +		echo "Simple evlist [Failed to list event]"
> +		err=1
> +		return
> +	fi
> +	echo "Simple evlist test [Success]"
> +}
> +
> +test_evlist_group() {
> +	echo "Group evlist test"
> +	if ! perf record -e "{cycles,instructions}" -o "${perfdata}" true 2> /dev/null
> +	then
> +		echo "Group evlist [Skipped event group recording failed]"
> +		return
> +	fi
> +
> +	if ! perf evlist -i "${perfdata}" -g | grep -q "{.*cycles.*,.*instructions.*}"
> +	then
> +		echo "Group evlist [Failed to list event group]"
> +		err=1
> +		return
> +	fi
> +	echo "Group evlist test [Success]"
> +}
> +
> +test_evlist_verbose() {
> +	echo "Event configuration evlist test"
> +	if ! perf record -e cycles -o "${perfdata}" true 2> /dev/null
> +	then
> +		echo "Event configuration evlist [Failed record]"
> +		err=1
> +		return
> +	fi
> +
> +	if ! perf evlist -i "${perfdata}" -v | grep -q "config:"
> +	then
> +		echo "Event configuration evlist [Failed to list verbose info]"
> +		err=1
> +		return
> +	fi
> +	echo "Event configuration evlist test [Success]"
> +}
> +
> +test_evlist_simple
> +test_evlist_group
> +test_evlist_verbose
> +
> +cleanup
> +exit $err
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Namhyung Kim]

On Sat, Nov 22, 2025 at 12:19:29AM -0800, Ian Rogers wrote:
> Setup qemu with KVM then run kvm stat and some host
> recording/reporting/build-id tests.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
>  1 file changed, 154 insertions(+)
>  create mode 100755 tools/perf/tests/shell/kvm.sh
> 
> diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> new file mode 100755
> index 000000000000..2fafde1a29cc
> --- /dev/null
> +++ b/tools/perf/tests/shell/kvm.sh
> @@ -0,0 +1,154 @@
> +#!/bin/bash
> +# perf kvm tests
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +err=0
> +perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
> +qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
> +
> +cleanup() {
> +	rm -f "${perfdata}"
> +	if [ -f "${qemu_pid_file}" ]; then
> +		if [ -s "${qemu_pid_file}" ]; then
> +			qemu_pid=$(cat "${qemu_pid_file}")
> +			if [ -n "${qemu_pid}" ]; then
> +				kill "${qemu_pid}" 2>/dev/null || true
> +			fi
> +		fi
> +		rm -f "${qemu_pid_file}"
> +	fi
> +	trap - EXIT TERM INT
> +}
> +
> +trap_cleanup() {
> +	echo "Unexpected signal in ${FUNCNAME[1]}"
> +	cleanup
> +	exit 1
> +}
> +trap trap_cleanup EXIT TERM INT
> +
> +skip() {
> +	echo "Skip: $1"
> +	cleanup
> +	exit 2
> +}
> +
> +test_kvm_stat() {
> +	echo "Testing perf kvm stat"
> +
> +	echo "Recording kvm events for pid ${qemu_pid}..."
> +	if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
> +		echo "Failed to record kvm events"
> +		err=1
> +		return
> +	fi
> +
> +	echo "Reporting kvm events..."
> +	if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
> +		echo "Failed to find VM-EXIT in report"
> +		perf kvm -i "${perfdata}" stat report 2>&1
> +		err=1
> +		return
> +	fi
> +
> +	echo "perf kvm stat test [Success]"
> +}
> +
> +test_kvm_record_report() {
> +	echo "Testing perf kvm record/report"
> +
> +	echo "Recording kvm profile for pid ${qemu_pid}..."
> +	# Use --host to avoid needing guest symbols/mounts for this simple test
> +	# We just want to verify the command runs and produces data
> +	# We run in background and kill it because 'perf kvm record' appends options
> +	# after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> +	perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> +	rec_pid=$!
> +	sleep 1
> +	kill -INT "${rec_pid}"
> +	wait "${rec_pid}" || true
> +
> +	echo "Reporting kvm profile..."
> +	# Check for some standard output from report
> +	if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
> +		echo "Failed to report kvm profile"
> +		perf kvm -i "${perfdata}" report --stdio 2>&1
> +		err=1
> +		return
> +	fi
> +
> +	echo "perf kvm record/report test [Success]"
> +}
> +
> +test_kvm_buildid_list() {
> +	echo "Testing perf kvm buildid-list"
> +
> +	# We reuse the perf.data from the previous record test
> +	if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
> +		echo "Failed to list buildids"
> +		perf kvm --host -i "${perfdata}" buildid-list 2>&1
> +		err=1
> +		return
> +	fi
> +
> +	echo "perf kvm buildid-list test [Success]"
> +}
> +
> +setup_qemu() {
> +	# Find qemu
> +	if [ "$(uname -m)" = "x86_64" ]; then
> +		qemu="qemu-system-x86_64"
> +	elif [ "$(uname -m)" = "aarch64" ]; then
> +		qemu="qemu-system-aarch64"
> +	elif [ "$(uname -m)" = "s390x" ]; then
> +		qemu="qemu-system-s390x"
> +	elif [ "$(uname -m)" = "ppc64le" ]; then
> +		qemu="qemu-system-ppc64"
> +	else
> +		qemu="qemu-system-$(uname -m)"
> +	fi
> +
> +	if ! which -s "$qemu"; then
> +		skip "$qemu not found"
> +	fi
> +
> +	if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
> +		skip "/dev/kvm not accessible"
> +	fi
> +
> +	if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
> +		skip "No permission to record kvm events"
> +	fi
> +
> +	echo "Starting $qemu..."
> +	# Start qemu in background, detached, with pidfile
> +	# We use -display none -daemonize and a monitor to keep it alive/controllable if needed
> +	# We don't need a real kernel, just KVM active.
> +	if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then

What is running in the guest?

Thanks,
Namhyung


> +		echo "Failed to start qemu"
> +		err=1
> +		return
> +	fi
> +
> +	# Wait a bit for qemu to start
> +	sleep 1
> +	qemu_pid=$(cat "${qemu_pid_file}")
> +
> +	if ! kill -0 "${qemu_pid}" 2>/dev/null; then
> +		echo "Qemu process failed to stay alive"
> +		err=1
> +		return
> +	fi
> +}
> +
> +setup_qemu
> +if [ $err -eq 0 ]; then
> +	test_kvm_stat
> +	test_kvm_record_report
> +	test_kvm_buildid_list
> +fi
> +
> +cleanup
> +exit $err
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Ian Rogers]

On Tue, Nov 25, 2025 at 11:21 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Sat, Nov 22, 2025 at 12:19:29AM -0800, Ian Rogers wrote:
> > Setup qemu with KVM then run kvm stat and some host
> > recording/reporting/build-id tests.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 154 insertions(+)
> >  create mode 100755 tools/perf/tests/shell/kvm.sh
> >
> > diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> > new file mode 100755
> > index 000000000000..2fafde1a29cc
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/kvm.sh
> > @@ -0,0 +1,154 @@
> > +#!/bin/bash
> > +# perf kvm tests
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +
> > +err=0
> > +perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
> > +qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
> > +
> > +cleanup() {
> > +     rm -f "${perfdata}"
> > +     if [ -f "${qemu_pid_file}" ]; then
> > +             if [ -s "${qemu_pid_file}" ]; then
> > +                     qemu_pid=$(cat "${qemu_pid_file}")
> > +                     if [ -n "${qemu_pid}" ]; then
> > +                             kill "${qemu_pid}" 2>/dev/null || true
> > +                     fi
> > +             fi
> > +             rm -f "${qemu_pid_file}"
> > +     fi
> > +     trap - EXIT TERM INT
> > +}
> > +
> > +trap_cleanup() {
> > +     echo "Unexpected signal in ${FUNCNAME[1]}"
> > +     cleanup
> > +     exit 1
> > +}
> > +trap trap_cleanup EXIT TERM INT
> > +
> > +skip() {
> > +     echo "Skip: $1"
> > +     cleanup
> > +     exit 2
> > +}
> > +
> > +test_kvm_stat() {
> > +     echo "Testing perf kvm stat"
> > +
> > +     echo "Recording kvm events for pid ${qemu_pid}..."
> > +     if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
> > +             echo "Failed to record kvm events"
> > +             err=1
> > +             return
> > +     fi
> > +
> > +     echo "Reporting kvm events..."
> > +     if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
> > +             echo "Failed to find VM-EXIT in report"
> > +             perf kvm -i "${perfdata}" stat report 2>&1
> > +             err=1
> > +             return
> > +     fi
> > +
> > +     echo "perf kvm stat test [Success]"
> > +}
> > +
> > +test_kvm_record_report() {
> > +     echo "Testing perf kvm record/report"
> > +
> > +     echo "Recording kvm profile for pid ${qemu_pid}..."
> > +     # Use --host to avoid needing guest symbols/mounts for this simple test
> > +     # We just want to verify the command runs and produces data
> > +     # We run in background and kill it because 'perf kvm record' appends options
> > +     # after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> > +     perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> > +     rec_pid=$!
> > +     sleep 1
> > +     kill -INT "${rec_pid}"
> > +     wait "${rec_pid}" || true
> > +
> > +     echo "Reporting kvm profile..."
> > +     # Check for some standard output from report
> > +     if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
> > +             echo "Failed to report kvm profile"
> > +             perf kvm -i "${perfdata}" report --stdio 2>&1
> > +             err=1
> > +             return
> > +     fi
> > +
> > +     echo "perf kvm record/report test [Success]"
> > +}
> > +
> > +test_kvm_buildid_list() {
> > +     echo "Testing perf kvm buildid-list"
> > +
> > +     # We reuse the perf.data from the previous record test
> > +     if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
> > +             echo "Failed to list buildids"
> > +             perf kvm --host -i "${perfdata}" buildid-list 2>&1
> > +             err=1
> > +             return
> > +     fi
> > +
> > +     echo "perf kvm buildid-list test [Success]"
> > +}
> > +
> > +setup_qemu() {
> > +     # Find qemu
> > +     if [ "$(uname -m)" = "x86_64" ]; then
> > +             qemu="qemu-system-x86_64"
> > +     elif [ "$(uname -m)" = "aarch64" ]; then
> > +             qemu="qemu-system-aarch64"
> > +     elif [ "$(uname -m)" = "s390x" ]; then
> > +             qemu="qemu-system-s390x"
> > +     elif [ "$(uname -m)" = "ppc64le" ]; then
> > +             qemu="qemu-system-ppc64"
> > +     else
> > +             qemu="qemu-system-$(uname -m)"
> > +     fi
> > +
> > +     if ! which -s "$qemu"; then
> > +             skip "$qemu not found"
> > +     fi
> > +
> > +     if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
> > +             skip "/dev/kvm not accessible"
> > +     fi
> > +
> > +     if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
> > +             skip "No permission to record kvm events"
> > +     fi
> > +
> > +     echo "Starting $qemu..."
> > +     # Start qemu in background, detached, with pidfile
> > +     # We use -display none -daemonize and a monitor to keep it alive/controllable if needed
> > +     # We don't need a real kernel, just KVM active.
> > +     if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then
>
> What is running in the guest?

Nothing. As no kernel or storage image were provided the guest should
be waiting at the bios similar to if you booted a PC with no storage
in it. I just wanted something quick and dirty to give the basic perf
kvm command a simple work out for the sake of testing coverage.

Thanks,
Ian

> Thanks,
> Namhyung
>
>
> > +             echo "Failed to start qemu"
> > +             err=1
> > +             return
> > +     fi
> > +
> > +     # Wait a bit for qemu to start
> > +     sleep 1
> > +     qemu_pid=$(cat "${qemu_pid_file}")
> > +
> > +     if ! kill -0 "${qemu_pid}" 2>/dev/null; then
> > +             echo "Qemu process failed to stay alive"
> > +             err=1
> > +             return
> > +     fi
> > +}
> > +
> > +setup_qemu
> > +if [ $err -eq 0 ]; then
> > +     test_kvm_stat
> > +     test_kvm_record_report
> > +     test_kvm_buildid_list
> > +fi
> > +
> > +cleanup
> > +exit $err
> > --
> > 2.52.0.rc2.455.g230fcf2819-goog
> >

Re: [PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Namhyung Kim]

On Thu, Nov 27, 2025 at 03:53:13AM -0800, Ian Rogers wrote:
> On Tue, Nov 25, 2025 at 11:21 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Sat, Nov 22, 2025 at 12:19:29AM -0800, Ian Rogers wrote:
> > > Setup qemu with KVM then run kvm stat and some host
> > > recording/reporting/build-id tests.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 154 insertions(+)
> > >  create mode 100755 tools/perf/tests/shell/kvm.sh
> > >
> > > diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> > > new file mode 100755
> > > index 000000000000..2fafde1a29cc
> > > --- /dev/null
> > > +++ b/tools/perf/tests/shell/kvm.sh
> > > @@ -0,0 +1,154 @@
> > > +#!/bin/bash
> > > +# perf kvm tests
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +set -e
> > > +
> > > +err=0
> > > +perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
> > > +qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
> > > +
> > > +cleanup() {
> > > +     rm -f "${perfdata}"
> > > +     if [ -f "${qemu_pid_file}" ]; then
> > > +             if [ -s "${qemu_pid_file}" ]; then
> > > +                     qemu_pid=$(cat "${qemu_pid_file}")
> > > +                     if [ -n "${qemu_pid}" ]; then
> > > +                             kill "${qemu_pid}" 2>/dev/null || true
> > > +                     fi
> > > +             fi
> > > +             rm -f "${qemu_pid_file}"
> > > +     fi
> > > +     trap - EXIT TERM INT
> > > +}
> > > +
> > > +trap_cleanup() {
> > > +     echo "Unexpected signal in ${FUNCNAME[1]}"
> > > +     cleanup
> > > +     exit 1
> > > +}
> > > +trap trap_cleanup EXIT TERM INT
> > > +
> > > +skip() {
> > > +     echo "Skip: $1"
> > > +     cleanup
> > > +     exit 2
> > > +}
> > > +
> > > +test_kvm_stat() {
> > > +     echo "Testing perf kvm stat"
> > > +
> > > +     echo "Recording kvm events for pid ${qemu_pid}..."
> > > +     if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
> > > +             echo "Failed to record kvm events"
> > > +             err=1
> > > +             return
> > > +     fi
> > > +
> > > +     echo "Reporting kvm events..."
> > > +     if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
> > > +             echo "Failed to find VM-EXIT in report"
> > > +             perf kvm -i "${perfdata}" stat report 2>&1
> > > +             err=1
> > > +             return
> > > +     fi
> > > +
> > > +     echo "perf kvm stat test [Success]"
> > > +}
> > > +
> > > +test_kvm_record_report() {
> > > +     echo "Testing perf kvm record/report"
> > > +
> > > +     echo "Recording kvm profile for pid ${qemu_pid}..."
> > > +     # Use --host to avoid needing guest symbols/mounts for this simple test
> > > +     # We just want to verify the command runs and produces data
> > > +     # We run in background and kill it because 'perf kvm record' appends options
> > > +     # after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> > > +     perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> > > +     rec_pid=$!
> > > +     sleep 1
> > > +     kill -INT "${rec_pid}"
> > > +     wait "${rec_pid}" || true
> > > +
> > > +     echo "Reporting kvm profile..."
> > > +     # Check for some standard output from report
> > > +     if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
> > > +             echo "Failed to report kvm profile"
> > > +             perf kvm -i "${perfdata}" report --stdio 2>&1
> > > +             err=1
> > > +             return
> > > +     fi
> > > +
> > > +     echo "perf kvm record/report test [Success]"
> > > +}
> > > +
> > > +test_kvm_buildid_list() {
> > > +     echo "Testing perf kvm buildid-list"
> > > +
> > > +     # We reuse the perf.data from the previous record test
> > > +     if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
> > > +             echo "Failed to list buildids"
> > > +             perf kvm --host -i "${perfdata}" buildid-list 2>&1
> > > +             err=1
> > > +             return
> > > +     fi
> > > +
> > > +     echo "perf kvm buildid-list test [Success]"
> > > +}
> > > +
> > > +setup_qemu() {
> > > +     # Find qemu
> > > +     if [ "$(uname -m)" = "x86_64" ]; then
> > > +             qemu="qemu-system-x86_64"
> > > +     elif [ "$(uname -m)" = "aarch64" ]; then
> > > +             qemu="qemu-system-aarch64"
> > > +     elif [ "$(uname -m)" = "s390x" ]; then
> > > +             qemu="qemu-system-s390x"
> > > +     elif [ "$(uname -m)" = "ppc64le" ]; then
> > > +             qemu="qemu-system-ppc64"
> > > +     else
> > > +             qemu="qemu-system-$(uname -m)"
> > > +     fi
> > > +
> > > +     if ! which -s "$qemu"; then
> > > +             skip "$qemu not found"
> > > +     fi
> > > +
> > > +     if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
> > > +             skip "/dev/kvm not accessible"
> > > +     fi
> > > +
> > > +     if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
> > > +             skip "No permission to record kvm events"
> > > +     fi
> > > +
> > > +     echo "Starting $qemu..."
> > > +     # Start qemu in background, detached, with pidfile
> > > +     # We use -display none -daemonize and a monitor to keep it alive/controllable if needed
> > > +     # We don't need a real kernel, just KVM active.
> > > +     if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then
> >
> > What is running in the guest?
> 
> Nothing. As no kernel or storage image were provided the guest should
> be waiting at the bios similar to if you booted a PC with no storage
> in it. I just wanted something quick and dirty to give the basic perf
> kvm command a simple work out for the sake of testing coverage.

Ok, it'd be fine as long as perf can get some samples in the guest.

Thanks,
Namhyung

Re: [PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Ian Rogers]

On Thu, Nov 27, 2025 at 12:55 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Nov 27, 2025 at 03:53:13AM -0800, Ian Rogers wrote:
> > On Tue, Nov 25, 2025 at 11:21 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > On Sat, Nov 22, 2025 at 12:19:29AM -0800, Ian Rogers wrote:
> > > > Setup qemu with KVM then run kvm stat and some host
> > > > recording/reporting/build-id tests.
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 154 insertions(+)
> > > >  create mode 100755 tools/perf/tests/shell/kvm.sh
> > > >
> > > > diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> > > > new file mode 100755
> > > > index 000000000000..2fafde1a29cc
> > > > --- /dev/null
> > > > +++ b/tools/perf/tests/shell/kvm.sh
> > > > @@ -0,0 +1,154 @@
> > > > +#!/bin/bash
> > > > +# perf kvm tests
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +set -e
> > > > +
> > > > +err=0
> > > > +perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
> > > > +qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
> > > > +
> > > > +cleanup() {
> > > > +     rm -f "${perfdata}"
> > > > +     if [ -f "${qemu_pid_file}" ]; then
> > > > +             if [ -s "${qemu_pid_file}" ]; then
> > > > +                     qemu_pid=$(cat "${qemu_pid_file}")
> > > > +                     if [ -n "${qemu_pid}" ]; then
> > > > +                             kill "${qemu_pid}" 2>/dev/null || true
> > > > +                     fi
> > > > +             fi
> > > > +             rm -f "${qemu_pid_file}"
> > > > +     fi
> > > > +     trap - EXIT TERM INT
> > > > +}
> > > > +
> > > > +trap_cleanup() {
> > > > +     echo "Unexpected signal in ${FUNCNAME[1]}"
> > > > +     cleanup
> > > > +     exit 1
> > > > +}
> > > > +trap trap_cleanup EXIT TERM INT
> > > > +
> > > > +skip() {
> > > > +     echo "Skip: $1"
> > > > +     cleanup
> > > > +     exit 2
> > > > +}
> > > > +
> > > > +test_kvm_stat() {
> > > > +     echo "Testing perf kvm stat"
> > > > +
> > > > +     echo "Recording kvm events for pid ${qemu_pid}..."
> > > > +     if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
> > > > +             echo "Failed to record kvm events"
> > > > +             err=1
> > > > +             return
> > > > +     fi
> > > > +
> > > > +     echo "Reporting kvm events..."
> > > > +     if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
> > > > +             echo "Failed to find VM-EXIT in report"
> > > > +             perf kvm -i "${perfdata}" stat report 2>&1
> > > > +             err=1
> > > > +             return
> > > > +     fi
> > > > +
> > > > +     echo "perf kvm stat test [Success]"
> > > > +}
> > > > +
> > > > +test_kvm_record_report() {
> > > > +     echo "Testing perf kvm record/report"
> > > > +
> > > > +     echo "Recording kvm profile for pid ${qemu_pid}..."
> > > > +     # Use --host to avoid needing guest symbols/mounts for this simple test
> > > > +     # We just want to verify the command runs and produces data
> > > > +     # We run in background and kill it because 'perf kvm record' appends options
> > > > +     # after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> > > > +     perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> > > > +     rec_pid=$!
> > > > +     sleep 1
> > > > +     kill -INT "${rec_pid}"
> > > > +     wait "${rec_pid}" || true
> > > > +
> > > > +     echo "Reporting kvm profile..."
> > > > +     # Check for some standard output from report
> > > > +     if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
> > > > +             echo "Failed to report kvm profile"
> > > > +             perf kvm -i "${perfdata}" report --stdio 2>&1
> > > > +             err=1
> > > > +             return
> > > > +     fi
> > > > +
> > > > +     echo "perf kvm record/report test [Success]"
> > > > +}
> > > > +
> > > > +test_kvm_buildid_list() {
> > > > +     echo "Testing perf kvm buildid-list"
> > > > +
> > > > +     # We reuse the perf.data from the previous record test
> > > > +     if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
> > > > +             echo "Failed to list buildids"
> > > > +             perf kvm --host -i "${perfdata}" buildid-list 2>&1
> > > > +             err=1
> > > > +             return
> > > > +     fi
> > > > +
> > > > +     echo "perf kvm buildid-list test [Success]"
> > > > +}
> > > > +
> > > > +setup_qemu() {
> > > > +     # Find qemu
> > > > +     if [ "$(uname -m)" = "x86_64" ]; then
> > > > +             qemu="qemu-system-x86_64"
> > > > +     elif [ "$(uname -m)" = "aarch64" ]; then
> > > > +             qemu="qemu-system-aarch64"
> > > > +     elif [ "$(uname -m)" = "s390x" ]; then
> > > > +             qemu="qemu-system-s390x"
> > > > +     elif [ "$(uname -m)" = "ppc64le" ]; then
> > > > +             qemu="qemu-system-ppc64"
> > > > +     else
> > > > +             qemu="qemu-system-$(uname -m)"
> > > > +     fi
> > > > +
> > > > +     if ! which -s "$qemu"; then
> > > > +             skip "$qemu not found"
> > > > +     fi
> > > > +
> > > > +     if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
> > > > +             skip "/dev/kvm not accessible"
> > > > +     fi
> > > > +
> > > > +     if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
> > > > +             skip "No permission to record kvm events"
> > > > +     fi
> > > > +
> > > > +     echo "Starting $qemu..."
> > > > +     # Start qemu in background, detached, with pidfile
> > > > +     # We use -display none -daemonize and a monitor to keep it alive/controllable if needed
> > > > +     # We don't need a real kernel, just KVM active.
> > > > +     if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then
> > >
> > > What is running in the guest?
> >
> > Nothing. As no kernel or storage image were provided the guest should
> > be waiting at the bios similar to if you booted a PC with no storage
> > in it. I just wanted something quick and dirty to give the basic perf
> > kvm command a simple work out for the sake of testing coverage.
>
> Ok, it'd be fine as long as perf can get some samples in the guest.

I hope we always get a sample in the guest, but I don't think it is a
given as qemu is running in the background, etc. Even without guest
samples the code is getting coverage in the perf tool we don't get by
other means, for example, finding the broken BUG_ON.

Thanks,
Ian

> Thanks,
> Namhyung
>

Re: [PATCH v1 08/17] perf jitdump: Fix missed dso__put

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:20AM -0800, Ian Rogers wrote:
> Reference count checking caught a missing dso__put following a
> machine__findnew_dso_id.

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/jitdump.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index b062b1f234b6..27a2ea1b000b 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -546,6 +546,8 @@ static int jit_repipe_code_load(struct jit_buf_desc *jd, union jr_entry *jr)
>  
>  		if (dso)
>  			dso__set_hit(dso);
> +
> +		dso__put(dso);
>  	}
>  out:
>  	perf_sample__exit(&sample);
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 07/17] perf mem-events: Don't leak online CPU map

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:19AM -0800, Ian Rogers wrote:
> Reference count checking found the online CPU map was being gotten but
> not put. Add in the missing put.

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/mem-events.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 80b3069427bc..0b49fce251fc 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -303,12 +303,15 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr, char **eve
>  	}
>  
>  	if (cpu_map) {
> -		if (!perf_cpu_map__equal(cpu_map, cpu_map__online())) {
> +		struct perf_cpu_map *online = cpu_map__online();
> +
> +		if (!perf_cpu_map__equal(cpu_map, online)) {
>  			char buf[200];
>  
>  			cpu_map__snprint(cpu_map, buf, sizeof(buf));
>  			pr_warning("Memory events are enabled on a subset of CPUs: %s\n", buf);
>  		}
> +		perf_cpu_map__put(online);
>  		perf_cpu_map__put(cpu_map);
>  	}
>  
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH v1 06/17] perf hist: In init, ensure mem_info is put on error paths

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:18AM -0800, Ian Rogers wrote:
> Rather than exit the internal map_symbols directly, put the mem-info
> that does this and also lowers the reference count on the mem-info
> itself otherwise the mem-info is being leaked.

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/hist.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 64ff427040c3..ef4b569f7df4 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
>  		map_symbol__exit(&he->branch_info->to.ms);
>  		zfree(&he->branch_info);
>  	}
> -	if (he->mem_info) {
> -		map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
> -		map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
> -	}
> +	if (he->mem_info)
> +		mem_info__zput(he->mem_info);
>  err:
>  	map_symbol__exit(&he->ms);
>  	zfree(&he->stat_acc);
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 06/17] perf hist: In init, ensure mem_info is put on error paths

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:18AM -0800, Ian Rogers wrote:
> Rather than exit the internal map_symbols directly, put the mem-info
> that does this and also lowers the reference count on the mem-info
> itself otherwise the mem-info is being leaked.

Shouldn't this have a:

Fixes: 56e144fe98260a0f ("perf mem_info: Add and use map_symbol__exit and addr_map_symbol__exit")

Other than that:

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/hist.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 64ff427040c3..ef4b569f7df4 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
>  		map_symbol__exit(&he->branch_info->to.ms);
>  		zfree(&he->branch_info);
>  	}
> -	if (he->mem_info) {
> -		map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
> -		map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
> -	}
> +	if (he->mem_info)
> +		mem_info__zput(he->mem_info);
>  err:
>  	map_symbol__exit(&he->ms);
>  	zfree(&he->stat_acc);
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

Re: [PATCH v1 09/17] perf c2c: Clean up some defensive gets and make asan clean

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:21AM -0800, Ian Rogers wrote:
> To deal with histogram code that had missing gets the c2c code had
> some defensive gets. Those other issues were cleaned up by the
> reference count checker, clean them up for the c2c command here.

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-c2c.c | 36 ++++++++++++++----------------------
>  1 file changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 14c3823f8fed..d390ae4e3ec8 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -322,7 +322,7 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  	struct c2c_stats stats = { .nr_entries = 0, };
>  	struct hist_entry *he;
>  	struct addr_location al;
> -	struct mem_info *mi, *mi_dup;
> +	struct mem_info *mi = NULL;
>  	struct callchain_cursor *cursor;
>  	int ret;
>  
> @@ -349,20 +349,15 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  		goto out;
>  	}
>  
> -	/*
> -	 * The mi object is released in hists__add_entry_ops,
> -	 * if it gets sorted out into existing data, so we need
> -	 * to take the copy now.
> -	 */
> -	mi_dup = mem_info__get(mi);
> -
>  	c2c_decode_stats(&stats, mi);
>  
>  	he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
>  				  &al, NULL, NULL, mi, NULL,
>  				  sample, true);
> -	if (he == NULL)
> -		goto free_mi;
> +	if (he == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
>  
>  	c2c_he = container_of(he, struct c2c_hist_entry, he);
>  	c2c_add_stats(&c2c_he->stats, &stats);
> @@ -393,17 +388,19 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  		int cpu = sample->cpu == (unsigned int) -1 ? 0 : sample->cpu;
>  		int node = c2c.cpu2node[cpu];
>  
> -		mi = mi_dup;
> -
>  		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2, machine->env);
> -		if (!c2c_hists)
> -			goto free_mi;
> +		if (!c2c_hists) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  
>  		he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
>  					  &al, NULL, NULL, mi, NULL,
>  					  sample, true);
> -		if (he == NULL)
> -			goto free_mi;
> +		if (he == NULL) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
>  
>  		c2c_he = container_of(he, struct c2c_hist_entry, he);
>  		c2c_add_stats(&c2c_he->stats, &stats);
> @@ -421,14 +418,9 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  	}
>  
>  out:
> +	mem_info__put(mi);
>  	addr_location__exit(&al);
>  	return ret;
> -
> -free_mi:
> -	mem_info__put(mi_dup);
> -	mem_info__put(mi);
> -	ret = -ENOMEM;
> -	goto out;
>  }
>  
>  static const char * const c2c_usage[] = {
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH v1 06/17] perf hist: In init, ensure mem_info is put on error paths

[Ian Rogers]

On Mon, Dec 1, 2025 at 9:14 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Sat, Nov 22, 2025 at 12:19:18AM -0800, Ian Rogers wrote:
> > Rather than exit the internal map_symbols directly, put the mem-info
> > that does this and also lowers the reference count on the mem-info
> > itself otherwise the mem-info is being leaked.
>
> Shouldn't this have a:
>
> Fixes: 56e144fe98260a0f ("perf mem_info: Add and use map_symbol__exit and addr_map_symbol__exit")

I don't mind the fixes tag. The change in that patch was:
```
        if (he->mem_info) {
-               map__put(he->mem_info->iaddr.ms.map);
-               map__put(he->mem_info->daddr.ms.map);
+               map_symbol__exit(&he->mem_info->iaddr.ms);
+               map_symbol__exit(&he->mem_info->daddr.ms);
       }
```
and so technically the memory leak predated that change.

Thanks,
Ian

> Other than that:
>
> Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/hist.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index 64ff427040c3..ef4b569f7df4 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -608,10 +608,8 @@ static int hist_entry__init(struct hist_entry *he,
> >               map_symbol__exit(&he->branch_info->to.ms);
> >               zfree(&he->branch_info);
> >       }
> > -     if (he->mem_info) {
> > -             map_symbol__exit(&mem_info__iaddr(he->mem_info)->ms);
> > -             map_symbol__exit(&mem_info__daddr(he->mem_info)->ms);
> > -     }
> > +     if (he->mem_info)
> > +             mem_info__zput(he->mem_info);
> >  err:
> >       map_symbol__exit(&he->ms);
> >       zfree(&he->stat_acc);
> > --
> > 2.52.0.rc2.455.g230fcf2819-goog
> >

Re: [PATCH v1 03/17] perf symbol-elf: Add missing puts on error path

[Arnaldo Carvalho de Melo]

On Sat, Nov 22, 2025 at 12:19:15AM -0800, Ian Rogers wrote:
> In dso__process_kernel_symbol if inserting a map fails, probably
> ENOMEM, then the reference count puts were missing on the dso and map.

Reviewed-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/symbol-elf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 9e820599bab3..4c76e680b13d 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1446,8 +1446,11 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
>  			map__set_mapping_type(curr_map, MAPPING_TYPE__IDENTITY);
>  		}
>  		dso__set_symtab_type(curr_dso, dso__symtab_type(dso));
> -		if (maps__insert(kmaps, curr_map))
> +		if (maps__insert(kmaps, curr_map)) {
> +			dso__put(curr_dso);
> +			map__put(curr_map);
>  			return -1;
> +		}
>  		dsos__add(&maps__machine(kmaps)->dsos, curr_dso);
>  		dso__set_loaded(curr_dso);
>  		dso__put(*curr_dsop);
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH v1 17/17] perf test kvm: Add some basic perf kvm test coverage

[Namhyung Kim]

On Fri, Nov 28, 2025 at 01:06:22AM -0800, Ian Rogers wrote:
> On Thu, Nov 27, 2025 at 12:55 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Thu, Nov 27, 2025 at 03:53:13AM -0800, Ian Rogers wrote:
> > > On Tue, Nov 25, 2025 at 11:21 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > >
> > > > On Sat, Nov 22, 2025 at 12:19:29AM -0800, Ian Rogers wrote:
> > > > > Setup qemu with KVM then run kvm stat and some host
> > > > > recording/reporting/build-id tests.
> > > > >
> > > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > > ---
> > > > >  tools/perf/tests/shell/kvm.sh | 154 ++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 154 insertions(+)
> > > > >  create mode 100755 tools/perf/tests/shell/kvm.sh
> > > > >
> > > > > diff --git a/tools/perf/tests/shell/kvm.sh b/tools/perf/tests/shell/kvm.sh
> > > > > new file mode 100755
> > > > > index 000000000000..2fafde1a29cc
> > > > > --- /dev/null
> > > > > +++ b/tools/perf/tests/shell/kvm.sh
> > > > > @@ -0,0 +1,154 @@
> > > > > +#!/bin/bash
> > > > > +# perf kvm tests
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +
> > > > > +set -e
> > > > > +
> > > > > +err=0
> > > > > +perfdata=$(mktemp /tmp/__perf_kvm_test.perf.data.XXXXX)
> > > > > +qemu_pid_file=$(mktemp /tmp/__perf_kvm_test.qemu.pid.XXXXX)
> > > > > +
> > > > > +cleanup() {
> > > > > +     rm -f "${perfdata}"
> > > > > +     if [ -f "${qemu_pid_file}" ]; then
> > > > > +             if [ -s "${qemu_pid_file}" ]; then
> > > > > +                     qemu_pid=$(cat "${qemu_pid_file}")
> > > > > +                     if [ -n "${qemu_pid}" ]; then
> > > > > +                             kill "${qemu_pid}" 2>/dev/null || true
> > > > > +                     fi
> > > > > +             fi
> > > > > +             rm -f "${qemu_pid_file}"
> > > > > +     fi
> > > > > +     trap - EXIT TERM INT
> > > > > +}
> > > > > +
> > > > > +trap_cleanup() {
> > > > > +     echo "Unexpected signal in ${FUNCNAME[1]}"
> > > > > +     cleanup
> > > > > +     exit 1
> > > > > +}
> > > > > +trap trap_cleanup EXIT TERM INT
> > > > > +
> > > > > +skip() {
> > > > > +     echo "Skip: $1"
> > > > > +     cleanup
> > > > > +     exit 2
> > > > > +}
> > > > > +
> > > > > +test_kvm_stat() {
> > > > > +     echo "Testing perf kvm stat"
> > > > > +
> > > > > +     echo "Recording kvm events for pid ${qemu_pid}..."
> > > > > +     if ! perf kvm stat record -p "${qemu_pid}" -o "${perfdata}" sleep 1; then
> > > > > +             echo "Failed to record kvm events"
> > > > > +             err=1
> > > > > +             return
> > > > > +     fi
> > > > > +
> > > > > +     echo "Reporting kvm events..."
> > > > > +     if ! perf kvm -i "${perfdata}" stat report 2>&1 | grep -q "VM-EXIT"; then
> > > > > +             echo "Failed to find VM-EXIT in report"
> > > > > +             perf kvm -i "${perfdata}" stat report 2>&1
> > > > > +             err=1
> > > > > +             return
> > > > > +     fi
> > > > > +
> > > > > +     echo "perf kvm stat test [Success]"
> > > > > +}
> > > > > +
> > > > > +test_kvm_record_report() {
> > > > > +     echo "Testing perf kvm record/report"
> > > > > +
> > > > > +     echo "Recording kvm profile for pid ${qemu_pid}..."
> > > > > +     # Use --host to avoid needing guest symbols/mounts for this simple test
> > > > > +     # We just want to verify the command runs and produces data
> > > > > +     # We run in background and kill it because 'perf kvm record' appends options
> > > > > +     # after the command, which breaks 'sleep' (e.g. it gets '-e cycles').
> > > > > +     perf kvm --host record -p "${qemu_pid}" -o "${perfdata}" &
> > > > > +     rec_pid=$!
> > > > > +     sleep 1
> > > > > +     kill -INT "${rec_pid}"
> > > > > +     wait "${rec_pid}" || true
> > > > > +
> > > > > +     echo "Reporting kvm profile..."
> > > > > +     # Check for some standard output from report
> > > > > +     if ! perf kvm -i "${perfdata}" report --stdio 2>&1 | grep -q "Event count"; then
> > > > > +             echo "Failed to report kvm profile"
> > > > > +             perf kvm -i "${perfdata}" report --stdio 2>&1
> > > > > +             err=1
> > > > > +             return
> > > > > +     fi
> > > > > +
> > > > > +     echo "perf kvm record/report test [Success]"
> > > > > +}
> > > > > +
> > > > > +test_kvm_buildid_list() {
> > > > > +     echo "Testing perf kvm buildid-list"
> > > > > +
> > > > > +     # We reuse the perf.data from the previous record test
> > > > > +     if ! perf kvm --host -i "${perfdata}" buildid-list 2>&1 | grep -q "."; then
> > > > > +             echo "Failed to list buildids"
> > > > > +             perf kvm --host -i "${perfdata}" buildid-list 2>&1
> > > > > +             err=1
> > > > > +             return
> > > > > +     fi
> > > > > +
> > > > > +     echo "perf kvm buildid-list test [Success]"
> > > > > +}
> > > > > +
> > > > > +setup_qemu() {
> > > > > +     # Find qemu
> > > > > +     if [ "$(uname -m)" = "x86_64" ]; then
> > > > > +             qemu="qemu-system-x86_64"
> > > > > +     elif [ "$(uname -m)" = "aarch64" ]; then
> > > > > +             qemu="qemu-system-aarch64"
> > > > > +     elif [ "$(uname -m)" = "s390x" ]; then
> > > > > +             qemu="qemu-system-s390x"
> > > > > +     elif [ "$(uname -m)" = "ppc64le" ]; then
> > > > > +             qemu="qemu-system-ppc64"
> > > > > +     else
> > > > > +             qemu="qemu-system-$(uname -m)"
> > > > > +     fi
> > > > > +
> > > > > +     if ! which -s "$qemu"; then
> > > > > +             skip "$qemu not found"
> > > > > +     fi
> > > > > +
> > > > > +     if [ ! -r /dev/kvm ] || [ ! -w /dev/kvm ]; then
> > > > > +             skip "/dev/kvm not accessible"
> > > > > +     fi
> > > > > +
> > > > > +     if ! perf kvm stat record -a sleep 0.01 >/dev/null 2>&1; then
> > > > > +             skip "No permission to record kvm events"
> > > > > +     fi
> > > > > +
> > > > > +     echo "Starting $qemu..."
> > > > > +     # Start qemu in background, detached, with pidfile
> > > > > +     # We use -display none -daemonize and a monitor to keep it alive/controllable if needed
> > > > > +     # We don't need a real kernel, just KVM active.
> > > > > +     if ! $qemu -enable-kvm -display none -daemonize -pidfile "${qemu_pid_file}" -monitor none; then
> > > >
> > > > What is running in the guest?
> > >
> > > Nothing. As no kernel or storage image were provided the guest should
> > > be waiting at the bios similar to if you booted a PC with no storage
> > > in it. I just wanted something quick and dirty to give the basic perf
> > > kvm command a simple work out for the sake of testing coverage.
> >
> > Ok, it'd be fine as long as perf can get some samples in the guest.
> 
> I hope we always get a sample in the guest, but I don't think it is a
> given as qemu is running in the background, etc. Even without guest
> samples the code is getting coverage in the perf tool we don't get by
> other means, for example, finding the broken BUG_ON.

Fair enough.  We can update it later if needed.

Thanks,
Namhyung

Re: [PATCH v1 00/17] perf: Small fixes and new tests

[Namhyung Kim]

On Sat, 22 Nov 2025 00:19:12 -0800, Ian Rogers wrote:
> Pushing for additional test coverage exposed minor fixes, a missing
> timechart option, and a number of reference counting issues. Fix the
> issues then add the new tests. The new tests cover c2c, additional
> build-id tests, perf top, timechart, kallsyms, perf script with
>  --dlfilter, evlist and kvm.
> 
> Ian Rogers (17):
>   perf kvm: Fix debug assertion
>   perf timechart: Add record support for output perf.data path
>   perf symbol-elf: Add missing puts on error path
>   perf symbol: Add missed dso__put
>   perf probe-event: Ensure probe event nsinfo is always cleared
>   perf hist: In init, ensure mem_info is put on error paths
>   perf mem-events: Don't leak online CPU map
>   perf jitdump: Fix missed dso__put
>   perf c2c: Clean up some defensive gets and make asan clean
>   perf tests c2c: Add a basic c2c
>   perf tests buildid: Add purge and remove testing
>   perf tests top: Add basic perf top coverage test
>   perf tests timechart: Add a perf timechart test
>   perf tests kallsyms: Add basic kallsyms test
>   perf tests script dlfilter: Add a dlfilter test
>   perf tests evlist: Add basic evlist test
>   perf test kvm: Add some basic perf kvm test coverage
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung