* [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
* [PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings
2026-09-12 3:11 [PATCH] perf thread_map: Deduplicate numerically equivalent PID and TID strings Hui Su
@ 2026-09-12 5:17 ` Hui Su
2026-09-14 1:19 ` Namhyung Kim
0 siblings, 1 reply; 3+ messages in thread
From: Hui Su @ 2026-09-12 5:17 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, 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] perf thread_map: Deduplicate numerically equivalent PID and TID strings
2026-09-12 5:17 ` [PATCH v2] " Hui Su
@ 2026-09-14 1:19 ` Namhyung Kim
0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2026-09-14 1:19 UTC (permalink / raw)
To: Hui Su
Cc: Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, James Clark,
Jiri Olsa, linux-perf-users, linux-kernel
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
>
^ 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®