[PATCH] perf test: Add option to change objdump binary

James Clark posted 1 patch 2 years, 1 month ago
There is a newer version of this series
tools/perf/tests/builtin-test.c | 3 +++
tools/perf/tests/code-reading.c | 2 +-
tools/perf/tests/tests.h        | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)
[PATCH] perf test: Add option to change objdump binary
Posted by James Clark 2 years, 1 month ago
All of the other Perf subcommands that use objdump have an option to
specify the binary, so add the same option to perf test.

This is useful if you have built the kernel with a different toolchain
to the system one, where the system objdump may fail to disassemble
vmlinux.

Now this can be fixed with something like this:

  $ perf test --objdump llvm-objdump "object code reading"

Signed-off-by: James Clark <james.clark@arm.com>
---
 tools/perf/tests/builtin-test.c | 3 +++
 tools/perf/tests/code-reading.c | 2 +-
 tools/perf/tests/tests.h        | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index cb6f1dd00dc4..a8d17dd50588 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -32,6 +32,7 @@
 
 static bool dont_fork;
 const char *dso_to_test;
+const char *test_objdump_path = "objdump";
 
 /*
  * List of architecture specific tests. Not a weak symbol as the array length is
@@ -529,6 +530,8 @@ int cmd_test(int argc, const char **argv)
 		    "Do not fork for testcase"),
 	OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
 	OPT_STRING(0, "dso", &dso_to_test, "dso", "dso to test"),
+	OPT_STRING(0, "objdump", &test_objdump_path, "path",
+		   "objdump binary to use for disassembly and annotations"),
 	OPT_END()
 	};
 	const char * const test_subcommands[] = { "list", NULL };
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 3af81012014e..b353358acc3a 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -185,7 +185,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 	int ret;
 
 	fmt = "%s -z -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
-	ret = snprintf(cmd, sizeof(cmd), fmt, "objdump", addr, addr + len,
+	ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, addr + len,
 		       filename);
 	if (ret <= 0 || (size_t)ret >= sizeof(cmd))
 		return -1;
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index b394f3ac2d66..dad3d7414142 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -207,5 +207,6 @@ DECLARE_WORKLOAD(brstack);
 DECLARE_WORKLOAD(datasym);
 
 extern const char *dso_to_test;
+extern const char *test_objdump_path;
 
 #endif /* TESTS_H */
-- 
2.34.1
Re: [PATCH] perf test: Add option to change objdump binary
Posted by Ian Rogers 2 years, 1 month ago
On Fri, Nov 3, 2023 at 4:35 AM James Clark <james.clark@arm.com> wrote:
>
> All of the other Perf subcommands that use objdump have an option to
> specify the binary, so add the same option to perf test.
>
> This is useful if you have built the kernel with a different toolchain
> to the system one, where the system objdump may fail to disassemble
> vmlinux.
>
> Now this can be fixed with something like this:
>
>   $ perf test --objdump llvm-objdump "object code reading"
>
> Signed-off-by: James Clark <james.clark@arm.com>

There is also "perf config" for things like this.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian
Re: [PATCH] perf test: Add option to change objdump binary
Posted by James Clark 2 years, 1 month ago

On 03/11/2023 15:41, Ian Rogers wrote:
> On Fri, Nov 3, 2023 at 4:35 AM James Clark <james.clark@arm.com> wrote:
>>
>> All of the other Perf subcommands that use objdump have an option to
>> specify the binary, so add the same option to perf test.
>>
>> This is useful if you have built the kernel with a different toolchain
>> to the system one, where the system objdump may fail to disassemble
>> vmlinux.
>>
>> Now this can be fixed with something like this:
>>
>>    $ perf test --objdump llvm-objdump "object code reading"
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
> 
> There is also "perf config" for things like this.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> Thanks,
> Ian

That seems a bit better for this use case so I added it in V2.

Thanks for the review.

James
Re: [PATCH] perf test: Add option to change objdump binary
Posted by Arnaldo Carvalho de Melo 2 years, 1 month ago
Em Tue, Nov 07, 2023 at 09:37:32AM +0000, James Clark escreveu:
> 
> 
> On 03/11/2023 15:41, Ian Rogers wrote:
> > On Fri, Nov 3, 2023 at 4:35 AM James Clark <james.clark@arm.com> wrote:
> > > 
> > > All of the other Perf subcommands that use objdump have an option to
> > > specify the binary, so add the same option to perf test.
> > > 
> > > This is useful if you have built the kernel with a different toolchain
> > > to the system one, where the system objdump may fail to disassemble
> > > vmlinux.
> > > 
> > > Now this can be fixed with something like this:
> > > 
> > >    $ perf test --objdump llvm-objdump "object code reading"
> > > 
> > > Signed-off-by: James Clark <james.clark@arm.com>
> > 
> > There is also "perf config" for things like this.
> > 
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > 
> > Thanks,
> > Ian
> 
> That seems a bit better for this use case so I added it in V2.

Have you posted it?
 
> Thanks for the review.
> 
> James

-- 

- Arnaldo
Re: [PATCH] perf test: Add option to change objdump binary
Posted by Arnaldo Carvalho de Melo 2 years, 1 month ago
Em Wed, Nov 08, 2023 at 05:48:52PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 07, 2023 at 09:37:32AM +0000, James Clark escreveu:
> > On 03/11/2023 15:41, Ian Rogers wrote:
> > > There is also "perf config" for things like this.

> > > Reviewed-by: Ian Rogers <irogers@google.com>

> > That seems a bit better for this use case so I added it in V2.
 
> Have you posted it?

Yeah, b4 found it, nevermind.

- Arnaldo