[PATCH 0/4] perf: Support thread maps larger than one record

[PATCH 0/4] perf: Support thread maps larger than one record

[Leo Yan]

PERF_RECORD_THREAD_MAP has a 16-bit size field and can hold at most
2,729 entries. Larger maps overflow the size field, producing records
that readers reject.

This series splits large maps into records with a continuation flag and
updates perf script and perf stat report to assemble the complete map
before allocating per-thread counters. It also adds boundary tests and
documents the format.

Maps that fit in one record keep their existing encoding. Split maps
require updated readers.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
Leo Yan (4):
      perf synthetic-events: Split oversized thread maps into records
      perf: Assemble split thread maps in script and stat report
      perf test: Cover multi-record thread map synthesis
      perf docs: Describe multi-record thread maps

 tools/lib/perf/include/perf/event.h                |   8 ++
 tools/perf/Documentation/perf.data-file-format.txt |  24 +++++
 tools/perf/builtin-script.c                        |  21 +++-
 tools/perf/builtin-stat.c                          |  25 +++--
 tools/perf/tests/thread-map.c                      | 111 +++++++++++++++++++++
 tools/perf/util/synthetic-events.c                 |  46 +++++++--
 tools/perf/util/thread_map.c                       |  51 ++++++++--
 tools/perf/util/thread_map.h                       |   2 +
 8 files changed, 256 insertions(+), 32 deletions(-)
---
base-commit: e8b3879165c74999b15e918b14ece7564cb09c59
change-id: 20261019-perf_fix_large_threads-f81558a92988

Best regards,
-- 
Leo Yan <leo.yan@arm.com>

[PATCH 1/4] perf synthetic-events: Split oversized thread maps into records

[Leo Yan]

PERF_RECORD_THREAD_MAP has a 16-bit header.size field. With 16 bytes of
fixed fields and 24 bytes per entry, a record can hold at most 2,729
threads within the 65,535-byte size limit.

For larger maps, header.size overflows and no longer describes the
entries counted by nr, so readers reject the record.

Split larger maps into records that fit the size field. Set
PERF_RECORD_MISC_THREAD_MAP_MORE in header.misc when another record
follows, and clear it in the final record. Preserve entry order and the
existing encoding for maps that fit in one record, including empty maps.

Stop synthesis if allocation or the event callback fails.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/lib/perf/include/perf/event.h |  8 +++++++
 tools/perf/util/synthetic-events.c  | 46 +++++++++++++++++++++++++++++--------
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
index fdced574c889e503bda3cf2f8457a14026140025..1785fea2873e4c4dca369b9257626302cece11c0 100644
--- a/tools/lib/perf/include/perf/event.h
+++ b/tools/lib/perf/include/perf/event.h
@@ -406,6 +406,14 @@ struct perf_record_thread_map_entry {
 	char			 comm[16];
 };
 
