Skip to contentSkip to conversation

perf tools: Fix build-id matching on vmlinux

PATCH Superseded Namhyung Kim 4 participants 14 messages Sep 5, 2014 – Sep 19, 2014

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Namhyung Kim

author maintainer
#0 lore raw

PATCH v2 1/1 perf tools: Fix build-id matching on vmlinux

There's a problem on finding correct kernel symbols when perf report runs on a different kernel. Although a part of the problem was solved by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel version when finding vmlinux"), there's a remaining problem still.

When perf records samples, it synthesizes the kernel map using machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text". You can easily see it using 'perf report -D' command.

After finishing record, it goes through the recorded events to find maps/dsos actually used. And then record build-id info of them.

During this process, it needs to load symbols in a dso and it'd call dso__load_vmlinux() since the default value of the symbol_conf.try_ vmlinux_path is true. However it changes dso->long_name to a real path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux) if one is running on a custom kernel.

It resulted in that perf report reads the build-id of the vmlinux, but cannot use it since it only knows about the [kernel.kallsyms] map. It then falls back to possible vmlinux paths by using the recorded kernel version (in case of a recent version) or a running kernel silently.

Even with the recent tools, this still has a possibility of breaking the result. As the build directory is a symbolic link, if one built a new kernel in the same directory with different source/config, the old link to vmlinux will point the new file. So it's absolutely needed to use build-id when finding a kernel image.

In this patch, it's now changed to try to search a kernel dso using "vmlinux" shortname (which always has a build-id) and, if not found, search "[kernel.kallsyms]".

Before:

  $ perf report
  # Children      Self  Command  Shared Object      Symbol
  # ........  ........  .......  .................  ...............................
  #
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] set_curr_task_rt
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] native_calibrate_tsc
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] tsc_refine_calibration_work
      71.87%    71.87%  swapper  [kernel.kallsyms]  [k] module_finalize
   ...

After (for the same perf.data):

      72.15%     0.00%  swapper  vmlinux  [k] cpu_startup_entry
      72.15%     0.00%  swapper  vmlinux  [k] arch_cpu_idle
      72.15%     0.00%  swapper  vmlinux  [k] default_idle
      71.87%    71.87%  swapper  vmlinux  [k] native_safe_halt

...

---
 tools/perf/util/machine.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/util/machine.c +7 -3
index b2ec38bf211e..d9f828f3b54f 100644
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);
+
+		if (kernel == NULL)
+			kernel = __dsos__findnew(&machine->kernel_dsos,
+						 kmmap_prefix);
 		if (kernel == NULL)
 			goto out_problem;
 
Signature · 1 line
2.1.0
reviewer
#1 lore raw
Quoted 64 lines · Namhyung Kim

There's a problem on finding correct kernel symbols when perf report runs on a different kernel. Although a part of the problem was solved by the prior commit 0a7e6d1b6844 ("perf tools: Check recorded kernel version when finding vmlinux"), there's a remaining problem still.

When perf records samples, it synthesizes the kernel map using machine__mmap_name() and ref_reloc_sym like "[kernel.kallsyms]_text". You can easily see it using 'perf report -D' command.

After finishing record, it goes through the recorded events to find maps/dsos actually used. And then record build-id info of them.

During this process, it needs to load symbols in a dso and it'd call dso__load_vmlinux() since the default value of the symbol_conf.try_ vmlinux_path is true. However it changes dso->long_name to a real path of the vmlinux file (e.g. /lib/modules/3.16.0-rc2+/build/vmlinux) if one is running on a custom kernel.

It resulted in that perf report reads the build-id of the vmlinux, but cannot use it since it only knows about the [kernel.kallsyms] map. It then falls back to possible vmlinux paths by using the recorded kernel version (in case of a recent version) or a running kernel silently.

Even with the recent tools, this still has a possibility of breaking the result. As the build directory is a symbolic link, if one built a new kernel in the same directory with different source/config, the old link to vmlinux will point the new file. So it's absolutely needed to use build-id when finding a kernel image.

In this patch, it's now changed to try to search a kernel dso using "vmlinux" shortname (which always has a build-id) and, if not found, search "[kernel.kallsyms]".

