All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf cpumap: Fix die and cluster IDs
@ 2024-12-18 11:55 James Clark
  2024-12-19  5:14 ` Namhyung Kim
  2025-01-28 19:15 ` Namhyung Kim
  0 siblings, 2 replies; 3+ messages in thread
From: James Clark @ 2024-12-18 11:55 UTC (permalink / raw)
  To: linux-perf-users, namhyung, acme, irogers
  Cc: James Clark, Peter Zijlstra, Ingo Molnar, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Adrian Hunter, Liang, Kan,
	Yicong Yang, linux-kernel

Now that filename__read_int() returns -errno instead of -1 these
statements need to be updated otherwise error values will be used as
die IDs.

This appears as a -2 die ID when the platform doesn't export one:

  $ perf stat --per-core -a -- true

  S36-D-2-C0            1               9.45 msec cpu-clock

And the session topology test fails:

  $ perf test -vvv topology

  CPU 0, core 0, socket 36
  CPU 1, core 1, socket 36
  CPU 2, core 2, socket 36
  CPU 3, core 3, socket 36
  FAILED tests/topology.c:137 Cpu map - Die ID doesn't match
  ---- end(-1) ----
  38: Session topology                                                : FAILED!

Fixes: 05be17eed774 ("tool api fs: Correctly encode errno for read/write open failures")
Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/util/cpumap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 27094211edd8..5c329ad614e9 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -293,7 +293,7 @@ struct aggr_cpu_id aggr_cpu_id__die(struct perf_cpu cpu, void *data)
 
 	die = cpu__get_die_id(cpu);
 	/* There is no die_id on legacy system. */
-	if (die == -1)
+	if (die < 0)
 		die = 0;
 
 	/*
@@ -322,7 +322,7 @@ struct aggr_cpu_id aggr_cpu_id__cluster(struct perf_cpu cpu, void *data)
 	struct aggr_cpu_id id;
 
 	/* There is no cluster_id on legacy system. */
-	if (cluster == -1)
+	if (cluster < 0)
 		cluster = 0;
 
 	id = aggr_cpu_id__die(cpu, data);
-- 
2.34.1


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

* Re: [PATCH] perf cpumap: Fix die and cluster IDs
  2024-12-18 11:55 [PATCH] perf cpumap: Fix die and cluster IDs James Clark
@ 2024-12-19  5:14 ` Namhyung Kim
  2025-01-28 19:15 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2024-12-19  5:14 UTC (permalink / raw)
  To: James Clark
  Cc: linux-perf-users, acme, irogers, Peter Zijlstra, Ingo Molnar,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	Liang, Kan, Yicong Yang, linux-kernel

Hello,

On Wed, Dec 18, 2024 at 11:55:51AM +0000, James Clark wrote:
> Now that filename__read_int() returns -errno instead of -1 these
> statements need to be updated otherwise error values will be used as
> die IDs.
> 
> This appears as a -2 die ID when the platform doesn't export one:
> 
>   $ perf stat --per-core -a -- true
> 
>   S36-D-2-C0            1               9.45 msec cpu-clock
> 
> And the session topology test fails:
> 
>   $ perf test -vvv topology
> 
>   CPU 0, core 0, socket 36
>   CPU 1, core 1, socket 36
>   CPU 2, core 2, socket 36
>   CPU 3, core 3, socket 36
>   FAILED tests/topology.c:137 Cpu map - Die ID doesn't match
>   ---- end(-1) ----
>   38: Session topology                                                : FAILED!
> 
> Fixes: 05be17eed774 ("tool api fs: Correctly encode errno for read/write open failures")
> Signed-off-by: James Clark <james.clark@linaro.org>

I think I reported the same issue earlier, anyway.

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

Thanks,
Namhyung

> ---
>  tools/perf/util/cpumap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 27094211edd8..5c329ad614e9 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -293,7 +293,7 @@ struct aggr_cpu_id aggr_cpu_id__die(struct perf_cpu cpu, void *data)
>  
>  	die = cpu__get_die_id(cpu);
>  	/* There is no die_id on legacy system. */
> -	if (die == -1)
> +	if (die < 0)
>  		die = 0;
>  
>  	/*
> @@ -322,7 +322,7 @@ struct aggr_cpu_id aggr_cpu_id__cluster(struct perf_cpu cpu, void *data)
>  	struct aggr_cpu_id id;
>  
>  	/* There is no cluster_id on legacy system. */
> -	if (cluster == -1)
> +	if (cluster < 0)
>  		cluster = 0;
>  
>  	id = aggr_cpu_id__die(cpu, data);
> -- 
> 2.34.1
> 

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

* Re: [PATCH] perf cpumap: Fix die and cluster IDs
  2024-12-18 11:55 [PATCH] perf cpumap: Fix die and cluster IDs James Clark
  2024-12-19  5:14 ` Namhyung Kim
@ 2025-01-28 19:15 ` Namhyung Kim
  1 sibling, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2025-01-28 19:15 UTC (permalink / raw)
  To: linux-perf-users, acme, irogers, James Clark
  Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Adrian Hunter, Liang, Kan, Yicong Yang, linux-kernel

On Wed, 18 Dec 2024 11:55:51 +0000, James Clark wrote:
> Now that filename__read_int() returns -errno instead of -1 these
> statements need to be updated otherwise error values will be used as
> die IDs.
> 
> This appears as a -2 die ID when the platform doesn't export one:
> 
>   $ perf stat --per-core -a -- true
> 
> [...]
Applied to perf-tools, thanks!

Best regards,
Namhyung



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

end of thread, other threads:[~2025-01-28 19:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-18 11:55 [PATCH] perf cpumap: Fix die and cluster IDs James Clark
2024-12-19  5:14 ` Namhyung Kim
2025-01-28 19:15 ` Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.