Hi Adrian,
On Wed, Sep 04, 2019 at 10:26:13AM +0300, Adrian Hunter wrote:
[...]
Could you take chance to review my below replying? I'd like to get your confirmation before I send out offical patch.
It is not necessary to do kallsyms__parse for x86_64, so it would be better to check the arch before calling that.
Thanks for suggestion, will do it in the formal patch.
However in general, having to copy and use kallsyms with perf.data if on a different arch does not seem very user friendly.
Agree. Seems it's more reasonable to save related info in perf.data; TBH, I have no idea for how to do that.
But really that is up to Arnaldo.
@Arnaldo, if possible could you take a look for below change?
If you don't think below code is the right thing and it's not a common issue, then maybe it's more feasible to resolve this issue only for Arm CoreSight specific.
Please let me know how about you think for this?
Thanks, Leo Yan
For your question for taking a perf.data file to a machine with a different architecture, we can firstly use command 'perf buildid-list' to print out the buildid for kallsyms, based on the dumped buildid we can find out the location for the saved kallsyms file; then we can use option '--kallsyms' to specify the offline kallsyms file and use the offline kallsyms to fixup kernel start address. The detailed commands are listed as below:
root@debian:~# perf buildid-list 7b36dfca8317ef74974ebd7ee5ec0a8b35c97640 [kernel.kallsyms] 56b84aa88a1bcfe222a97a53698b92723a3977ca /usr/lib/systemd/systemd 0956b952e9cd673d48ff2cfeb1a9dbd0c853e686 /usr/lib/aarch64-linux-gnu/libm-2.28.so [...]
root@debian:~# perf script --kallsyms ~/.debug/[kernel.kallsyms]/7b36dfca8317ef74974ebd7ee5ec0a8b35c97640/kallsyms
The amended patch is as below, please review and always welcome any suggestions or comments!
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 5734460fc89e..593f05cc453f 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -2672,9 +2672,26 @@ int machine__nr_cpus_avail(struct machine *machine) return machine ? perf_env__nr_cpus_avail(machine->env) : 0; } +static int machine__fixup_kernel_start(void *arg,
const char *name __maybe_unused,char type,u64 start)+{
- struct machine *machine = arg;
- type = toupper(type);
- /* Fixup for text, weak, data and bss sections. */
- if (type == 'T' || type == 'W' || type == 'D' || type == 'B')
machine->kernel_start = min(machine->kernel_start, start);- return 0;
+}
int machine__get_kernel_start(struct machine *machine) { struct map *map = machine__kernel_map(machine);
- char filename[PATH_MAX]; int err = 0;
/* @@ -2696,6 +2713,22 @@ int machine__get_kernel_start(struct machine *machine) if (!err && !machine__is(machine, "x86_64")) machine->kernel_start = map->start; }
- if (symbol_conf.kallsyms_name != NULL) {
strncpy(filename, symbol_conf.kallsyms_name, PATH_MAX);- } else {
machine__get_kallsyms_filename(machine, filename, PATH_MAX);if (symbol__restricted_filename(filename, "/proc/kallsyms"))goto out;- }
- if (kallsyms__parse(filename, machine, machine__fixup_kernel_start))
pr_warning("Fail to fixup kernel start address. skipping...\n");+out: return err; }
Thanks, Leo Yan