mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] perf thread_map: Deduplicate numerically equivalent PID and TID strings
@ 2026-09-12  3:11 Hui Su
  2026-09-12  5:17 ` [PATCH v2] " Hui Su
  0 siblings, 1 reply; 3+ messages in thread
From: Hui Su @ 2026-09-12  3:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Ian Rogers, Adrian Hunter, James Clark, Jiri Olsa,
	linux-perf-users, linux-kernel, Hui Su

thread_map__new_by_pid_str() and thread_map__new_by_tid_str() attempt
to deduplicate adjacent PIDs/TIDs using prev_pid and prev_tid.
However, prev_pid and prev_tid were never updated inside the loop,
making the check "if (pid == prev_pid)" dead code.

Furthermore, even if prev_pid/prev_tid were updated, strlist sorts
lexicographically (e.g. "010", "011", "10"), which means identical
numeric values with different string representations (e.g. leading
zeros) are not adjacent in the list and would never be deduplicated.

Before this fix, running duplicate TID and PID tests fails:

  $ perf test -v 35
  35: Thread map                                                      :
  --- start ---
  test child forked, pid 363297
  FAILED tests/thread-map.c:63 wrong nr for duplicate TIDs
    (nr=3, expected 1)
  test child finished with -1
  ---- end ----
  Thread map: FAILED!

Replace the ineffective prev_pid/prev_tid check with an intlist seen-set.
This ensures that any numeric duplicate PID or TID is properly
recognized and skipped, regardless of string formatting or order.

Add tests for numerically equivalent PID and TID strings.

After this fix:

  $ perf test -v 35
  35: Thread map                                                      :
  --- start ---
  test child forked, pid 363650
  test child finished with 0
  ---- end ----
  Thread map: Ok

Fixes: b52956c961be ("perf tools: Allow multiple threads or processes in record, stat, top")
Signed-off-by: Hui Su <sh_def@163.com>
---
 tools/perf/tests/thread-map.c | 33 +++++++++++++++++++++++++++++++++
 tools/perf/util/thread_map.c  | 28 ++++++++++++++++++++--------
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
index 877868107455..0299c8fd0a6a 100644
--- a/tools/perf/tests/thread-map.c
+++ b/tools/perf/tests/thread-map.c
@@ -56,6 +56,39 @@ static int test__thread_map(struct test_suite *test __maybe_unused, int subtest
 	TEST_ASSERT_VAL("wrong refcnt",
 			refcount_read(&map->refcnt) == 1);
 	perf_thread_map__put(map);
+
+	/* test numeric deduplication of TIDs */
+	map = thread_map__new_by_tid_str("123,0123,00123");
+	TEST_ASSERT_VAL("failed to alloc map", map);
+	TEST_ASSERT_VAL("wrong nr for duplicate TIDs", map->nr == 1);
+	TEST_ASSERT_VAL("wrong pid", perf_thread_map__pid(map, 0) == 123);
+	perf_thread_map__put(map);
+
+	/* test non-adjacent numeric duplicates (strlist lexicographic: 010, 011, 10) */
+	map = thread_map__new_by_tid_str("010,011,10");
+	TEST_ASSERT_VAL("failed to alloc map", map);
+	TEST_ASSERT_VAL("wrong nr for non-adjacent duplicate TIDs", map->nr == 2);
+	perf_thread_map__put(map);
+
+	/* test numeric deduplication of PIDs */
+	{
+		struct perf_thread_map *base, *dup;
+		char pid_str[64];
+
+		base = thread_map__new_by_pid(getpid());
+		TEST_ASSERT_VAL("failed to alloc baseline map", base);
+
+		snprintf(pid_str, sizeof(pid_str), "%d,0%d", getpid(), getpid());
+
+		dup = thread_map__new_str(pid_str, NULL, false);
+		TEST_ASSERT_VAL("failed to alloc duplicate pid map", dup);
+		TEST_ASSERT_VAL("wrong nr for duplicate PIDs",
+				dup->nr == base->nr);
+
+		perf_thread_map__put(dup);
+		perf_thread_map__put(base);
+	}
+
 	return 0;
 }
 
diff --git a/tools/perf/util/thread_map.c b/tools/perf/util/thread_map.c
index 48c70f149e92..7b59b3f7f2e4 100644
--- a/tools/perf/util/thread_map.c
+++ b/tools/perf/util/thread_map.c
@@ -10,6 +10,7 @@
 #include <unistd.h>
 #include "string2.h"
 #include "strlist.h"
+#include "intlist.h"
 #include <string.h>
 #include <api/fs/fs.h>
 #include <linux/string.h>
@@ -163,12 +164,13 @@ static struct perf_thread_map *thread_map__new_by_pid_str(const char *pid_str)
 	int items, total_tasks = 0;
 	struct dirent **namelist = NULL;
 	int i, j = 0;
-	pid_t pid, prev_pid = INT_MAX;
+	pid_t pid;
 	struct str_node *pos;
 	struct strlist *slist = strlist__new(pid_str, NULL);
+	struct intlist *seen = intlist__new(NULL);
 
-	if (!slist)
-		return NULL;
+	if (!slist || !seen)
+		goto out;
 
 	strlist__for_each_entry(pos, slist) {
 		pid = strtol(pos->s, NULL, 10);
@@ -176,9 +178,12 @@ static struct perf_thread_map *thread_map__new_by_pid_str(const char *pid_str)
 		if (pid == INT_MIN || pid == INT_MAX)
 			goto out_free_threads;
 
-		if (pid == prev_pid)
+		if (intlist__has_entry(seen, (unsigned long)pid))
 			continue;
 
+		if (intlist__add(seen, (unsigned long)pid))
+			goto out_free_threads;
+
 		sprintf(name, "/proc/%d/task", pid);
 		items = scandir(name, &namelist, filter, NULL);
 		if (items <= 0)
@@ -200,6 +205,7 @@ static struct perf_thread_map *thread_map__new_by_pid_str(const char *pid_str)
 	}
 
 out:
+	intlist__delete(seen);
 	strlist__delete(slist);
 	if (threads)
 		refcount_set(&threads->refcnt, 1);
@@ -219,17 +225,19 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str)
 {
 	struct perf_thread_map *threads = NULL, *nt;
 	int ntasks = 0;
-	pid_t tid, prev_tid = INT_MAX;
+	pid_t tid;
 	struct str_node *pos;
 	struct strlist *slist;
+	struct intlist *seen;
 
 	/* perf-stat expects threads to be generated even if tid not given */
 	if (!tid_str)
 		return perf_thread_map__new_dummy();
 
 	slist = strlist__new(tid_str, NULL);
-	if (!slist)
-		return NULL;
+	seen = intlist__new(NULL);
+	if (!slist || !seen)
+		goto out;
 
 	strlist__for_each_entry(pos, slist) {
 		tid = strtol(pos->s, NULL, 10);
@@ -237,9 +245,12 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str)
 		if (tid == INT_MIN || tid == INT_MAX)
 			goto out_free_threads;
 
-		if (tid == prev_tid)
+		if (intlist__has_entry(seen, (unsigned long)tid))
 			continue;
 
+		if (intlist__add(seen, (unsigned long)tid))
+			goto out_free_threads;
+
 		ntasks++;
 		nt = perf_thread_map__realloc(threads, ntasks);
 
@@ -251,6 +262,7 @@ struct perf_thread_map *thread_map__new_by_tid_str(const char *tid_str)
 		threads->nr = ntasks;
 	}
 out:
+	intlist__delete(seen);
 	strlist__delete(slist);
 	if (threads)
 		refcount_set(&threads->refcnt, 1);
-- 
2.55.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-14  1:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-12  3:11 [PATCH] perf thread_map: Deduplicate numerically equivalent PID and TID strings Hui Su
2026-09-12  5:17 ` [PATCH v2] " Hui Su
2026-09-14  1:19   ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®