[PATCH] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[PATCH] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[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

[PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[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, the new tests fail at the first duplicate TID case:

  $ 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>
---
Differences in v2:
 - Explicitly include <stdio.h> in tools/perf/tests/thread-map.c for snprintf.
 - Link to v1: https://lore.kernel.org/r/20260912031112.1814574-2-sh_def@163.com/

 tools/perf/tests/thread-map.c | 34 ++++++++++++++++++++++++++++++++++
 tools/perf/util/thread_map.c  | 28 ++++++++++++++++++++--------
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
index 877868107455..690ccae17c31 100644
--- a/tools/perf/tests/thread-map.c
+++ b/tools/perf/tests/thread-map.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
@@ -56,6 +57,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

Re: [PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[Namhyung Kim]

On Sat, Sep 12, 2026 at 02:17:47PM +0900, Hui Su wrote:
> 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, the new tests fail at the first duplicate TID case:
> 
>   $ 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>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
> Differences in v2:
>  - Explicitly include <stdio.h> in tools/perf/tests/thread-map.c for snprintf.
>  - Link to v1: https://lore.kernel.org/r/20260912031112.1814574-2-sh_def@163.com/
> 
>  tools/perf/tests/thread-map.c | 34 ++++++++++++++++++++++++++++++++++
>  tools/perf/util/thread_map.c  | 28 ++++++++++++++++++++--------
>  2 files changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
> index 877868107455..690ccae17c31 100644
> --- a/tools/perf/tests/thread-map.c
> +++ b/tools/perf/tests/thread-map.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <sys/types.h>
> @@ -56,6 +57,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
> 

Re: [PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[Ian Rogers]

On Fri, Sep 11, 2026 at 10:17 PM Hui Su <sh_def@163.com> wrote:
>
> 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, the new tests fail at the first duplicate TID case:
>
>   $ 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>
> ---
> Differences in v2:
>  - Explicitly include <stdio.h> in tools/perf/tests/thread-map.c for snprintf.
>  - Link to v1: https://lore.kernel.org/r/20260912031112.1814574-2-sh_def@163.com/
>
>  tools/perf/tests/thread-map.c | 34 ++++++++++++++++++++++++++++++++++
>  tools/perf/util/thread_map.c  | 28 ++++++++++++++++++++--------
>  2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/tests/thread-map.c b/tools/perf/tests/thread-map.c
> index 877868107455..690ccae17c31 100644
> --- a/tools/perf/tests/thread-map.c
> +++ b/tools/perf/tests/thread-map.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <sys/types.h>
> @@ -56,6 +57,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);

Why not parse the pid_str into the intlist and then remove duplicates?

Thanks,
Ian

>
> -       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
>

Re: [PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings

[Hui Su]

> Why not parse the pid_str into the intlist and then remove duplicates?

Good point. I initially kept the strlist and used a separate intlist as a
seen-set to minimize the change.

One wrinkle is that intlist__new(pid_str) currently treats duplicate numeric
values as an error: intlist__parse_list() propagates -EEXIST from
intlist__add(). Thus, "123,0123" currently makes intlist__new() fail instead
of deduplicating the values.

I think the cleaner solution is to make intlist parsing fold duplicate values,
for example by using intlist__findnew(), and then iterate the intlist directly
in the PID/TID paths. That removes both the extra strlist and the separate
seen-set.

I'll check whether any existing intlist__new() users rely on duplicates being
rejected, and whether any thread_map callers rely on the current lexicographic
ordering from strlist, since intlist iteration is numeric.

I'll rework it that way for v3.

Thanks,
Hui