+/*
+ * PERF_RECORD_MISC_THREAD_MAP_MORE means another THREAD_MAP record follows
+ * for the same map. A clear bit marks the final record.
+ * Each nr counts only that record's entries; concatenate records in order.
+ * A map fitting in one record has the bit clear.
+ */
+#define PERF_RECORD_MISC_THREAD_MAP_MORE (1U << 0)
+
 struct perf_record_thread_map {
 	struct perf_event_header		 header;
 	__u64					 nr;
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 3c8d29f27fd91a2cb5ddcf3a20664024edf697c7..938fb63473c479bda167acfe432e7a2d623c6ef0 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1332,16 +1332,18 @@ int perf_event__synthesize_kernel_mmap(const struct perf_tool *tool,
 	return perf_event__synthesize_extra_kmaps(tool, process, machine);
 }
 
-int perf_event__synthesize_thread_map2(const struct perf_tool *tool,
-				      struct perf_thread_map *threads,
-				      perf_event__handler_t process,
-				      struct machine *machine)
+static int __perf_event__synthesize_thread_map2(const struct perf_tool *tool,
+					       struct perf_thread_map *threads,
+					       perf_event__handler_t process,
+					       struct machine *machine,
+					       int offset, int nr)
 {
 	union perf_event *event;
-	int i, err, size;
+	int i, err;
+	size_t size;
 
 	size  = sizeof(event->thread_map);
-	size +=	threads->nr * sizeof(event->thread_map.entries[0]);
+	size += nr * sizeof(event->thread_map.entries[0]);
 
 	event = zalloc(size);
 	if (!event)
@@ -1349,16 +1351,18 @@ int perf_event__synthesize_thread_map2(const struct perf_tool *tool,
 
 	event->header.type = PERF_RECORD_THREAD_MAP;
 	event->header.size = size;
-	event->thread_map.nr = threads->nr;
+	event->header.misc = offset + nr < threads->nr ?
+			     PERF_RECORD_MISC_THREAD_MAP_MORE : 0;
+	event->thread_map.nr = nr;
 
-	for (i = 0; i < threads->nr; i++) {
+	for (i = 0; i < nr; i++) {
 		struct perf_record_thread_map_entry *entry = &event->thread_map.entries[i];
-		char *comm = perf_thread_map__comm(threads, i);
+		char *comm = perf_thread_map__comm(threads, offset + i);
 
 		if (!comm)
 			comm = (char *) "";
 
-		entry->pid = perf_thread_map__pid(threads, i);
+		entry->pid = perf_thread_map__pid(threads, offset + i);
 		strncpy((char *) &entry->comm, comm, sizeof(entry->comm));
 	}
 
@@ -1368,6 +1372,28 @@ int perf_event__synthesize_thread_map2(const struct perf_tool *tool,
 	return err;
 }
 
+int perf_event__synthesize_thread_map2(const struct perf_tool *tool,
+				      struct perf_thread_map *threads,
+				      perf_event__handler_t process,
+				      struct machine *machine)
+{
+	const int max_nr = (UINT16_MAX - sizeof(struct perf_record_thread_map)) /
+			   sizeof(struct perf_record_thread_map_entry);
+	int offset = 0;
+	int nr, err;
+
+	do {
+		nr = min(threads->nr - offset, max_nr);
+		err = __perf_event__synthesize_thread_map2(tool, threads, process, machine,
+							 offset, nr);
+		if (err)
+			return err;
+		offset += nr;
+	} while (offset < threads->nr);
+
+	return 0;
+}
+
 struct synthesize_cpu_map_data {
 	const struct perf_cpu_map *map;
 	int nr;

-- 
2.34.1

[PATCH 2/4] perf: Assemble split thread maps in script and stat report

[Leo Yan]

perf script and perf stat report accept only the first thread-map record
and ignore subsequent records. For split maps, this leaves per-thread
counters sized for only part of the map.

Add thread_map__append_event() to assemble records in order, and reuse
it for single-record construction. Free any command names copied by a
failed append.

Keep the map private until the final record arrives. Publish it to the
evlist and allocate counters once both the complete thread map and CPU
map are available. This avoids reallocating a map already shared with
the evlist.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/builtin-script.c  | 21 ++++++++++++++----
 tools/perf/builtin-stat.c    | 25 ++++++++++++++++------
 tools/perf/util/thread_map.c | 51 ++++++++++++++++++++++++++++++++++----------
 tools/perf/util/thread_map.h |  2 ++
 4 files changed, 77 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b6691ebb1b4eff14cabe99e4388368503af28a6b..e41925fb86b8d8acff5c85fb4951720fcb5dde0d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -162,6 +162,7 @@ struct perf_script {
 	bool			show_cgroup_events;
 	bool			show_text_poke_events;
 	bool			allocated;
+	bool			threads_is_appending;
 	bool			per_event_dump;
 	bool			stitch_lbr;
 	struct evswitch		evswitch;
@@ -3416,6 +3417,10 @@ static int __cmd_script(struct perf_script *script)
 	}
 
 	ret = perf_session__process_events(script->session);
+	if (!ret && !session_done() && script->threads_is_appending) {
+		pr_err("Incomplete thread map: missing final record.\n");
+		ret = -EINVAL;
+	}
 
 	if (script->per_event_dump)
 		perf_script__exit_per_event_dump_stats(script);
@@ -3975,6 +3980,10 @@ static int set_maps(struct perf_script *script)
 {
 	struct evlist *evlist = script->session->evlist;
 
+	/* Appending can move the map, so share it only after the final record. */
+	if (script->threads_is_appending)
+		return 0;
+
 	if (!script->cpus || !script->threads)
 		return 0;
 
@@ -3996,18 +4005,22 @@ int process_thread_map_event(const struct perf_tool *tool,
 			     union perf_event *event)
 {
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
+	int ret;
 
 	if (dump_trace)
 		perf_event__fprintf_thread_map(event, stdout);
 
-	if (script->threads) {
+	if (script->threads && !script->threads_is_appending) {
 		pr_warning("Extra thread map event, ignoring.\n");
 		return 0;
 	}
 
-	script->threads = thread_map__new_event(&event->thread_map);
-	if (!script->threads)
-		return -ENOMEM;
+	ret = thread_map__append_event(&script->threads, &event->thread_map);
+	if (ret)
+		return ret;
+
+	script->threads_is_appending =
+		event->header.misc & PERF_RECORD_MISC_THREAD_MAP_MORE;
 
 	return set_maps(script);
 }
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 37752d6bfef451e1e8e5ccdddfa741c2c6795882..19d120b64706563251cec000eb7706cd4ad5897d 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -147,6 +147,7 @@ struct perf_stat {
 	u64			 bytes_written;
 	struct perf_tool	 tool;
 	bool			 maps_allocated;
+	bool			 threads_is_appending;
 	struct perf_cpu_map	*cpus;
 	struct perf_thread_map *threads;
 	enum aggr_mode		 aggr_mode;
@@ -2280,6 +2281,10 @@ int process_stat_config_event(const struct perf_tool *tool,
 
 static int set_maps(struct perf_stat *st)
 {
+	/* Appending can move the map, so share it only after the final record. */
+	if (st->threads_is_appending)
+		return 0;
+
 	if (!st->cpus || !st->threads)
 		return 0;
 
@@ -2301,15 +2306,19 @@ int process_thread_map_event(const struct perf_tool *tool,
 			     union perf_event *event)
 {
 	struct perf_stat *st = container_of(tool, struct perf_stat, tool);
+	int ret;
 
-	if (st->threads) {
+	if (st->threads && !st->threads_is_appending) {
 		pr_warning("Extra thread map event, ignoring.\n");
 		return 0;
 	}
 
-	st->threads = thread_map__new_event(&event->thread_map);
-	if (!st->threads)
-		return -ENOMEM;
+	ret = thread_map__append_event(&st->threads, &event->thread_map);
+	if (ret)
+		return ret;
+
+	st->threads_is_appending =
+		event->header.misc & PERF_RECORD_MISC_THREAD_MAP_MORE;
 
 	return set_maps(st);
 }
@@ -2405,11 +2414,13 @@ static int __cmd_report(int argc, const char **argv)
 	evsel_list         = session->evlist;
 
 	ret = perf_session__process_events(session);
-	if (ret)
-		return ret;
+	if (!ret && !session_done() && perf_stat.threads_is_appending) {
+		pr_err("Incomplete thread map: missing final record.\n");
+		ret = -EINVAL;
+	}
 
 	perf_session__delete(session);
-	return 0;
+	return ret;
 }
 
 static void setup_system_wide(int forks)
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 918ab318023205e48ccc18d9a0dd675c1b77f1ed..4b3e0cd022fb79635b914610f32cb0c9c161568f 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -336,28 +336,57 @@ void thread_map__read_comms(struct perf_thread_map *threads)
 		comm_init(threads, i);
 }
 
-static void thread_map__copy_event(struct perf_thread_map *threads,
-				   struct perf_record_thread_map *event)
+int thread_map__append_event(struct perf_thread_map **map,
+			    const struct perf_record_thread_map *event)
 {
-	unsigned i;
+	struct perf_thread_map *threads = *map;
+	bool new_map = !threads;
+	u64 old_nr = threads ? threads->nr : 0, nr, i;
 
-	threads->nr = (int) event->nr;
+	/*
+	 * Trust event->nr to describe chunks of a valid thread map, whose total
+	 * count and allocation size are representable without overflow.
+	 */
+	nr = old_nr + event->nr;
+
+	threads = perf_thread_map__realloc(threads, nr);
+	if (!threads)
+		return -ENOMEM;
+
+	if (new_map) {
+		threads->nr = 0;
+		refcount_set(&threads->refcnt, 1);
+	}
+	*map = threads;
 
 	for (i = 0; i < event->nr; i++) {
-		perf_thread_map__set_pid(threads, i, (pid_t) event->entries[i].pid);
-		threads->map[i].comm = strndup(event->entries[i].comm, 16);
+		u64 idx = old_nr + i;
+
+		perf_thread_map__set_pid(threads, idx,
+					 (pid_t) event->entries[i].pid);
+		threads->map[idx].comm = strndup(event->entries[i].comm,
+						 sizeof(event->entries[i].comm));
+		if (!threads->map[idx].comm)
+			goto out_free;
 	}
+	threads->nr = nr;
 
-	refcount_set(&threads->refcnt, 1);
+	return 0;
+
+out_free:
+	while (i--)
+		zfree(&threads->map[old_nr + i].comm);
+	return -ENOMEM;
 }
 
 struct perf_thread_map *thread_map__new_event(struct perf_record_thread_map *event)
 {
-	struct perf_thread_map *threads;
+	struct perf_thread_map *threads = NULL;
 
-	threads = thread_map__alloc(event->nr);
-	if (threads)
-		thread_map__copy_event(threads, event);
+	if (thread_map__append_event(&threads, event)) {
+		perf_thread_map__put(threads);
+		return NULL;
+	}
 
 	return threads;
 }
diff --git a/tools/perf/util/thread_map.h b/tools/perf/util/thread_map.h
index d700a22b7ae4663aca843baf80f7e5f522d6e607..7ddb9dd3fe9f2525cc7ba358bd6294a293cac910 100644
--- a/tools/perf/util/thread_map.h
+++ b/tools/perf/util/thread_map.h
@@ -13,6 +13,8 @@ struct perf_thread_map *thread_map__new_by_pid(pid_t pid);
 struct perf_thread_map *thread_map__new_by_tid(pid_t tid);
 struct perf_thread_map *thread_map__new(pid_t pid, pid_t tid);
 struct perf_thread_map *thread_map__new_event(struct perf_record_thread_map *event);
+int thread_map__append_event(struct perf_thread_map **map,
+			     const struct perf_record_thread_map *event);
 
 struct perf_thread_map *thread_map__new_str(const char *pid,
 		const char *tid, bool all_threads);

-- 
2.34.1

[PATCH 3/4] perf test: Cover multi-record thread map synthesis

[Leo Yan]

Extend the thread-map synthesis test with empty maps and sizes around
the one-record and two-record limits.

Check record sizes, entry counts, continuation flags and entry order.
Reassemble each map and verify PIDs and command names, including missing
names and full-width names without a terminating NUL.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/tests/thread-map.c | 111 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
index 877868107455e87e526e2b336ca1ca3c7314033e..ced1a357233d68f5b81d6f2136e8cc0d0a95fc15 100644
--- a/tools/perf/tests/thread-map.c
+++ b/tools/perf/tests/thread-map.c
@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <stdlib.h>
 #include <string.h>
+#include <errno.h>
+#include <stdint.h>
+#include <linux/kernel.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <sys/prctl.h>
@@ -8,6 +11,7 @@
 #include "thread_map.h"
 #include "debug.h"
 #include "event.h"
+#include "tool.h"
 #include "util/synthetic-events.h"
 #include <perf/event.h>
 #include <internal/threadmap.h>
@@ -86,9 +90,110 @@ static int process_event(const struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
+struct thread_map_synthesize {
+	struct perf_tool tool;
+	struct perf_thread_map *source;
+	struct perf_thread_map *threads;
+	int offset;
+	int calls;
+};
+
+static int process_thread_map_chunk(const struct perf_tool *tool,
+				    union perf_event *event,
+				    struct perf_sample *sample __maybe_unused,
+				    struct machine *machine __maybe_unused)
+{
+	struct thread_map_synthesize *data = container_of(tool, struct thread_map_synthesize, tool);
+	struct perf_record_thread_map *map = &event->thread_map;
+	const int max_nr = (UINT16_MAX - sizeof(*map)) / sizeof(map->entries[0]);
+	int nr = min(data->source->nr - data->offset, max_nr);
+	bool more = data->offset + nr < data->source->nr;
+	int ret;
+
+	data->calls++;
+	TEST_ASSERT_VAL("wrong chunk type", event->header.type == PERF_RECORD_THREAD_MAP);
+	TEST_ASSERT_VAL("wrong chunk nr", map->nr == (u64)nr);
+	TEST_ASSERT_VAL("wrong chunk size",
+			event->header.size == sizeof(*map) + nr * sizeof(map->entries[0]));
+	TEST_ASSERT_VAL("wrong continuation marker",
+			event->header.misc == (more ? PERF_RECORD_MISC_THREAD_MAP_MORE : 0));
+
+	ret = thread_map__append_event(&data->threads, map);
+	if (ret)
+		return ret;
+	data->offset += nr;
+	return 0;
+}
+
+static int check_thread_map_chunks(struct thread_map_synthesize *data)
+{
+	const int max_nr = (UINT16_MAX - sizeof(struct perf_record_thread_map)) /
+			   sizeof(struct perf_record_thread_map_entry);
+	int ret;
+
+	ret = perf_event__synthesize_thread_map2(&data->tool, data->source,
+					       process_thread_map_chunk, NULL);
+	TEST_ASSERT_VAL("failed to synthesize chunks", !ret);
+	TEST_ASSERT_VAL("wrong chunk count",
+			data->calls == max(1, (data->source->nr + max_nr - 1) / max_nr));
+	TEST_ASSERT_VAL("incomplete thread map", data->offset == data->source->nr);
+	TEST_ASSERT_VAL("failed to assemble thread map", data->threads);
+	TEST_ASSERT_VAL("wrong assembled nr", data->threads->nr == data->source->nr);
+	TEST_ASSERT_VAL("wrong assembled refcnt", refcount_read(&data->threads->refcnt) == 1);
+
+	for (int i = 0; i < data->source->nr; i++) {
+		const char *expected = perf_thread_map__comm(data->source, i);
+		const char *comm = perf_thread_map__comm(data->threads, i);
+
+		if (!expected)
+			expected = "";
+		TEST_ASSERT_VAL("wrong assembled pid",
+				perf_thread_map__pid(data->threads, i) ==
+				perf_thread_map__pid(data->source, i));
+		TEST_ASSERT_VAL("wrong assembled comm", comm && !strncmp(comm, expected, 16));
+		TEST_ASSERT_VAL("assembled comm not terminated",
+				strlen(comm) == strnlen(expected, 16));
+	}
+	return 0;
+}
+
+static int test_thread_map_chunks(int nr)
+{
+	struct thread_map_synthesize data = { 0 };
+	int ret = -ENOMEM;
+
+	data.source = perf_thread_map__new_array(nr, NULL);
+	if (!data.source)
+		return ret;
+
+	for (int i = 0; i < nr; i++) {
+		char comm[32];
+
+		/* Include missing comms and full-width, non-NUL-terminated wire comms. */
+		perf_thread_map__set_pid(data.source, i, nr - i);
+		if (i % 3 == 0)
+			continue;
+		if (i % 3 == 1)
+			snprintf(comm, sizeof(comm), "thread-%d", i);
+		else
+			strcpy(comm, "1234567890123456");
+		data.source->map[i].comm = strdup(comm);
+		if (!data.source->map[i].comm)
+			goto out;
+	}
+
+	ret = check_thread_map_chunks(&data);
+out:
+	perf_thread_map__put(data.threads);
+	perf_thread_map__put(data.source);
+	return ret;
+}
+
 static int test__thread_map_synthesize(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
 {
 	struct perf_thread_map *threads;
+	static const int sizes[] = { 0, 1, 2729, 2730, 3000, 5458, 5459 };
+	int ret;
 
 	TEST_ASSERT_VAL("failed to set process name",
 			!prctl(PR_SET_NAME, NAMEUL, 0, 0, 0));
@@ -103,6 +208,12 @@ static int test__thread_map_synthesize(struct test_suite *test __maybe_unused, i
 		!perf_event__synthesize_thread_map2(NULL, threads, process_event, NULL));
 
 	perf_thread_map__put(threads);
+	for (unsigned int i = 0; i < ARRAY_SIZE(sizes); i++) {
+		ret = test_thread_map_chunks(sizes[i]);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 

-- 
2.34.1

[PATCH 4/4] perf docs: Describe multi-record thread maps

[Leo Yan]

Document the PERF_RECORD_THREAD_MAP layout using the structure names
and flexible array declaration from the code.

Describe the continuation flag and ordered assembly of split maps.
Explain that readers must collect the complete map before allocating
per-thread counters or processing statistics, and that split maps
require updated readers.

Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/Documentation/perf.data-file-format.txt | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
index 1f8e7d8e9ae21d732df698c9d2560377dea5f03f..6b59404ef0ff92102aeb63fc0d1da283536a4add 100644
--- a/tools/perf/Documentation/perf.data-file-format.txt
+++ b/tools/perf/Documentation/perf.data-file-format.txt
@@ -651,6 +651,30 @@ struct auxtrace_error_event {
 	char msg[MAX_AUXTRACE_ERROR_MSG];
 };
 
+	PERF_RECORD_THREAD_MAP			= 73,
+
+Lists thread IDs and command names.
+
+struct perf_record_thread_map_entry {
+	uint64_t pid;
+	char comm[16];
+};
+
+struct perf_record_thread_map {
+	struct perf_event_header header;
+	uint64_t nr;
+	struct perf_record_thread_map_entry entries[];
+};
+
+Large maps are split into records that fit in the 16-bit header.size field.
+PERF_RECORD_MISC_THREAD_MAP_MORE (1 << 0) in header.misc is set when another
+record follows and clear for the final record. Each record's nr counts only
+its own entries. Concatenate entries in record order, preserving their
+positions, until a record with MORE clear completes the map. Consumers must
+assemble the complete map before allocating per-thread counters or processing
+statistics. Split maps require readers that support this convention; maps
+that fit in one record are unchanged.
+
 	PERF_RECORD_HEADER_FEATURE		= 80,
 
 Describes a header feature. These are records used in pipe-mode that

-- 
2.34.1

Re: [PATCH 3/4] perf test: Cover multi-record thread map synthesis

[sashiko-bot]

> Extend the thread-map synthesis test with empty maps and sizes around
> the one-record and two-record limits.
> 
> Check record sizes, entry counts, continuation flags and entry order.
> Reassemble each map and verify PIDs and command names, including missing
> names and full-width names without a terminating NUL.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-perf_fix_large_threads-v1-0-69c9b86b9ae5@arm.com?part=3

Re: [PATCH 4/4] perf docs: Describe multi-record thread maps

[sashiko-bot]

> Document the PERF_RECORD_THREAD_MAP layout using the structure names
> and flexible array declaration from the code.
> 
> Describe the continuation flag and ordered assembly of split maps.
> Explain that readers must collect the complete map before allocating
> per-thread counters or processing statistics, and that split maps
> require updated readers.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-perf_fix_large_threads-v1-0-69c9b86b9ae5@arm.com?part=4

Re: [PATCH 1/4] perf synthetic-events: Split oversized thread maps into records

[sashiko-bot]

> PERF_RECORD_THREAD_MAP has a 16-bit header.size field. With 16 bytes of
> fixed fields and 24 bytes per entry, a record can hold at most 2,729
> threads within the 65,535-byte size limit.
> 
> For larger maps, header.size overflows and no longer describes the
> entries counted by nr, so readers reject the record.
> 
> Split larger maps into records that fit the size field. Set
> PERF_RECORD_MISC_THREAD_MAP_MORE in header.misc when another record
> follows, and clear it in the final record. Preserve entry order and the
> existing encoding for maps that fit in one record, including empty maps.
> 
> Stop synthesis if allocation or the event callback fails.
> 
> Signed-off-by: Leo Yan <leo.yan@arm.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-perf_fix_large_threads-v1-0-69c9b86b9ae5@arm.com?part=1

Re: [PATCH 2/4] perf: Assemble split thread maps in script and stat report

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] perf: NULL dereference on incomplete thread map when
  set_maps() is deferred
- [Low] perf stat: leak of perf_stat.threads and perf_stat.cpus in
  __cmd_report()
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Medium] perf stat: memory leaks of session, evlist, and maps in
  __cmd_report() [1]
--

commit 296ec71892d6fd09202f3090775a27df449bf978
Author: Leo Yan <leo.yan@arm.com>

perf: Assemble split thread maps in script and stat report

This commit adds thread_map__append_event() to assemble split
PERF_RECORD_THREAD_MAP records in perf script and perf stat report before
publishing the completed thread map to the evlist and allocating stats.

> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b6691ebb1b4ef..e41925fb86b8d 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c

[ ... ]

> @@ -3416,6 +3417,10 @@ static int __cmd_script(struct perf_script *script)
>  	}
>  
>  	ret = perf_session__process_events(script->session);
> +	if (!ret && !session_done() && script->threads_is_appending) {
> +		pr_err("Incomplete thread map: missing final record.\n");
> +		ret = -EINVAL;
> +	}
>  
>  	if (script->per_event_dump)
>  		perf_script__exit_per_event_dump_stats(script);
> @@ -3975,6 +3980,10 @@ static int set_maps(struct perf_script *script)
>  {
>  	struct evlist *evlist = script->session->evlist;
>  
> +	/* Appending can move the map, so share it only after the final record. */
> +	if (script->threads_is_appending)
> +		return 0;
> +
>  	if (!script->cpus || !script->threads)
>  		return 0;

[Severity: Medium]
Can deferring set_maps() while threads_is_appending is true cause a NULL
pointer dereference in perf_counts() when the final PERF_RECORD_THREAD_MAP
record is missing?

If a PERF_RECORD_THREAD_MAP record with PERF_RECORD_MISC_THREAD_MAP_MORE set
is followed by PERF_RECORD_CPU_MAP and stat events without the final thread
map record, set_maps() returns early and skips perf_evlist__set_maps() and
evlist__alloc_stats(), leaving counter->counts NULL on each evsel.  Because
threads_is_appending is only checked after perf_session__process_events()
returns, subsequent PERF_RECORD_STAT_ROUND or PERF_RECORD_STAT events are
processed first: process_stat_round_event() -> perf_stat_process_counter() ->
process_counter_maps() calls perf_counts(counter->counts, idx, thread) and
dereferences NULL, or perf_event__process_stat_event() fails earlier with an
"Invalid CPU -1" error that masks the incomplete thread map message.  The
same regression affects set_maps() and __cmd_report() in
tools/perf/builtin-stat.c.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 37752d6bfef45..19d120b647065 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c

[ ... ]

> @@ -2301,15 +2306,19 @@ int process_thread_map_event(const struct perf_tool *tool,
>  			     union perf_event *event)
>  {
>  	struct perf_stat *st = container_of(tool, struct perf_stat, tool);
> +	int ret;
>  
> -	if (st->threads) {
> +	if (st->threads && !st->threads_is_appending) {
>  		pr_warning("Extra thread map event, ignoring.\n");
>  		return 0;
>  	}
>  
> -	st->threads = thread_map__new_event(&event->thread_map);
> -	if (!st->threads)
> -		return -ENOMEM;
> +	ret = thread_map__append_event(&st->threads, &event->thread_map);
> +	if (ret)
> +		return ret;
> +
> +	st->threads_is_appending =
> +		event->header.misc & PERF_RECORD_MISC_THREAD_MAP_MORE;
>  
>  	return set_maps(st);
>  }

[Severity: Low]
Does this leak st->threads when thread_map__append_event() fails partway
through or when the final thread map record is missing?

thread_map__append_event() stores the allocated map in st->threads before
duplicating the comm strings, and set_maps() keeps st->threads private while
st->threads_is_appending is true.  Unlike cmd_script(), which calls
perf_script__exit() to release script->threads and script->cpus,
__cmd_report() only calls perf_session__delete(session) and never calls
perf_thread_map__put(perf_stat.threads).

> @@ -2405,11 +2414,13 @@ static int __cmd_report(int argc, const char **argv)
>  	evsel_list         = session->evlist;
>  
>  	ret = perf_session__process_events(session);
> -	if (ret)
> -		return ret;
> +	if (!ret && !session_done() && perf_stat.threads_is_appending) {
> +		pr_err("Incomplete thread map: missing final record.\n");
> +		ret = -EINVAL;
> +	}
>  
>  	perf_session__delete(session);
> -	return 0;
> +	return ret;
>  }

[Severity: Medium]
This problem wasn't introduced by this patch, but does __cmd_report() leak
perf_stat.threads, perf_stat.cpus, evsel_list, and the aggregation maps
[1]?

cmd_stat() allocates evsel_list with evlist__new() and returns directly from
__cmd_report(), bypassing the cleanup at the end of cmd_stat().  If
perf_session__new() fails in __cmd_report(), returning early before
evlist__put(evsel_list) leaks the initial evsel_list.  During event
processing, set_maps() passes perf_stat.cpus and perf_stat.threads to
perf_evlist__set_maps(), which takes an extra reference on each map, and
process_stat_config_event() allocates aggregation maps;perf_session__delete()
only drops session->evlist's references and __cmd_report() never drops
perf_stat.threads, perf_stat.cpus, or the aggregation maps via
perf_stat__exit_aggr_mode().

[1] https://sashiko.dev/#/bug/linux-b77574e3-cc57-4338-8810-73a0cfa6fff7

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-perf_fix_large_threads-v1-0-69c9b86b9ae5@arm.com?part=2