[PATCH] perf arm-spe: Use old behavior when opening old SPE files

James Clark posted 1 patch 3 weeks, 5 days ago
tools/perf/util/arm-spe.c | 50 +++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 26 deletions(-)
[PATCH] perf arm-spe: Use old behavior when opening old SPE files
Posted by James Clark 3 weeks, 5 days ago
Since the linked commit, we stopped interpreting data source if the
perf.data file doesn't have the new metadata version. This means that
perf c2c will show no samples in this case.

Keep the old behavior so old files can be opened, but also still show
the new warning that updating might improve the decoding.

Also re-write the warning to be more concise and specific to a user.

Fixes: ba5e7169e548 ("perf arm-spe: Use metadata to decide the data source feature")
Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/util/arm-spe.c | 50 +++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/arm-spe.c b/tools/perf/util/arm-spe.c
index 856cc36f33d7..fb5ab8607b64 100644
--- a/tools/perf/util/arm-spe.c
+++ b/tools/perf/util/arm-spe.c
@@ -546,37 +546,35 @@ static bool arm_spe__is_common_ds_encoding(struct arm_spe_queue *speq)
 	u64 *metadata = NULL;
 	u64 midr = 0;
 
-	/*
-	 * Metadata version 1 doesn't contain any info for MIDR.
-	 * Simply return false in this case.
-	 */
+	/* Metadata version 1 assumes all CPUs are the same (old behavior) */
 	if (spe->metadata_ver == 1) {
-		pr_warning_once("The data file contains metadata version 1, "
-				"which is absent the info for data source. "
-				"Please upgrade the tool to record data.\n");
-		return false;
-	}
-
-	/* CPU ID is -1 for per-thread mode */
-	if (speq->cpu < 0) {
-		/*
-		 * On the heterogeneous system, due to CPU ID is -1,
-		 * cannot confirm the data source packet is supported.
-		 */
-		if (!spe->is_homogeneous)
-			return false;
+		const char *cpuid;
 
-		/* In homogeneous system, simply use CPU0's metadata */
-		if (spe->metadata)
-			metadata = spe->metadata[0];
+		pr_warning_once("Old SPE metadata, re-record to improve decode accuracy\n");
+		cpuid = perf_env__cpuid(spe->session->evlist->env);
+		midr = strtol(cpuid, NULL, 16);
 	} else {
-		metadata = arm_spe__get_metadata_by_cpu(spe, speq->cpu);
-	}
+		/* CPU ID is -1 for per-thread mode */
+		if (speq->cpu < 0) {
+			/*
+			 * On the heterogeneous system, due to CPU ID is -1,
+			 * cannot confirm the data source packet is supported.
+			 */
+			if (!spe->is_homogeneous)
+				return false;
+
+			/* In homogeneous system, simply use CPU0's metadata */
+			if (spe->metadata)
+				metadata = spe->metadata[0];
+		} else {
+			metadata = arm_spe__get_metadata_by_cpu(spe, speq->cpu);
+		}
 
-	if (!metadata)
-		return false;
+		if (!metadata)
+			return false;
 
-	midr = metadata[ARM_SPE_CPU_MIDR];
+		midr = metadata[ARM_SPE_CPU_MIDR];
+	}
 
 	is_in_cpu_list = is_midr_in_range_list(midr, common_ds_encoding_cpus);
 	if (is_in_cpu_list)
-- 
2.34.1
Re: [PATCH] perf arm-spe: Use old behavior when opening old SPE files
Posted by Namhyung Kim 3 weeks, 3 days ago
On Tue, 29 Oct 2024 14:37:33 +0000, James Clark wrote:

> Since the linked commit, we stopped interpreting data source if the
> perf.data file doesn't have the new metadata version. This means that
> perf c2c will show no samples in this case.
> 
> Keep the old behavior so old files can be opened, but also still show
> the new warning that updating might improve the decoding.
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung
Re: [PATCH] perf arm-spe: Use old behavior when opening old SPE files
Posted by Leo Yan 3 weeks, 5 days ago
On Tue, Oct 29, 2024 at 02:37:33PM +0000, James Clark wrote:
> 
> Since the linked commit, we stopped interpreting data source if the
> perf.data file doesn't have the new metadata version. This means that
> perf c2c will show no samples in this case.
> 
> Keep the old behavior so old files can be opened, but also still show
> the new warning that updating might improve the decoding.
> 
> Also re-write the warning to be more concise and specific to a user.
> 
> Fixes: ba5e7169e548 ("perf arm-spe: Use metadata to decide the data source feature")
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Leo Yan <leo.yan@arm.com>