Before:

  $ perf report
  # Children      Self  Command  Shared Object      Symbol
  # ........  ........  .......  .................  ...............................
  #
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] set_curr_task_rt
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] native_calibrate_tsc
      72.15%     0.00%  swapper  [kernel.kallsyms]  [k] tsc_refine_calibration_work
      71.87%    71.87%  swapper  [kernel.kallsyms]  [k] module_finalize
   ...

After (for the same perf.data):

      72.15%     0.00%  swapper  vmlinux  [k] cpu_startup_entry
      72.15%     0.00%  swapper  vmlinux  [k] arch_cpu_idle
      72.15%     0.00%  swapper  vmlinux  [k] default_idle
      71.87%    71.87%  swapper  vmlinux  [k] native_safe_halt

...

---
 tools/perf/util/machine.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/util/machine.c +0 -0
index b2ec38bf211e..d9f828f3b54f 100644

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 10 lines
tools/perf/util/machine.c +3 -3
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 8 lines
tools/perf/util/machine.c +4 -0
+
+		if (kernel == NULL)
+			kernel = __dsos__findnew(&machine->kernel_dsos,
+						 kmmap_prefix);
 		if (kernel == NULL)
 			goto out_problem;
 
#2 lore raw

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

- Arnaldo

author maintainer
#3 lore raw

Hi Adrian and Arnaldo,

2014-09-05 (금), 11:11 -0300, Arnaldo Carvalho de Melo:

Quoted 20 lines

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 6 lines

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

I don't get it. AFAIK when perf record runs, it uses [kernel.kallsyms] name for kernel map. But it can be changed only if it found a "vmlinux" file in the vmlinux_path[]. So short name will always be "vmlinux" - okay, it might be vmlinux-$(uname -r) too; I can add it.

As you know, the vmlinux file is a ELF image that created during kernel build process so I guess its name is fixed. It's different from the vmlinu"z" which resides in /boot directory.

Am I missing something?

Thanks,
Namhyung

#4 lore raw

Em Sat, Sep 06, 2014 at 12:16:40AM +0900, Namhyung Kim escreveu:

Quoted 35 lines

Hi Adrian and Arnaldo,

2014-09-05 (금), 11:11 -0300, Arnaldo Carvalho de Melo:

Quoted 20 lines

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 6 lines

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

I don't get it. AFAIK when perf record runs, it uses [kernel.kallsyms] name for kernel map. But it can be changed only if it found a "vmlinux" file in the vmlinux_path[]. So short name will always be "vmlinux" - okay, it might be vmlinux-$(uname -r) too; I can add it.

Quoted 5 lines

As you know, the vmlinux file is a ELF image that created during kernel build process so I guess its name is fixed. It's different from the vmlinu"z" which resides in /boot directory.

Am I missing something?

[acme@zoo linux]$ perf report --help 2>&1 | grep vmlin

       -k, --vmlinux=<file>
           vmlinux pathname

[acme@zoo linux]$

Yes, it is using [kernel.kallsyms].ref_reloc_sym for the synthesized KERNEL mmap event, but it stores the full path in the build-id table.

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name. That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

- Arnaldo

#5 lore raw
Quoted 47 lines · Arnaldo Carvalho de Melo

Em Sat, Sep 06, 2014 at 12:16:40AM +0900, Namhyung Kim escreveu:

Quoted 35 lines

Hi Adrian and Arnaldo,

2014-09-05 (금), 11:11 -0300, Arnaldo Carvalho de Melo:

Quoted 20 lines

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
                                strlen(kmmap_prefix));
                /*
                 * Should be there already, from the build-id table in
-                * the header.
+                * the header (but maybe with a different name: "vmlinux").
                 */
-               struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-                                                    kmmap_prefix);
+               struct dso *kernel = dsos__find(&machine->kernel_dsos,
+                                               "vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 6 lines

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

I don't get it. AFAIK when perf record runs, it uses [kernel.kallsyms] name for kernel map. But it can be changed only if it found a "vmlinux" file in the vmlinux_path[]. So short name will always be "vmlinux" - okay, it might be vmlinux-$(uname -r) too; I can add it.

Quoted 5 lines

As you know, the vmlinux file is a ELF image that created during kernel build process so I guess its name is fixed. It's different from the vmlinu"z" which resides in /boot directory.

Am I missing something?

[acme@zoo linux]$ perf report --help 2>&1 | grep vmlin

       -k, --vmlinux=<file>
           vmlinux pathname

[acme@zoo linux]$

Quoted 8 lines

Yes, it is using [kernel.kallsyms].ref_reloc_sym for the synthesized KERNEL mmap event, but it stores the full path in the build-id table.

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name. That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

Can't you figure out it is from the kernel (in perf record) by remembering you got the buildid from /sys/kernel/notes?

Quoted 6 lines

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

- Arnaldo

#6 lore raw

Em Sun, Sep 07, 2014 at 10:24:12PM +0200, Stephane Eranian escreveu:

Quoted 52 lines
Quoted 50 lines · Arnaldo Carvalho de Melo

Em Sat, Sep 06, 2014 at 12:16:40AM +0900, Namhyung Kim escreveu:

Quoted 35 lines

Hi Adrian and Arnaldo,

2014-09-05 (금), 11:11 -0300, Arnaldo Carvalho de Melo:

Quoted 20 lines

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
                                strlen(kmmap_prefix));
                /*
                 * Should be there already, from the build-id table in
-                * the header.
+                * the header (but maybe with a different name: "vmlinux").
                 */
-               struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-                                                    kmmap_prefix);
+               struct dso *kernel = dsos__find(&machine->kernel_dsos,
+                                               "vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 6 lines

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

I don't get it. AFAIK when perf record runs, it uses [kernel.kallsyms] name for kernel map. But it can be changed only if it found a "vmlinux" file in the vmlinux_path[]. So short name will always be "vmlinux" - okay, it might be vmlinux-$(uname -r) too; I can add it.

Quoted 5 lines

As you know, the vmlinux file is a ELF image that created during kernel build process so I guess its name is fixed. It's different from the vmlinu"z" which resides in /boot directory.

Am I missing something?

[acme@zoo linux]$ perf report --help 2>&1 | grep vmlin

       -k, --vmlinux=<file>
           vmlinux pathname

[acme@zoo linux]$

Yes, it is using [kernel.kallsyms].ref_reloc_sym for the synthesized KERNEL mmap event, but it stores the full path in the build-id table.

Quoted 7 lines
Quoted 5 lines

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name. That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

Can't you figure out it is from the kernel (in perf record) by remembering you got the buildid from /sys/kernel/notes?

Sure, the question was more about how to encode that in the build-id table in the perf.data file, i.e. we don't have a flags field there that we could use a bit for this purpose (hey, this entry is for the kernel, please match it to the PERF_RECORD_MMAP [kernel_kallsyms] synthesized for the kernel).

We know it is a kernel because the name is [kernel.kallsyms] in the synthesized PERF_RECORD_MMAP. So perhaps we should have the kernel pathname right after the reference relocation symbol, that way we would use it to get the entry in the build-id table.

I.e. parts of what we need are in the synthesized PERF_RECORD_MMAP (how to do relocation, start-end kernel mmap, and we have parts of it in the build-id table: the build-id and the full pathname. How to connect both is what we're trying to achieve here, and in a backwards compatible way.

The (ugly) way I found to achieve all this is to concatenate the full pathname right after the ref reloc symbol in the synthesized PERF_RECORD_MMAP for the kernel mmap.

Older tools would just ignore what is after the ref reloc symbol (need to check if we put the zero there, if not, we can add the zero + the full pathname.).

Newer tools would look if the size of the record is longer than the strlen(ref_reloc_symbol), and if it is, it means we have the kernel full pathname after the ref_reloc_symbol, that we can use to find the build-id in the build-id table.

In the future this is all moot if we add the build-id to all PERF_RECORD_MMAP events, as described below:

Quoted 4 lines
Quoted 4 lines

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

- Arnaldo

author maintainer
#7 lore raw
Quoted 9 lines · Arnaldo Carvalho de Melo

Em Sun, Sep 07, 2014 at 10:24:12PM +0200, Stephane Eranian escreveu:

Quoted 2 lines

Can't you figure out it is from the kernel (in perf record) by remembering you got the buildid from /sys/kernel/notes?

Sure, the question was more about how to encode that in the build-id table in the perf.data file, i.e. we don't have a flags field there that we could use a bit for this purpose (hey, this entry is for the kernel, please match it to the PERF_RECORD_MMAP [kernel_kallsyms] synthesized for the kernel).

So why not adding a new build_id2_event? :)

Quoted 16 lines

We know it is a kernel because the name is [kernel.kallsyms] in the synthesized PERF_RECORD_MMAP. So perhaps we should have the kernel pathname right after the reference relocation symbol, that way we would use it to get the entry in the build-id table.

I.e. parts of what we need are in the synthesized PERF_RECORD_MMAP (how to do relocation, start-end kernel mmap, and we have parts of it in the build-id table: the build-id and the full pathname. How to connect both is what we're trying to achieve here, and in a backwards compatible way.

The (ugly) way I found to achieve all this is to concatenate the full pathname right after the ref reloc symbol in the synthesized PERF_RECORD_MMAP for the kernel mmap.

Quoted 8 lines

Older tools would just ignore what is after the ref reloc symbol (need to check if we put the zero there, if not, we can add the zero + the full pathname.).

Newer tools would look if the size of the record is longer than the strlen(ref_reloc_symbol), and if it is, it means we have the kernel full pathname after the ref_reloc_symbol, that we can use to find the build-id in the build-id table.

Or else, what about synthesizing a build-id event right after the kernel mmap event (in perf_event__synthesize_kernel_mmap)? This way we can have same effect of extending the mmap event while keeping backward compatibility IMHO. At report time, we might need to set a flag that it has just seen the kernel mmap event so that it expects to see a matching build-id event (use it as a kernel dso).

Thanks,
Namhyung

Quoted 10 lines

In the future this is all moot if we add the build-id to all PERF_RECORD_MMAP events, as described below:

Quoted 4 lines
Quoted 4 lines

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

- Arnaldo

author maintainer
#8 lore raw
In reply to Arnaldo Carvalho de Melo

Hi Arnaldo,

Quoted 40 lines · Arnaldo Carvalho de Melo

Em Sat, Sep 06, 2014 at 12:16:40AM +0900, Namhyung Kim escreveu:

Quoted 39 lines

Hi Adrian and Arnaldo,

2014-09-05 (금), 11:11 -0300, Arnaldo Carvalho de Melo:

Quoted 20 lines

Em Fri, Sep 05, 2014 at 10:22:40AM +0300, Adrian Hunter escreveu:

Quoted 10 lines · Namhyung Kim
Quoted 9 lines

+++ b/tools/perf/util/machine.c

tools/perf/util/machine.c +1 -2
@@ -1060,10 +1060,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix));
 		/*
 		 * Should be there already, from the build-id table in
-		 * the header.
+		 * the header (but maybe with a different name: "vmlinux").
 		 */
-		struct dso *kernel = __dsos__findnew(&machine->kernel_dsos,
Quoted 8 lines

tools/perf/util/machine.c static int machine__process_kernel_mmap_event(struct machine *machine, · line 1066

Quoted 3 lines
tools/perf/util/machine.c +2 -1
-						     kmmap_prefix);
+		struct dso *kernel = dsos__find(&machine->kernel_dsos,
+						"vmlinux", true);

Isn't "vmlinux" just the basename of the original file name, so if it had a different name this wouldn't work e.g. if the filename had been /boot/vmlinuz-3.11.0-26-generic then you would need "vmlinuz-3.11.0-26-generic" for this to work?

Quoted 6 lines

Yeah, looking for well known pathnames to then check if the build-id matches the one we're looking for, be it because we obtained it from /sys/kernel/notes (for the running kernel), or from the perf.data file build-id table is ok, as we don't know where it is.

Plain sticking "vmlinux" there is not.

I don't get it. AFAIK when perf record runs, it uses [kernel.kallsyms] name for kernel map. But it can be changed only if it found a "vmlinux" file in the vmlinux_path[]. So short name will always be "vmlinux" - okay, it might be vmlinux-$(uname -r) too; I can add it.

As you know, the vmlinux file is a ELF image that created during kernel build process so I guess its name is fixed. It's different from the vmlinu"z" which resides in /boot directory.

Quoted 6 lines
Quoted 1 line

Am I missing something?

[acme@zoo linux]$ perf report --help 2>&1 | grep vmlin

       -k, --vmlinux=<file>
           vmlinux pathname

[acme@zoo linux]$

Hmm.. this is a report option to specify a vmlinux file rather than finding it using recorded info. When this option was given by user, it doesn't even try to search the vmlinux path (or build-id table) and just uses the given path.

Quoted 6 lines

Yes, it is using [kernel.kallsyms].ref_reloc_sym for the synthesized KERNEL mmap event, but it stores the full path in the build-id table.

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name.

Maybe we can add a new build_id2_event (like mmap2) that has a new field to record that info.

Quoted 7 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Thanks,
Namhyung

#9 lore raw

Em Fri, Sep 12, 2014 at 03:14:51PM +0900, Namhyung Kim escreveu:

Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name.

Maybe we can add a new build_id2_event (like mmap2) that has a new field to record that info.

Humm, take a look at machine__write_buildid_table() -> write_buildid(), we already seem to set build_id_event.header.misc suitably, no?

Quoted 9 lines
Quoted 7 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Been there, done that, backtracked, yes, we can't do that at 'record time', else we would be adding way too much noise to the recording phase.

The idea is for the _kernel_ to do that, would take sizeof(buildid) (about 20 bytes) on the per-DSO structure in the kernel.

When ELF loading it, stashing the contents of an ELF session (.note.gnu.build-id) somewhere accessible at PERF_RECORD_MMAP3 generation time.

Doing that at the time we load the DSO from disk should add negligible overhead.

With that we would not need to go over all the perf.data stream to generate the build-id table after all record sessions, they would be at the struct map already.

We could then have multiple entries in the build-id table for the same pathname, with different build ids, that is something we don't support now and that provides bogus results when it happens.

- Arnaldo

author maintainer
#10 lore raw
Quoted 10 lines · Arnaldo Carvalho de Melo

Em Fri, Sep 12, 2014 at 03:14:51PM +0900, Namhyung Kim escreveu:

Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name.

Maybe we can add a new build_id2_event (like mmap2) that has a new field to record that info.

Humm, take a look at machine__write_buildid_table() -> write_buildid(), we already seem to set build_id_event.header.misc suitably, no?

AFAIK the PERF_RECORD_MISC_KERNEL is set for both of kernel image and modules so we cannot distinguish a kernel from others (without checking names).

Quoted 22 lines
Quoted 10 lines
Quoted 8 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Been there, done that, backtracked, yes, we can't do that at 'record time', else we would be adding way too much noise to the recording phase.

The idea is for the _kernel_ to do that, would take sizeof(buildid) (about 20 bytes) on the per-DSO structure in the kernel.

When ELF loading it, stashing the contents of an ELF session (.note.gnu.build-id) somewhere accessible at PERF_RECORD_MMAP3 generation time.

Quoted 6 lines

Doing that at the time we load the DSO from disk should add negligible overhead.

With that we would not need to go over all the perf.data stream to generate the build-id table after all record sessions, they would be at the struct map already.

Yes, it'll solve the problem and simplifies the perf record. However I suspect it's not acceptable to keep such info in the kernel only for the sake of perf.

Quoted 4 lines

We could then have multiple entries in the build-id table for the same pathname, with different build ids, that is something we don't support now and that provides bogus results when it happens.

Maybe we can save and compare timestamp of build-id/map event and sample event. I'm experimenting to separate meta events (comm, mmap, ...) from sample events with a similar idea... :)

Thanks,
Namhyung

#11 lore raw

Em Fri, Sep 19, 2014 at 03:26:25PM +0900, Namhyung Kim escreveu:

Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 7 lines

Em Fri, Sep 12, 2014 at 03:14:51PM +0900, Namhyung Kim escreveu:

Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name.

Maybe we can add a new build_id2_event (like mmap2) that has a new field to record that info.

Quoted 6 lines
Quoted 2 lines

Humm, take a look at machine__write_buildid_table() -> write_buildid(), we already seem to set build_id_event.header.misc suitably, no?

AFAIK the PERF_RECORD_MISC_KERNEL is set for both of kernel image and modules so we cannot distinguish a kernel from others (without checking names).

Ok, not perfect, but I think we improve the situation by using this piece of information while looking if the file ends in .ko, in which case we can mark it as a kernel module, if it doesn't end in .ko, we have a kernel.

Better than today, probably good enough, no?

Quoted 24 lines
Quoted 23 lines
Quoted 9 lines
Quoted 7 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Been there, done that, backtracked, yes, we can't do that at 'record time', else we would be adding way too much noise to the recording phase.

The idea is for the _kernel_ to do that, would take sizeof(buildid) (about 20 bytes) on the per-DSO structure in the kernel.

When ELF loading it, stashing the contents of an ELF session (.note.gnu.build-id) somewhere accessible at PERF_RECORD_MMAP3 generation time.

Doing that at the time we load the DSO from disk should add negligible overhead.

Quoted 7 lines
Quoted 3 lines

With that we would not need to go over all the perf.data stream to generate the build-id table after all record sessions, they would be at the struct map already.

Yes, it'll solve the problem and simplifies the perf record. However I suspect it's not acceptable to keep such info in the kernel only for the sake of perf.

Hey, perf is a crucial part of development, even if it was the sole user, I think this would be acceptable.

But this is not perf specific at all, any other tool that needs to map from sample to a symtab _needs_ this, there is no other way to, on a system with running apps and its libraries, to safely go from DSO pathname to its corresponding binary.

Also this enables one to unambiguously describe a running environment without packing tons of duplicated payloads for multiple workload runs, be it for profiling or for any other problem characterization task.

And we have build-ids available for quite a while in all DSOs in the system.

I bet gdb, systemtap, etc would be glad to use this if available.

Quoted 7 lines
Quoted 3 lines

We could then have multiple entries in the build-id table for the same pathname, with different build ids, that is something we don't support now and that provides bogus results when it happens.

Maybe we can save and compare timestamp of build-id/map event and sample event. I'm experimenting to separate meta events (comm, mmap, ...) from sample events with a similar idea... :)

The point is to go from an entry in /proc/pid/maps to the file where it was loaded from, in a system where updates may have taken place for that pathname.

- Arnaldo

author maintainer
#12 lore raw

2014-09-19 (금), 11:16 -0300, Arnaldo Carvalho de Melo:

Quoted 17 lines

Em Fri, Sep 19, 2014 at 03:26:25PM +0900, Namhyung Kim escreveu:

Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 7 lines

Em Fri, Sep 12, 2014 at 03:14:51PM +0900, Namhyung Kim escreveu:

Quoted 6 lines · Arnaldo Carvalho de Melo
Quoted 2 lines

It seems we need a way to state that an entry in the build-id table is for the kernel, without looking at its file name.

Maybe we can add a new build_id2_event (like mmap2) that has a new field to record that info.

Quoted 6 lines
Quoted 2 lines

Humm, take a look at machine__write_buildid_table() -> write_buildid(), we already seem to set build_id_event.header.misc suitably, no?

AFAIK the PERF_RECORD_MISC_KERNEL is set for both of kernel image and modules so we cannot distinguish a kernel from others (without checking names).

Quoted 6 lines

Ok, not perfect, but I think we improve the situation by using this piece of information while looking if the file ends in .ko, in which case we can mark it as a kernel module, if it doesn't end in .ko, we have a kernel.

Better than today, probably good enough, no?

Yep, I'll check .ko suffix in v3.

Quoted 41 lines
Quoted 25 lines
Quoted 24 lines
Quoted 10 lines
Quoted 8 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Been there, done that, backtracked, yes, we can't do that at 'record time', else we would be adding way too much noise to the recording phase.

The idea is for the _kernel_ to do that, would take sizeof(buildid) (about 20 bytes) on the per-DSO structure in the kernel.

When ELF loading it, stashing the contents of an ELF session (.note.gnu.build-id) somewhere accessible at PERF_RECORD_MMAP3 generation time.

Doing that at the time we load the DSO from disk should add negligible overhead.

Quoted 7 lines
Quoted 3 lines

With that we would not need to go over all the perf.data stream to generate the build-id table after all record sessions, they would be at the struct map already.

Yes, it'll solve the problem and simplifies the perf record. However I suspect it's not acceptable to keep such info in the kernel only for the sake of perf.

Hey, perf is a crucial part of development, even if it was the sole user, I think this would be acceptable.

But this is not perf specific at all, any other tool that needs to map from sample to a symtab _needs_ this, there is no other way to, on a system with running apps and its libraries, to safely go from DSO pathname to its corresponding binary.

Quoted 8 lines

Also this enables one to unambiguously describe a running environment without packing tons of duplicated payloads for multiple workload runs, be it for profiling or for any other problem characterization task.

And we have build-ids available for quite a while in all DSOs in the system.

I bet gdb, systemtap, etc would be glad to use this if available.

Hmm.. okay then. But IIUC kernel only knows about an executable (and dynamic linker) at load time not libraries as they're loaded by the linker. So kernel needs to check whether a file is an ELF and has build-id for every file, right?

Thanks,
Namhyung

Quoted 14 lines
Quoted 8 lines
Quoted 4 lines

We could then have multiple entries in the build-id table for the same pathname, with different build ids, that is something we don't support now and that provides bogus results when it happens.

Maybe we can save and compare timestamp of build-id/map event and sample event. I'm experimenting to separate meta events (comm, mmap, ...) from sample events with a similar idea... :)

The point is to go from an entry in /proc/pid/maps to the file where it was loaded from, in a system where updates may have taken place for that pathname.

- Arnaldo

#13 lore raw

Em Sat, Sep 20, 2014 at 12:42:46AM +0900, Namhyung Kim escreveu:

Quoted 12 lines

2014-09-19 (금), 11:16 -0300, Arnaldo Carvalho de Melo:

Quoted 11 lines

Em Fri, Sep 19, 2014 at 03:26:25PM +0900, Namhyung Kim escreveu:

Quoted 9 lines · Arnaldo Carvalho de Melo
Quoted 4 lines

Em Fri, Sep 12, 2014 at 03:14:51PM +0900, Namhyung Kim escreveu:

Quoted 1 line · Arnaldo Carvalho de Melo

Humm, take a look at machine__write_buildid_table() -> write_buildid(), we already seem to set build_id_event.header.misc suitably, no?

AFAIK the PERF_RECORD_MISC_KERNEL is set for both of kernel image and modules so we cannot distinguish a kernel from others (without checking names).

Quoted 8 lines
Quoted 6 lines

Ok, not perfect, but I think we improve the situation by using this piece of information while looking if the file ends in .ko, in which case we can mark it as a kernel module, if it doesn't end in .ko, we have a kernel.

Better than today, probably good enough, no?

Yep, I'll check .ko suffix in v3.

Thanks!

Quoted 47 lines
Quoted 47 lines
Quoted 24 lines
Quoted 23 lines
Quoted 9 lines
Quoted 7 lines

That or to put the build-id into the synthesized kernel mmap event. Which is better, because:

That leads to another problem that needs to get solved eventually: We need to have the build-id into PERF_RECORD_MMAP, because we're now using just the mmap filename as the key, not the contents, and for long running sessions, DSOs can get updated, etc.

But it'd require a realtime processing of events at record time.

Been there, done that, backtracked, yes, we can't do that at 'record time', else we would be adding way too much noise to the recording phase.

The idea is for the _kernel_ to do that, would take sizeof(buildid) (about 20 bytes) on the per-DSO structure in the kernel.

When ELF loading it, stashing the contents of an ELF session (.note.gnu.build-id) somewhere accessible at PERF_RECORD_MMAP3 generation time.

Doing that at the time we load the DSO from disk should add negligible overhead.

Quoted 7 lines
Quoted 3 lines

With that we would not need to go over all the perf.data stream to generate the build-id table after all record sessions, they would be at the struct map already.

Yes, it'll solve the problem and simplifies the perf record. However I suspect it's not acceptable to keep such info in the kernel only for the sake of perf.

Hey, perf is a crucial part of development, even if it was the sole user, I think this would be acceptable.

But this is not perf specific at all, any other tool that needs to map from sample to a symtab _needs_ this, there is no other way to, on a system with running apps and its libraries, to safely go from DSO pathname to its corresponding binary.

Also this enables one to unambiguously describe a running environment without packing tons of duplicated payloads for multiple workload runs, be it for profiling or for any other problem characterization task.

And we have build-ids available for quite a while in all DSOs in the system.

Quoted 6 lines
Quoted 1 line

I bet gdb, systemtap, etc would be glad to use this if available.

Hmm.. okay then. But IIUC kernel only knows about an executable (and dynamic linker) at load time not libraries as they're loaded by the linker. So kernel needs to check whether a file is an ELF and has build-id for every file, right?

Yes, when it figures out it is an ELF file and invokes this DSO loading routines, there is where we should insert the code that will read/store the build id so that when we emit the PERF_RECORD_MMAP3 we can read it.

- Arnaldo