[PATCH V2 0/9] perf tools: kaslr fixes

[PATCH V2 0/9] perf tools: kaslr fixes

[Adrian Hunter]

Hi

Here are some patches that improve perf tools
handling of relocation.

This has become an issue as mentioned in this
thread:

	http://marc.info/?l=linux-kernel&m=139030004314756

Changes in V2:
	Fix some typos in commit messages
	perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()
		Fix "/proc/kallsyms" -> filename
		Use an array of symbol names
	perf buildid-cache: Check relocation when checking for existing kcore
		New patch


Adrian Hunter (9):
      perf tools: Fix symbol annotation for relocated kernel
      perf tools: Add kallsyms__get_function_start()
      perf tools: Add machine__get_kallsyms_filename()
      perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()
      perf record: Get ref_reloc_sym from kernel map
      perf tools: Prevent the use of kcore if the kernel has moved
      perf tools: Test does not need to set up ref_reloc_sym
      perf tools: Adjust kallsyms for relocated kernel
      perf buildid-cache: Check relocation when checking for existing kcore

 tools/perf/builtin-buildid-cache.c  | 33 ++++++++++++++++---
 tools/perf/builtin-record.c         | 10 ++----
 tools/perf/tests/vmlinux-kallsyms.c | 10 ------
 tools/perf/util/event.c             | 36 ++++++++++----------
 tools/perf/util/event.h             |  6 ++--
 tools/perf/util/machine.c           | 42 +++++++++++++++++++-----
 tools/perf/util/machine.h           |  2 ++
 tools/perf/util/map.c               |  5 +--
 tools/perf/util/map.h               |  1 +
 tools/perf/util/symbol-elf.c        |  2 ++
 tools/perf/util/symbol.c            | 65 +++++++++++++++++++++++++++++++++----
 11 files changed, 153 insertions(+), 59 deletions(-)


Regards
Adrian

[PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Adrian Hunter]

Kernel maps map memory addresses to file offsets.
For symbol annotation, objdump needs the object VMA
addresses.  For an unrelocated kernel, that is the
same as the memory address.

The addresses passed to objdump for symbol annotation
did not take into account kernel relocation.  This
patch fixes that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/map.c        | 5 +++--
 tools/perf/util/map.h        | 1 +
 tools/perf/util/symbol-elf.c | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index ee1dd68..b46f527 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
 	map->start    = start;
 	map->end      = end;
 	map->pgoff    = pgoff;
+	map->reloc    = 0;
 	map->dso      = dso;
 	map->map_ip   = map__map_ip;
 	map->unmap_ip = map__unmap_ip;
@@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
 	if (map->dso->rel)
 		return rip - map->pgoff;
 
-	return map->unmap_ip(map, rip);
+	return map->unmap_ip(map, rip) - map->reloc;
 }
 
 /**
@@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 	if (map->dso->rel)
 		return map->unmap_ip(map, ip + map->pgoff);
 
-	return ip;
+	return ip + map->reloc;
 }
 
 void map_groups__init(struct map_groups *mg)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 18068c6..257e513 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -36,6 +36,7 @@ struct map {
 	bool			erange_warned;
 	u32			priv;
 	u64			pgoff;
+	u64			reloc;
 	u32			maj, min; /* only valid for MMAP2 record */
 	u64			ino;      /* only valid for MMAP2 record */
 	u64			ino_generation;/* only valid for MMAP2 record */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7594567..8ce52da 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
 			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
 				continue;
 			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
+			map->reloc = kmap->ref_reloc_sym->addr -
+				     kmap->ref_reloc_sym->unrelocated_addr;
 			break;
 		}
 	}
-- 
1.7.11.7

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Arnaldo Carvalho de Melo]

Em Wed, Jan 29, 2014 at 04:14:36PM +0200, Adrian Hunter escreveu:
> Kernel maps map memory addresses to file offsets.
> For symbol annotation, objdump needs the object VMA
> addresses.  For an unrelocated kernel, that is the
> same as the memory address.
> 
> The addresses passed to objdump for symbol annotation
> did not take into account kernel relocation.  This
> patch fixes that.

Question: To fix the problem reported by Linus, i.e. the very minimal
fix, we only need this patch, right?

Reading the other patches now.

- Arnaldo
 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/map.c        | 5 +++--
>  tools/perf/util/map.h        | 1 +
>  tools/perf/util/symbol-elf.c | 2 ++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index ee1dd68..b46f527 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
>  	map->start    = start;
>  	map->end      = end;
>  	map->pgoff    = pgoff;
> +	map->reloc    = 0;
>  	map->dso      = dso;
>  	map->map_ip   = map__map_ip;
>  	map->unmap_ip = map__unmap_ip;
> @@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
>  	if (map->dso->rel)
>  		return rip - map->pgoff;
>  
> -	return map->unmap_ip(map, rip);
> +	return map->unmap_ip(map, rip) - map->reloc;
>  }
>  
>  /**
> @@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
>  	if (map->dso->rel)
>  		return map->unmap_ip(map, ip + map->pgoff);
>  
> -	return ip;
> +	return ip + map->reloc;
>  }
>  
>  void map_groups__init(struct map_groups *mg)
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 18068c6..257e513 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -36,6 +36,7 @@ struct map {
>  	bool			erange_warned;
>  	u32			priv;
>  	u64			pgoff;
> +	u64			reloc;
>  	u32			maj, min; /* only valid for MMAP2 record */
>  	u64			ino;      /* only valid for MMAP2 record */
>  	u64			ino_generation;/* only valid for MMAP2 record */
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 7594567..8ce52da 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
>  			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
>  				continue;
>  			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
> +			map->reloc = kmap->ref_reloc_sym->addr -
> +				     kmap->ref_reloc_sym->unrelocated_addr;
>  			break;
>  		}
>  	}
> -- 
> 1.7.11.7

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Adrian Hunter]

On 29/01/14 20:57, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 29, 2014 at 04:14:36PM +0200, Adrian Hunter escreveu:
>> Kernel maps map memory addresses to file offsets.
>> For symbol annotation, objdump needs the object VMA
>> addresses.  For an unrelocated kernel, that is the
>> same as the memory address.
>>
>> The addresses passed to objdump for symbol annotation
>> did not take into account kernel relocation.  This
>> patch fixes that.
> 
> Question: To fix the problem reported by Linus, i.e. the very minimal
> fix, we only need this patch, right?

Yes but the other fixes are needed too.

> 
> Reading the other patches now.
> 
> - Arnaldo
>  
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/map.c        | 5 +++--
>>  tools/perf/util/map.h        | 1 +
>>  tools/perf/util/symbol-elf.c | 2 ++
>>  3 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
>> index ee1dd68..b46f527 100644
>> --- a/tools/perf/util/map.c
>> +++ b/tools/perf/util/map.c
>> @@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
>>  	map->start    = start;
>>  	map->end      = end;
>>  	map->pgoff    = pgoff;
>> +	map->reloc    = 0;
>>  	map->dso      = dso;
>>  	map->map_ip   = map__map_ip;
>>  	map->unmap_ip = map__unmap_ip;
>> @@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
>>  	if (map->dso->rel)
>>  		return rip - map->pgoff;
>>  
>> -	return map->unmap_ip(map, rip);
>> +	return map->unmap_ip(map, rip) - map->reloc;
>>  }
>>  
>>  /**
>> @@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
>>  	if (map->dso->rel)
>>  		return map->unmap_ip(map, ip + map->pgoff);
>>  
>> -	return ip;
>> +	return ip + map->reloc;
>>  }
>>  
>>  void map_groups__init(struct map_groups *mg)
>> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
>> index 18068c6..257e513 100644
>> --- a/tools/perf/util/map.h
>> +++ b/tools/perf/util/map.h
>> @@ -36,6 +36,7 @@ struct map {
>>  	bool			erange_warned;
>>  	u32			priv;
>>  	u64			pgoff;
>> +	u64			reloc;
>>  	u32			maj, min; /* only valid for MMAP2 record */
>>  	u64			ino;      /* only valid for MMAP2 record */
>>  	u64			ino_generation;/* only valid for MMAP2 record */
>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>> index 7594567..8ce52da 100644
>> --- a/tools/perf/util/symbol-elf.c
>> +++ b/tools/perf/util/symbol-elf.c
>> @@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
>>  			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
>>  				continue;
>>  			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
>> +			map->reloc = kmap->ref_reloc_sym->addr -
>> +				     kmap->ref_reloc_sym->unrelocated_addr;
>>  			break;
>>  		}
>>  	}
>> -- 
>> 1.7.11.7
> 
> 

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Ingo Molnar]

* Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 29/01/14 20:57, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jan 29, 2014 at 04:14:36PM +0200, Adrian Hunter escreveu:
> >> Kernel maps map memory addresses to file offsets.
> >> For symbol annotation, objdump needs the object VMA
> >> addresses.  For an unrelocated kernel, that is the
> >> same as the memory address.
> >>
> >> The addresses passed to objdump for symbol annotation
> >> did not take into account kernel relocation.  This
> >> patch fixes that.
> > 
> > Question: To fix the problem reported by Linus, i.e. the very minimal
> > fix, we only need this patch, right?
> 
> Yes but the other fixes are needed too.

So, for the specific case of kernel address layout randomization, how 
does this fix Linus's bug with KASLR enabled? How does the code 
recover the random, runtime offset of the relocated kernel, which 
varies from boot to boot?

Thanks,

	Ingo

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Adrian Hunter]

On 30/01/14 10:59, Ingo Molnar wrote:
> 
> * Adrian Hunter <adrian.hunter@intel.com> wrote:
> 
>> On 29/01/14 20:57, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Jan 29, 2014 at 04:14:36PM +0200, Adrian Hunter escreveu:
>>>> Kernel maps map memory addresses to file offsets.
>>>> For symbol annotation, objdump needs the object VMA
>>>> addresses.  For an unrelocated kernel, that is the
>>>> same as the memory address.
>>>>
>>>> The addresses passed to objdump for symbol annotation
>>>> did not take into account kernel relocation.  This
>>>> patch fixes that.
>>>
>>> Question: To fix the problem reported by Linus, i.e. the very minimal
>>> fix, we only need this patch, right?
>>
>> Yes but the other fixes are needed too.
> 
> So, for the specific case of kernel address layout randomization, how 
> does this fix Linus's bug with KASLR enabled? How does the code 
> recover the random, runtime offset of the relocated kernel, which 
> varies from boot to boot?

By comparing the address of a symbol ("_text" or "_stext")
in /proc/kallsyms (or perf.data - see below) with the same
symbol in vmlinux.

perf tools call this the ref_reloc_sym and stores it in
perf.data hidden in the synthesized kernel mmap record.
e.g.

0xd8 [0x50]: event: 1
.
. ... raw event: size 80 bytes
.  0000:  01 00 00 00 01 00 50 00 ff ff ff ff 00 00 00 00  ......P.........
.  0010:  00 00 00 17 00 00 00 00 ff ff ff a8 ff ff ff ff  ................
.  0020:  c8 01 00 98 ff ff ff ff 5b 6b 65 72 6e 65 6c 2e  ........[kernel.
.  0030:  6b 61 6c 6c 73 79 6d 73 5d 5f 73 74 65 78 74 00  kallsyms]_stext.
.  0040:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
.
0 0xd8 [0x50]: PERF_RECORD_MMAP -1/0: [0x17000000(0xffffffffa8ffffff) @ 0xffffffff980001c8]: x [kernel.kallsyms]_stext

That tells perf tools that _stext was 0xffffffff980001c8.
Compare to vmlinux:

$ objdump -t vmlinux | grep _stext
ffffffff810001c8 g       .text	0000000000000000 _stext

So the relocation is 0xffffffff980001c8 - 0xffffffff810001c8
= 0x17000000

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Ingo Molnar]

* Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 30/01/14 10:59, Ingo Molnar wrote:
> > 
> > * Adrian Hunter <adrian.hunter@intel.com> wrote:
> > 
> >> On 29/01/14 20:57, Arnaldo Carvalho de Melo wrote:
> >>> Em Wed, Jan 29, 2014 at 04:14:36PM +0200, Adrian Hunter escreveu:
> >>>> Kernel maps map memory addresses to file offsets.
> >>>> For symbol annotation, objdump needs the object VMA
> >>>> addresses.  For an unrelocated kernel, that is the
> >>>> same as the memory address.
> >>>>
> >>>> The addresses passed to objdump for symbol annotation
> >>>> did not take into account kernel relocation.  This
> >>>> patch fixes that.
> >>>
> >>> Question: To fix the problem reported by Linus, i.e. the very minimal
> >>> fix, we only need this patch, right?
> >>
> >> Yes but the other fixes are needed too.
> > 
> > So, for the specific case of kernel address layout randomization, how 
> > does this fix Linus's bug with KASLR enabled? How does the code 
> > recover the random, runtime offset of the relocated kernel, which 
> > varies from boot to boot?
> 
> By comparing the address of a symbol ("_text" or "_stext")
> in /proc/kallsyms (or perf.data - see below) with the same
> symbol in vmlinux.
> 
> perf tools call this the ref_reloc_sym and stores it in
> perf.data hidden in the synthesized kernel mmap record.
> e.g.
> 
> 0xd8 [0x50]: event: 1
> .
> . ... raw event: size 80 bytes
> .  0000:  01 00 00 00 01 00 50 00 ff ff ff ff 00 00 00 00  ......P.........
> .  0010:  00 00 00 17 00 00 00 00 ff ff ff a8 ff ff ff ff  ................
> .  0020:  c8 01 00 98 ff ff ff ff 5b 6b 65 72 6e 65 6c 2e  ........[kernel.
> .  0030:  6b 61 6c 6c 73 79 6d 73 5d 5f 73 74 65 78 74 00  kallsyms]_stext.
> .  0040:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> .
> 0 0xd8 [0x50]: PERF_RECORD_MMAP -1/0: [0x17000000(0xffffffffa8ffffff) @ 0xffffffff980001c8]: x [kernel.kallsyms]_stext
> 
> That tells perf tools that _stext was 0xffffffff980001c8.
> Compare to vmlinux:
> 
> $ objdump -t vmlinux | grep _stext
> ffffffff810001c8 g       .text	0000000000000000 _stext
> 
> So the relocation is 0xffffffff980001c8 - 0xffffffff810001c8
> = 0x17000000

Ok, cool, thanks!

I'd suggest the whole fix series if perf/urgent material.

Thanks,

	Ingo

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Arnaldo Carvalho de Melo]

Em Thu, Jan 30, 2014 at 10:20:36AM +0100, Ingo Molnar escreveu:
> Ok, cool, thanks!
> 
> I'd suggest the whole fix series if perf/urgent material.

Getting segfaults with:

[acme@ssdandy linux]$ git log --oneline | head -10
c059c953531f perf tests: vmlinux-kallsyms test does not need to set up ref_reloc_sym
be2530836ebe perf symbols: Prevent the use of kcore if the kernel has moved
9b2fd7840cb0 perf record: Get ref_reloc_sym from kernel map
bcf5e06a0c75 perf symbols: Add kallsyms__get_function_start()
94591ed0963e perf machine: Add machine__get_kallsyms_filename()
18923cddb839 perf symbols: Fix symbol annotation for relocated kernel
f428ebd184c8 perf tools: Fix AAAAARGH64 memory barriers
950b8354716e perf tools: Demangle kernel and kernel module symbols too
0d4dd797564c perf/doc: Remove mention of non-existent set_perf_event_pending() from design.txt
993e5ee67a90 Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent
[acme@ssdandy linux]$ 

[acme@ssdandy linux]$ perf record usleep 1
Segmentation fault (core dumped)
[acme@ssdandy linux]$ uname -a
Linux ssdandy 3.12.0-rc6+ #2 SMP Mon Oct 21 11:19:07 BRT 2013 x86_64 x86_64 x86_64 GNU/Linux
[acme@ssdandy linux]$ cat /etc/fedora-release 
Fedora release 18 (Spherical Cow)
[acme@ssdandy linux]$ 

- Arnaldo

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Arnaldo Carvalho de Melo]

Em Thu, Jan 30, 2014 at 03:08:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 30, 2014 at 10:20:36AM +0100, Ingo Molnar escreveu:
> > I'd suggest the whole fix series if perf/urgent material.
 
> Getting segfaults with:
 
> [acme@ssdandy linux]$ git log --oneline | head -10
> c059c953531f perf tests: vmlinux-kallsyms test does not need to set up ref_reloc_sym
> be2530836ebe perf symbols: Prevent the use of kcore if the kernel has moved
> 9b2fd7840cb0 perf record: Get ref_reloc_sym from kernel map
> bcf5e06a0c75 perf symbols: Add kallsyms__get_function_start()
> 94591ed0963e perf machine: Add machine__get_kallsyms_filename()
> 18923cddb839 perf symbols: Fix symbol annotation for relocated kernel

Program received signal SIGSEGV, Segmentation fault.
perf_event__synthesize_kernel_mmap (tool=0x81fd20 <record>, process=0x42ad51 <process_synthesized_event>, machine=0x916380) at util/event.c:520
520		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
Missing separate debuginfos, use: debuginfo-install audit-libs-2.3.2-1.fc18.x86_64 bzip2-libs-1.0.6-7.fc18.x86_64 elfutils-libelf-0.155-1.fc18.x86_64 elfutils-libs-0.155-1.fc18.x86_64 libunwind-1.0.1-5.fc18.x86_64 nss-softokn-freebl-3.15.2-2.fc18.x86_64 numactl-libs-2.0.7-7.fc18.x86_64 perl-libs-5.16.3-245.fc18.x86_64 python-libs-2.7.3-13.fc18.x86_64 slang-2.2.4-5.fc18.x86_64 xz-libs-5.1.2-2alpha.fc18.x86_64
(gdb) bt
#0  perf_event__synthesize_kernel_mmap (tool=0x81fd20 <record>, process=0x42ad51 <process_synthesized_event>, machine=0x916380) at util/event.c:520
#1  0x000000000042bc24 in __cmd_record (rec=0x81fd20 <record>, argc=2, argv=0x7fffffffe160) at builtin-record.c:456
#2  0x000000000042c960 in cmd_record (argc=2, argv=0x7fffffffe160, prefix=0x0) at builtin-record.c:971
#3  0x000000000041a181 in run_builtin (p=0x81ee30 <commands+144>, argc=3, argv=0x7fffffffe160) at perf.c:319
#4  0x000000000041a3b9 in handle_internal_command (argc=3, argv=0x7fffffffe160) at perf.c:376
#5  0x000000000041a505 in run_argv (argcp=0x7fffffffe03c, argv=0x7fffffffe030) at perf.c:420
#6  0x000000000041a7ec in main (argc=3, argv=0x7fffffffe160) at perf.c:529
(gdb) list
515			event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
516		}
517	
518		map = machine->vmlinux_maps[MAP__FUNCTION];
519		kmap = map__kmap(map);
520		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
521				"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
522		size = PERF_ALIGN(size, sizeof(u64));
523		event->mmap.header.type = PERF_RECORD_MMAP;
524		event->mmap.header.size = (sizeof(event->mmap) -
(gdb) p mmap_name
$4 = 0x7fffffffabd0 "[kernel.kallsyms]"
(gdb) p kmap->ref_reloc_sym
$5 = (struct ref_reloc_sym *) 0x0
(gdb)

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Arnaldo Carvalho de Melo]

Em Thu, Jan 30, 2014 at 03:12:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 30, 2014 at 03:08:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Getting segfaults with:
>  
> > [acme@ssdandy linux]$ git log --oneline | head -10
> > c059c953531f perf tests: vmlinux-kallsyms test does not need to set up ref_reloc_sym
> > be2530836ebe perf symbols: Prevent the use of kcore if the kernel has moved
> > 9b2fd7840cb0 perf record: Get ref_reloc_sym from kernel map
> > bcf5e06a0c75 perf symbols: Add kallsyms__get_function_start()
> > 94591ed0963e perf machine: Add machine__get_kallsyms_filename()
> > 18923cddb839 perf symbols: Fix symbol annotation for relocated kernel
> 
> Program received signal SIGSEGV, Segmentation fault.
> perf_event__synthesize_kernel_mmap (tool=0x81fd20 <record>, process=0x42ad51 <process_synthesized_event>, machine=0x916380) at util/event.c:520
> 520		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
> 521				"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
> (gdb) p mmap_name
> $4 = 0x7fffffffabd0 "[kernel.kallsyms]"
> (gdb) p kmap->ref_reloc_sym
> $5 = (struct ref_reloc_sym *) 0x0
> (gdb)

This is the buggy one:

[acme@ssdandy linux]$ perf record usleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.018 MB perf.data (~770 samples) ]
[acme@ssdandy linux]$ git show 9b2fd7840cb0d23068bc97e08db46f57f41021fe
commit 9b2fd7840cb0d23068bc97e08db46f57f41021fe
Author: Adrian Hunter <adrian.hunter@intel.com>
Date:   Wed Jan 29 16:14:40 2014 +0200

    perf record: Get ref_reloc_sym from kernel map
    
    Now that ref_reloc_sym is set up when the kernel map is created, 'perf
    record' does not need to pass the symbol names to
    perf_event__synthesize_kernel_mmap() which can read the values needed
    from ref_reloc_sym directly.

Re: [PATCH V2 1/9] perf tools: Fix symbol annotation for relocated kernel

[Arnaldo Carvalho de Melo]

Em Thu, Jan 30, 2014 at 03:15:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jan 30, 2014 at 03:12:32PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jan 30, 2014 at 03:08:23PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Getting segfaults with:
> >  
> > > [acme@ssdandy linux]$ git log --oneline | head -10
> > > c059c953531f perf tests: vmlinux-kallsyms test does not need to set up ref_reloc_sym
> > > be2530836ebe perf symbols: Prevent the use of kcore if the kernel has moved
> > > 9b2fd7840cb0 perf record: Get ref_reloc_sym from kernel map

bummer, I got the message with 4/9 delayed and didn't notice it was
missing, after applying it in the intended order, the segfault was
cured.

thanks to Adrian for bringing this to my attention.

- Arnaldo

> > > bcf5e06a0c75 perf symbols: Add kallsyms__get_function_start()
> > > 94591ed0963e perf machine: Add machine__get_kallsyms_filename()
> > > 18923cddb839 perf symbols: Fix symbol annotation for relocated kernel
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > perf_event__synthesize_kernel_mmap (tool=0x81fd20 <record>, process=0x42ad51 <process_synthesized_event>, machine=0x916380) at util/event.c:520
> > 520		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
> > 521				"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
> > (gdb) p mmap_name
> > $4 = 0x7fffffffabd0 "[kernel.kallsyms]"
> > (gdb) p kmap->ref_reloc_sym
> > $5 = (struct ref_reloc_sym *) 0x0
> > (gdb)
> 
> This is the buggy one:
> 
> [acme@ssdandy linux]$ perf record usleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.018 MB perf.data (~770 samples) ]
> [acme@ssdandy linux]$ git show 9b2fd7840cb0d23068bc97e08db46f57f41021fe
> commit 9b2fd7840cb0d23068bc97e08db46f57f41021fe
> Author: Adrian Hunter <adrian.hunter@intel.com>
> Date:   Wed Jan 29 16:14:40 2014 +0200
> 
>     perf record: Get ref_reloc_sym from kernel map
>     
>     Now that ref_reloc_sym is set up when the kernel map is created, 'perf
>     record' does not need to pass the symbol names to
>     perf_event__synthesize_kernel_mmap() which can read the values needed
>     from ref_reloc_sym directly.

[tip:perf/urgent] perf symbols: Fix symbol annotation for relocated kernel

[tip-bot for Adrian Hunter]

Commit-ID:  9176753d1ed56951a6ee2a0f0a3f367904e35567
Gitweb:     http://git.kernel.org/tip/9176753d1ed56951a6ee2a0f0a3f367904e35567
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:36 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:47 -0300

perf symbols: Fix symbol annotation for relocated kernel

Kernel maps map memory addresses to file offsets.

For symbol annotation, objdump needs the object VMA addresses.  For an
unrelocated kernel, that is the same as the memory address.

The addresses passed to objdump for symbol annotation did not take into
account kernel relocation.

This patch fixes that.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-2-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c        | 5 +++--
 tools/perf/util/map.h        | 1 +
 tools/perf/util/symbol-elf.c | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 3b97513..39cd2d0 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
 	map->start    = start;
 	map->end      = end;
 	map->pgoff    = pgoff;
+	map->reloc    = 0;
 	map->dso      = dso;
 	map->map_ip   = map__map_ip;
 	map->unmap_ip = map__unmap_ip;
@@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
 	if (map->dso->rel)
 		return rip - map->pgoff;
 
-	return map->unmap_ip(map, rip);
+	return map->unmap_ip(map, rip) - map->reloc;
 }
 
 /**
@@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 	if (map->dso->rel)
 		return map->unmap_ip(map, ip + map->pgoff);
 
-	return ip;
+	return ip + map->reloc;
 }
 
 void map_groups__init(struct map_groups *mg)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 18068c6..257e513 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -36,6 +36,7 @@ struct map {
 	bool			erange_warned;
 	u32			priv;
 	u64			pgoff;
+	u64			reloc;
 	u32			maj, min; /* only valid for MMAP2 record */
 	u64			ino;      /* only valid for MMAP2 record */
 	u64			ino_generation;/* only valid for MMAP2 record */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 8f12f0f..3e9f336 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
 			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
 				continue;
 			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
+			map->reloc = kmap->ref_reloc_sym->addr -
+				     kmap->ref_reloc_sym->unrelocated_addr;
 			break;
 		}
 	}

[PATCH V2 2/9] perf tools: Add kallsyms__get_function_start()

[Adrian Hunter]

Separate out the logic used to find the start
address of the reference symbol used to track
kernel relocation.  kallsyms__get_function_start()
is used in subsequent patches.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/event.c | 18 +++++++++++++++---
 tools/perf/util/event.h |  3 +++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fc1c2f..17476df 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -470,6 +470,17 @@ static int find_symbol_cb(void *arg, const char *name, char type,
 	return 1;
 }
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name)
+{
+	struct process_symbol_args args = { .name = symbol_name, };
+
+	if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
+		return 0;
+
+	return args.start;
+}
+
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
 				       struct machine *machine,
@@ -480,13 +491,13 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	char path[PATH_MAX];
 	char name_buff[PATH_MAX];
 	struct map *map;
+	u64 start;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
 	 * available use this, and after it is use this as a fallback for older
 	 * kernels.
 	 */
-	struct process_symbol_args args = { .name = symbol_name, };
 	union perf_event *event = zalloc((sizeof(event->mmap) +
 					  machine->id_hdr_size));
 	if (event == NULL) {
@@ -513,7 +524,8 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		}
 	}
 
-	if (kallsyms__parse(filename, &args, find_symbol_cb) <= 0) {
+	start = kallsyms__get_function_start(filename, symbol_name);
+	if (!start) {
 		free(event);
 		return -ENOENT;
 	}
@@ -525,7 +537,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = args.start;
+	event->mmap.pgoff = start;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index faf6e21..66a0c03 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -279,4 +279,7 @@ size_t perf_event__fprintf_mmap2(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_task(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf(union perf_event *event, FILE *fp);
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name);
+
 #endif /* __PERF_RECORD_H */
-- 
1.7.11.7

[tip:perf/urgent] perf tools: Add kallsyms__get_function_start()

[tip-bot for Adrian Hunter]

Commit-ID:  29b596b57426831fce92cd0ebb01c77627616fdf
Gitweb:     http://git.kernel.org/tip/29b596b57426831fce92cd0ebb01c77627616fdf
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:37 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:47 -0300

perf tools: Add kallsyms__get_function_start()

Separate out the logic used to find the start address of the reference
symbol used to track kernel relocation.  kallsyms__get_function_start()
is used in subsequent patches.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-3-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c | 18 +++++++++++++++---
 tools/perf/util/event.h |  3 +++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fc1c2f..17476df 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -470,6 +470,17 @@ static int find_symbol_cb(void *arg, const char *name, char type,
 	return 1;
 }
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name)
+{
+	struct process_symbol_args args = { .name = symbol_name, };
+
+	if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
+		return 0;
+
+	return args.start;
+}
+
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
 				       struct machine *machine,
@@ -480,13 +491,13 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	char path[PATH_MAX];
 	char name_buff[PATH_MAX];
 	struct map *map;
+	u64 start;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
 	 * available use this, and after it is use this as a fallback for older
 	 * kernels.
 	 */
-	struct process_symbol_args args = { .name = symbol_name, };
 	union perf_event *event = zalloc((sizeof(event->mmap) +
 					  machine->id_hdr_size));
 	if (event == NULL) {
@@ -513,7 +524,8 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		}
 	}
 
-	if (kallsyms__parse(filename, &args, find_symbol_cb) <= 0) {
+	start = kallsyms__get_function_start(filename, symbol_name);
+	if (!start) {
 		free(event);
 		return -ENOENT;
 	}
@@ -525,7 +537,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = args.start;
+	event->mmap.pgoff = start;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index faf6e21..66a0c03 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -279,4 +279,7 @@ size_t perf_event__fprintf_mmap2(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_task(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf(union perf_event *event, FILE *fp);
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name);
+
 #endif /* __PERF_RECORD_H */

[PATCH V2 3/9] perf tools: Add machine__get_kallsyms_filename()

[Adrian Hunter]

Separate out the logic used to make the
kallsyms full path name for a machine.
It will be reused in a subsequent patch.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/machine.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ded7459..290c2e6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -496,19 +496,22 @@ static int symbol__in_kernel(void *arg, const char *name,
 	return 1;
 }
 
+static void machine__get_kallsyms_filename(struct machine *machine, char *buf,
+					   size_t bufsz)
+{
+	if (machine__is_default_guest(machine))
+		scnprintf(buf, bufsz, "%s", symbol_conf.default_guest_kallsyms);
+	else
+		scnprintf(buf, bufsz, "%s/proc/kallsyms", machine->root_dir);
+}
+
 /* Figure out the start address of kernel map from /proc/kallsyms */
 static u64 machine__get_kernel_start_addr(struct machine *machine)
 {
-	const char *filename;
-	char path[PATH_MAX];
+	char filename[PATH_MAX];
 	struct process_args args;
 
-	if (machine__is_default_guest(machine))
-		filename = (char *)symbol_conf.default_guest_kallsyms;
-	else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-		filename = path;
-	}
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
 
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return 0;
-- 
1.7.11.7

[tip:perf/urgent] perf machine: Add machine__get_kallsyms_filename()

[tip-bot for Adrian Hunter]

Commit-ID:  15a0a8706c32bd38bff9ebf7c6ef24f32d1ea921
Gitweb:     http://git.kernel.org/tip/15a0a8706c32bd38bff9ebf7c6ef24f32d1ea921
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:38 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:48 -0300

perf machine: Add machine__get_kallsyms_filename()

Separate out the logic used to make the kallsyms full path name for a
machine.  It will be reused in a subsequent patch.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-4-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ded7459..290c2e6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -496,19 +496,22 @@ static int symbol__in_kernel(void *arg, const char *name,
 	return 1;
 }
 
+static void machine__get_kallsyms_filename(struct machine *machine, char *buf,
+					   size_t bufsz)
+{
+	if (machine__is_default_guest(machine))
+		scnprintf(buf, bufsz, "%s", symbol_conf.default_guest_kallsyms);
+	else
+		scnprintf(buf, bufsz, "%s/proc/kallsyms", machine->root_dir);
+}
+
 /* Figure out the start address of kernel map from /proc/kallsyms */
 static u64 machine__get_kernel_start_addr(struct machine *machine)
 {
-	const char *filename;
-	char path[PATH_MAX];
+	char filename[PATH_MAX];
 	struct process_args args;
 
-	if (machine__is_default_guest(machine))
-		filename = (char *)symbol_conf.default_guest_kallsyms;
-	else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-		filename = path;
-	}
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
 
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return 0;

[PATCH V2 4/9] perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()

[Adrian Hunter]

The ref_reloc_sym is always needed for the
kernel map in order to check for relocation.
Consequently set it up when the kernel map is
created.  Otherwise it was only being set up
by 'perf record'.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/machine.c | 23 +++++++++++++++++++++++
 tools/perf/util/machine.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 290c2e6..c872991 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -832,9 +832,25 @@ static int machine__create_modules(struct machine *machine)
 	return 0;
 }
 
+const char *ref_reloc_sym_names[] = {"_text", "_stext", NULL};
+
 int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
+	char filename[PATH_MAX];
+	const char *name;
+	u64 addr = 0;
+	int i;
+
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
+
+	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+		addr = kallsyms__get_function_start(filename, name);
+		if (addr)
+			break;
+	}
+	if (!addr)
+		return -1;
 
 	if (kernel == NULL ||
 	    __machine__create_kernel_maps(machine, kernel) < 0)
@@ -853,6 +869,13 @@ int machine__create_kernel_maps(struct machine *machine)
 	 * Now that we have all the maps created, just set the ->end of them:
 	 */
 	map_groups__fixup_end(&machine->kmaps);
+
+	if (maps__set_kallsyms_ref_reloc_sym(machine->vmlinux_maps, name,
+					     addr)) {
+		machine__destroy_kernel_maps(machine);
+		return -1;
+	}
+
 	return 0;
 }
 
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 4771330..f77e91e 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -18,6 +18,8 @@ union perf_event;
 #define	HOST_KERNEL_ID			(-1)
 #define	DEFAULT_GUEST_KERNEL_ID		(0)
 
+extern const char *ref_reloc_sym_names[];
+
 struct machine {
 	struct rb_node	  rb_node;
 	pid_t		  pid;
-- 
1.7.11.7

[tip:perf/urgent] perf machine: Set up ref_reloc_sym in machine__create_kernel_maps()

[tip-bot for Adrian Hunter]

Commit-ID:  5512cf24bed2de56f1ef44b6cc9a0a9b15499cea
Gitweb:     http://git.kernel.org/tip/5512cf24bed2de56f1ef44b6cc9a0a9b15499cea
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:39 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:49 -0300

perf machine: Set up ref_reloc_sym in machine__create_kernel_maps()

The ref_reloc_sym is always needed for the kernel map in order to check
for relocation.  Consequently set it up when the kernel map is created.
Otherwise it was only being set up by 'perf record'.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-5-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 23 +++++++++++++++++++++++
 tools/perf/util/machine.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 290c2e6..c872991 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -832,9 +832,25 @@ static int machine__create_modules(struct machine *machine)
 	return 0;
 }
 
+const char *ref_reloc_sym_names[] = {"_text", "_stext", NULL};
+
 int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
+	char filename[PATH_MAX];
+	const char *name;
+	u64 addr = 0;
+	int i;
+
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
+
+	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+		addr = kallsyms__get_function_start(filename, name);
+		if (addr)
+			break;
+	}
+	if (!addr)
+		return -1;
 
 	if (kernel == NULL ||
 	    __machine__create_kernel_maps(machine, kernel) < 0)
@@ -853,6 +869,13 @@ int machine__create_kernel_maps(struct machine *machine)
 	 * Now that we have all the maps created, just set the ->end of them:
 	 */
 	map_groups__fixup_end(&machine->kmaps);
+
+	if (maps__set_kallsyms_ref_reloc_sym(machine->vmlinux_maps, name,
+					     addr)) {
+		machine__destroy_kernel_maps(machine);
+		return -1;
+	}
+
 	return 0;
 }
 
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 4771330..f77e91e 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -18,6 +18,8 @@ union perf_event;
 #define	HOST_KERNEL_ID			(-1)
 #define	DEFAULT_GUEST_KERNEL_ID		(0)
 
+extern const char *ref_reloc_sym_names[];
+
 struct machine {
 	struct rb_node	  rb_node;
 	pid_t		  pid;

[PATCH V2 5/9] perf record: Get ref_reloc_sym from kernel map

[Adrian Hunter]

Now that ref_reloc_sym is set up when the kernel
map is created, 'perf record' does not need to
pass the symbol names to
perf_event__synthesize_kernel_mmap() which can
read the values needed from ref_reloc_sym directly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 10 ++--------
 tools/perf/util/event.c     | 26 ++++++--------------------
 tools/perf/util/event.h     |  3 +--
 3 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3c394bf..af47531 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -287,10 +287,7 @@ static void perf_event__synthesize_guest_os(struct machine *machine, void *data)
 	 * have no _text sometimes.
 	 */
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record guest kernel [%d]'s reference"
 		       " relocation symbol.\n", machine->pid);
@@ -457,10 +454,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record kernel reference relocation symbol\n"
 		       "Symbol resolution may be skewed if relocation was used (e.g. kexec).\n"
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17476df..b0f3ca8 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -483,15 +483,13 @@ u64 kallsyms__get_function_start(const char *kallsyms_filename,
 
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name)
+				       struct machine *machine)
 {
 	size_t size;
-	const char *filename, *mmap_name;
-	char path[PATH_MAX];
+	const char *mmap_name;
 	char name_buff[PATH_MAX];
 	struct map *map;
-	u64 start;
+	struct kmap *kmap;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
@@ -513,31 +511,19 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		 * see kernel/perf_event.c __perf_event_mmap
 		 */
 		event->header.misc = PERF_RECORD_MISC_KERNEL;
-		filename = "/proc/kallsyms";
 	} else {
 		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
-		if (machine__is_default_guest(machine))
-			filename = (char *) symbol_conf.default_guest_kallsyms;
-		else {
-			sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-			filename = path;
-		}
-	}
-
-	start = kallsyms__get_function_start(filename, symbol_name);
-	if (!start) {
-		free(event);
-		return -ENOENT;
 	}
 
 	map = machine->vmlinux_maps[MAP__FUNCTION];
+	kmap = map__kmap(map);
 	size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
-			"%s%s", mmap_name, symbol_name) + 1;
+			"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
 	size = PERF_ALIGN(size, sizeof(u64));
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = start;
+	event->mmap.pgoff = kmap->ref_reloc_sym->addr;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 66a0c03..851fa06 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -214,8 +214,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
 				   struct machine *machine, bool mmap_data);
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name);
+				       struct machine *machine);
 
 int perf_event__synthesize_modules(struct perf_tool *tool,
 				   perf_event__handler_t process,
-- 
1.7.11.7

[tip:perf/urgent] perf record: Get ref_reloc_sym from kernel map

[tip-bot for Adrian Hunter]

Commit-ID:  0ae617bedde062003fd70e566e9a2601e273ea0e
Gitweb:     http://git.kernel.org/tip/0ae617bedde062003fd70e566e9a2601e273ea0e
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:40 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:50 -0300

perf record: Get ref_reloc_sym from kernel map

Now that ref_reloc_sym is set up when the kernel map is created,
'perf record' does not need to pass the symbol names to
perf_event__synthesize_kernel_mmap() which can read the values needed
from ref_reloc_sym directly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-6-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 10 ++--------
 tools/perf/util/event.c     | 26 ++++++--------------------
 tools/perf/util/event.h     |  3 +--
 3 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3c394bf..af47531 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -287,10 +287,7 @@ static void perf_event__synthesize_guest_os(struct machine *machine, void *data)
 	 * have no _text sometimes.
 	 */
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record guest kernel [%d]'s reference"
 		       " relocation symbol.\n", machine->pid);
@@ -457,10 +454,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record kernel reference relocation symbol\n"
 		       "Symbol resolution may be skewed if relocation was used (e.g. kexec).\n"
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17476df..b0f3ca8 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -483,15 +483,13 @@ u64 kallsyms__get_function_start(const char *kallsyms_filename,
 
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name)
+				       struct machine *machine)
 {
 	size_t size;
-	const char *filename, *mmap_name;
-	char path[PATH_MAX];
+	const char *mmap_name;
 	char name_buff[PATH_MAX];
 	struct map *map;
-	u64 start;
+	struct kmap *kmap;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
@@ -513,31 +511,19 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		 * see kernel/perf_event.c __perf_event_mmap
 		 */
 		event->header.misc = PERF_RECORD_MISC_KERNEL;
-		filename = "/proc/kallsyms";
 	} else {
 		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
-		if (machine__is_default_guest(machine))
-			filename = (char *) symbol_conf.default_guest_kallsyms;
-		else {
-			sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-			filename = path;
-		}
-	}
-
-	start = kallsyms__get_function_start(filename, symbol_name);
-	if (!start) {
-		free(event);
-		return -ENOENT;
 	}
 
 	map = machine->vmlinux_maps[MAP__FUNCTION];
+	kmap = map__kmap(map);
 	size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
-			"%s%s", mmap_name, symbol_name) + 1;
+			"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
 	size = PERF_ALIGN(size, sizeof(u64));
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = start;
+	event->mmap.pgoff = kmap->ref_reloc_sym->addr;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 66a0c03..851fa06 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -214,8 +214,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
 				   struct machine *machine, bool mmap_data);
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name);
+				       struct machine *machine);
 
 int perf_event__synthesize_modules(struct perf_tool *tool,
 				   perf_event__handler_t process,

[PATCH V2 6/9] perf tools: Prevent the use of kcore if the kernel has moved

[Adrian Hunter]

Use of kcore is predicated upon it matching
the recorded data.  If the kernel has been
relocated at boot time (i.e. since the data
was recorded) then do not use kcore.

Note that it is possible to make a copy
of kcore at the time the data is recorded
using 'perf buildid-cache'.  Then perf tools
will use the copy because it does match the
data.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 39ce9ad..4ac1f87 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -976,6 +976,23 @@ static int validate_kcore_modules(const char *kallsyms_filename,
 	return 0;
 }
 
+static int validate_kcore_addresses(const char *kallsyms_filename,
+				    struct map *map)
+{
+	struct kmap *kmap = map__kmap(map);
+
+	if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) {
+		u64 start;
+
+		start = kallsyms__get_function_start(kallsyms_filename,
+						     kmap->ref_reloc_sym->name);
+		if (start != kmap->ref_reloc_sym->addr)
+			return -EINVAL;
+	}
+
+	return validate_kcore_modules(kallsyms_filename, map);
+}
+
 struct kcore_mapfn_data {
 	struct dso *dso;
 	enum map_type type;
@@ -1019,8 +1036,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 					     kallsyms_filename))
 		return -EINVAL;
 
-	/* All modules must be present at their original addresses */
-	if (validate_kcore_modules(kallsyms_filename, map))
+	/* Modules and kernel must be present at their original addresses */
+	if (validate_kcore_addresses(kallsyms_filename, map))
 		return -EINVAL;
 
 	md.dso = dso;
@@ -1424,7 +1441,7 @@ static int find_matching_kcore(struct map *map, char *dir, size_t dir_sz)
 			continue;
 		scnprintf(kallsyms_filename, sizeof(kallsyms_filename),
 			  "%s/%s/kallsyms", dir, dent->d_name);
-		if (!validate_kcore_modules(kallsyms_filename, map)) {
+		if (!validate_kcore_addresses(kallsyms_filename, map)) {
 			strlcpy(dir, kallsyms_filename, dir_sz);
 			ret = 0;
 			break;
@@ -1479,7 +1496,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
 		if (fd != -1) {
 			close(fd);
 			/* If module maps match go with /proc/kallsyms */
-			if (!validate_kcore_modules("/proc/kallsyms", map))
+			if (!validate_kcore_addresses("/proc/kallsyms", map))
 				goto proc_kallsyms;
 		}
 
-- 
1.7.11.7

[tip:perf/urgent] perf symbols: Prevent the use of kcore if the kernel has moved

[tip-bot for Adrian Hunter]

Commit-ID:  a00d28cb72d3629c6481fe21ba6c6b4f96caed49
Gitweb:     http://git.kernel.org/tip/a00d28cb72d3629c6481fe21ba6c6b4f96caed49
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:41 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:51 -0300

perf symbols: Prevent the use of kcore if the kernel has moved

Use of kcore is predicated upon it matching the recorded data.  If the
kernel has been relocated at boot time (i.e. since the data was
recorded) then do not use kcore.

Note that it is possible to make a copy of kcore at the time the data is
recorded using 'perf buildid-cache'.  Then the perf tools will use the
copy because it does match the data.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-7-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 39ce9ad..4ac1f87 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -976,6 +976,23 @@ static int validate_kcore_modules(const char *kallsyms_filename,
 	return 0;
 }
 
+static int validate_kcore_addresses(const char *kallsyms_filename,
+				    struct map *map)
+{
+	struct kmap *kmap = map__kmap(map);
+
+	if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) {
+		u64 start;
+
+		start = kallsyms__get_function_start(kallsyms_filename,
+						     kmap->ref_reloc_sym->name);
+		if (start != kmap->ref_reloc_sym->addr)
+			return -EINVAL;
+	}
+
+	return validate_kcore_modules(kallsyms_filename, map);
+}
+
 struct kcore_mapfn_data {
 	struct dso *dso;
 	enum map_type type;
@@ -1019,8 +1036,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 					     kallsyms_filename))
 		return -EINVAL;
 
-	/* All modules must be present at their original addresses */
-	if (validate_kcore_modules(kallsyms_filename, map))
+	/* Modules and kernel must be present at their original addresses */
+	if (validate_kcore_addresses(kallsyms_filename, map))
 		return -EINVAL;
 
 	md.dso = dso;
@@ -1424,7 +1441,7 @@ static int find_matching_kcore(struct map *map, char *dir, size_t dir_sz)
 			continue;
 		scnprintf(kallsyms_filename, sizeof(kallsyms_filename),
 			  "%s/%s/kallsyms", dir, dent->d_name);
-		if (!validate_kcore_modules(kallsyms_filename, map)) {
+		if (!validate_kcore_addresses(kallsyms_filename, map)) {
 			strlcpy(dir, kallsyms_filename, dir_sz);
 			ret = 0;
 			break;
@@ -1479,7 +1496,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
 		if (fd != -1) {
 			close(fd);
 			/* If module maps match go with /proc/kallsyms */
-			if (!validate_kcore_modules("/proc/kallsyms", map))
+			if (!validate_kcore_addresses("/proc/kallsyms", map))
 				goto proc_kallsyms;
 		}
 

[PATCH V2 7/9] perf tools: Test does not need to set up ref_reloc_sym

[Adrian Hunter]

Now that ref_reloc_sym is set up by
machine__create_kernel_maps(), the
"vmlinux symtab matches kallsyms" test
does have to do it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/vmlinux-kallsyms.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 2bd13ed..3d90880 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -26,7 +26,6 @@ int test__vmlinux_matches_kallsyms(void)
 	struct map *kallsyms_map, *vmlinux_map;
 	struct machine kallsyms, vmlinux;
 	enum map_type type = MAP__FUNCTION;
-	struct ref_reloc_sym ref_reloc_sym = { .name = "_stext", };
 	u64 mem_start, mem_end;
 
 	/*
@@ -70,14 +69,6 @@ int test__vmlinux_matches_kallsyms(void)
 	 */
 	kallsyms_map = machine__kernel_map(&kallsyms, type);
 
-	sym = map__find_symbol_by_name(kallsyms_map, ref_reloc_sym.name, NULL);
-	if (sym == NULL) {
-		pr_debug("dso__find_symbol_by_name ");
-		goto out;
-	}
-
-	ref_reloc_sym.addr = UM(sym->start);
-
 	/*
 	 * Step 5:
 	 *
@@ -89,7 +80,6 @@ int test__vmlinux_matches_kallsyms(void)
 	}
 
 	vmlinux_map = machine__kernel_map(&vmlinux, type);
-	map__kmap(vmlinux_map)->ref_reloc_sym = &ref_reloc_sym;
 
 	/*
 	 * Step 6:
-- 
1.7.11.7

[tip:perf/urgent] perf tests: No need to set up ref_reloc_sym

[tip-bot for Adrian Hunter]

Commit-ID:  c080f72753def150993144d755379941f8b14683
Gitweb:     http://git.kernel.org/tip/c080f72753def150993144d755379941f8b14683
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:42 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:52 -0300

perf tests: No need to set up ref_reloc_sym

Now that ref_reloc_sym is set up by machine__create_kernel_maps(), the
"vmlinux symtab matches kallsyms" test does have to do it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-8-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/vmlinux-kallsyms.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 2bd13ed..3d90880 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -26,7 +26,6 @@ int test__vmlinux_matches_kallsyms(void)
 	struct map *kallsyms_map, *vmlinux_map;
 	struct machine kallsyms, vmlinux;
 	enum map_type type = MAP__FUNCTION;
-	struct ref_reloc_sym ref_reloc_sym = { .name = "_stext", };
 	u64 mem_start, mem_end;
 
 	/*
@@ -70,14 +69,6 @@ int test__vmlinux_matches_kallsyms(void)
 	 */
 	kallsyms_map = machine__kernel_map(&kallsyms, type);
 
-	sym = map__find_symbol_by_name(kallsyms_map, ref_reloc_sym.name, NULL);
-	if (sym == NULL) {
-		pr_debug("dso__find_symbol_by_name ");
-		goto out;
-	}
-
-	ref_reloc_sym.addr = UM(sym->start);
-
 	/*
 	 * Step 5:
 	 *
@@ -89,7 +80,6 @@ int test__vmlinux_matches_kallsyms(void)
 	}
 
 	vmlinux_map = machine__kernel_map(&vmlinux, type);
-	map__kmap(vmlinux_map)->ref_reloc_sym = &ref_reloc_sym;
 
 	/*
 	 * Step 6:

[PATCH V2 8/9] perf tools: Adjust kallsyms for relocated kernel

[Adrian Hunter]

If the kernel is relocated at boot time, kallsyms
will not match data recorded previously.  That
does not matter for modules because they are
corrected anyway.  It also does not matter if
vmlinux is being used for symbols. But if perf
tools has only kallsyms then the symbols will not
match.  Fix by applying the delta gained by
comparing the old and current addresses of the
relocation reference symbol.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4ac1f87..a9d758a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
  * kernel range is broken in several maps, named [kernel].N, as we don't have
  * the original ELF section names vmlinux have.
  */
-static int dso__split_kallsyms(struct dso *dso, struct map *map,
+static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
 			       symbol_filter_t filter)
 {
 	struct map_groups *kmaps = map__kmap(map)->kmaps;
@@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			char dso_name[PATH_MAX];
 			struct dso *ndso;
 
+			if (delta) {
+				/* Kernel was relocated at boot time */
+				pos->start -= delta;
+				pos->end -= delta;
+			}
+
 			if (count == 0) {
 				curr_map = map;
 				goto filter_symbol;
@@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
 			map_groups__insert(kmaps, curr_map);
 			++kernel_range;
+		} else if (delta) {
+			/* Kernel was relocated at boot time */
+			pos->start -= delta;
+			pos->end -= delta;
 		}
 filter_symbol:
 		if (filter && filter(curr_map, pos)) {
@@ -1130,15 +1140,41 @@ out_err:
 	return -EINVAL;
 }
 
+/*
+ * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
+ * delta based on the relocation reference symbol.
+ */
+static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
+{
+	struct kmap *kmap = map__kmap(map);
+	u64 addr;
+
+	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
+		return 0;
+
+	addr = kallsyms__get_function_start(filename,
+					    kmap->ref_reloc_sym->name);
+	if (!addr)
+		return -1;
+
+	*delta = addr - kmap->ref_reloc_sym->addr;
+	return 0;
+}
+
 int dso__load_kallsyms(struct dso *dso, const char *filename,
 		       struct map *map, symbol_filter_t filter)
 {
+	u64 delta = 0;
+
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return -1;
 
 	if (dso__load_all_kallsyms(dso, filename, map) < 0)
 		return -1;
 
+	if (kallsyms__delta(map, filename, &delta))
+		return -1;
+
 	symbols__fixup_duplicate(&dso->symbols[map->type]);
 	symbols__fixup_end(&dso->symbols[map->type]);
 
@@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
 	if (!dso__load_kcore(dso, map, filename))
 		return dso__split_kallsyms_for_kcore(dso, map, filter);
 	else
-		return dso__split_kallsyms(dso, map, filter);
+		return dso__split_kallsyms(dso, map, delta, filter);
 }
 
 static int dso__load_perf_map(struct dso *dso, struct map *map,
-- 
1.7.11.7

Re: [PATCH V2 8/9] perf tools: Adjust kallsyms for relocated kernel

[Arnaldo Carvalho de Melo]

Em Wed, Jan 29, 2014 at 04:14:43PM +0200, Adrian Hunter escreveu:
> If the kernel is relocated at boot time, kallsyms
> will not match data recorded previously.  That
> does not matter for modules because they are
> corrected anyway.  It also does not matter if
> vmlinux is being used for symbols. But if perf
> tools has only kallsyms then the symbols will not
> match.  Fix by applying the delta gained by
> comparing the old and current addresses of the
> relocation reference symbol.

Don't we have map->reloc? Can't we use it here, i.e. be consistent and
have the unrelocated address + the relocation, and then, when using
map->{map,unmap} take that into account, as you did to the objdump
variants, no?

I applied the others, testing now.

- Arnaldo
 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 4ac1f87..a9d758a 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
>   * kernel range is broken in several maps, named [kernel].N, as we don't have
>   * the original ELF section names vmlinux have.
>   */
> -static int dso__split_kallsyms(struct dso *dso, struct map *map,
> +static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
>  			       symbol_filter_t filter)
>  {
>  	struct map_groups *kmaps = map__kmap(map)->kmaps;
> @@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
>  			char dso_name[PATH_MAX];
>  			struct dso *ndso;
>  
> +			if (delta) {
> +				/* Kernel was relocated at boot time */
> +				pos->start -= delta;
> +				pos->end -= delta;
> +			}
> +
>  			if (count == 0) {
>  				curr_map = map;
>  				goto filter_symbol;
> @@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
>  			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
>  			map_groups__insert(kmaps, curr_map);
>  			++kernel_range;
> +		} else if (delta) {
> +			/* Kernel was relocated at boot time */
> +			pos->start -= delta;
> +			pos->end -= delta;
>  		}
>  filter_symbol:
>  		if (filter && filter(curr_map, pos)) {
> @@ -1130,15 +1140,41 @@ out_err:
>  	return -EINVAL;
>  }
>  
> +/*
> + * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
> + * delta based on the relocation reference symbol.
> + */
> +static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
> +{
> +	struct kmap *kmap = map__kmap(map);
> +	u64 addr;
> +
> +	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
> +		return 0;
> +
> +	addr = kallsyms__get_function_start(filename,
> +					    kmap->ref_reloc_sym->name);
> +	if (!addr)
> +		return -1;
> +
> +	*delta = addr - kmap->ref_reloc_sym->addr;
> +	return 0;
> +}
> +
>  int dso__load_kallsyms(struct dso *dso, const char *filename,
>  		       struct map *map, symbol_filter_t filter)
>  {
> +	u64 delta = 0;
> +
>  	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
>  		return -1;
>  
>  	if (dso__load_all_kallsyms(dso, filename, map) < 0)
>  		return -1;
>  
> +	if (kallsyms__delta(map, filename, &delta))
> +		return -1;
> +
>  	symbols__fixup_duplicate(&dso->symbols[map->type]);
>  	symbols__fixup_end(&dso->symbols[map->type]);
>  
> @@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
>  	if (!dso__load_kcore(dso, map, filename))
>  		return dso__split_kallsyms_for_kcore(dso, map, filter);
>  	else
> -		return dso__split_kallsyms(dso, map, filter);
> +		return dso__split_kallsyms(dso, map, delta, filter);
>  }
>  
>  static int dso__load_perf_map(struct dso *dso, struct map *map,
> -- 
> 1.7.11.7

Re: [PATCH V2 8/9] perf tools: Adjust kallsyms for relocated kernel

[Adrian Hunter]

On 29/01/14 21:08, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 29, 2014 at 04:14:43PM +0200, Adrian Hunter escreveu:
>> If the kernel is relocated at boot time, kallsyms
>> will not match data recorded previously.  That
>> does not matter for modules because they are
>> corrected anyway.  It also does not matter if
>> vmlinux is being used for symbols. But if perf
>> tools has only kallsyms then the symbols will not
>> match.  Fix by applying the delta gained by
>> comparing the old and current addresses of the
>> relocation reference symbol.
> 
> Don't we have map->reloc? Can't we use it here, i.e. be consistent and
> have the unrelocated address + the relocation, and then, when using
> map->{map,unmap} take that into account, as you did to the objdump
> variants, no?

map->reloc is calculated from vmlinux.  No vmlinux, no map->reloc.
e.g. we don't need to calculate objdump addresses if we have no
object code anyway.  Also without vmlinux we do not know what the
actual relocation was, we just know the difference between where
the kernel was located when the data was recorded and where it
is now.  That is the "delta".

This is the case where all we have is kallsyms.  In that case
perf tools uses identity maps (identity__map_ip).  An alternative
would be to change to use map__[un]map_ip() and set map->pgoff to
match the current kallsyms. i.e. map->pgoff = map->start + delta

So we need either to change the symbols to match the mapping or
change the mapping to match the symbols.  I went with the former
because we are already changing the module symbols so it seemed
consistent.

> 
> I applied the others, testing now.
> 
> - Arnaldo
>  
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 4ac1f87..a9d758a 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
>>   * kernel range is broken in several maps, named [kernel].N, as we don't have
>>   * the original ELF section names vmlinux have.
>>   */
>> -static int dso__split_kallsyms(struct dso *dso, struct map *map,
>> +static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
>>  			       symbol_filter_t filter)
>>  {
>>  	struct map_groups *kmaps = map__kmap(map)->kmaps;
>> @@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
>>  			char dso_name[PATH_MAX];
>>  			struct dso *ndso;
>>  
>> +			if (delta) {
>> +				/* Kernel was relocated at boot time */
>> +				pos->start -= delta;
>> +				pos->end -= delta;
>> +			}
>> +
>>  			if (count == 0) {
>>  				curr_map = map;
>>  				goto filter_symbol;
>> @@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
>>  			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
>>  			map_groups__insert(kmaps, curr_map);
>>  			++kernel_range;
>> +		} else if (delta) {
>> +			/* Kernel was relocated at boot time */
>> +			pos->start -= delta;
>> +			pos->end -= delta;
>>  		}
>>  filter_symbol:
>>  		if (filter && filter(curr_map, pos)) {
>> @@ -1130,15 +1140,41 @@ out_err:
>>  	return -EINVAL;
>>  }
>>  
>> +/*
>> + * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
>> + * delta based on the relocation reference symbol.
>> + */
>> +static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
>> +{
>> +	struct kmap *kmap = map__kmap(map);
>> +	u64 addr;
>> +
>> +	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
>> +		return 0;
>> +
>> +	addr = kallsyms__get_function_start(filename,
>> +					    kmap->ref_reloc_sym->name);
>> +	if (!addr)
>> +		return -1;
>> +
>> +	*delta = addr - kmap->ref_reloc_sym->addr;
>> +	return 0;
>> +}
>> +
>>  int dso__load_kallsyms(struct dso *dso, const char *filename,
>>  		       struct map *map, symbol_filter_t filter)
>>  {
>> +	u64 delta = 0;
>> +
>>  	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
>>  		return -1;
>>  
>>  	if (dso__load_all_kallsyms(dso, filename, map) < 0)
>>  		return -1;
>>  
>> +	if (kallsyms__delta(map, filename, &delta))
>> +		return -1;
>> +
>>  	symbols__fixup_duplicate(&dso->symbols[map->type]);
>>  	symbols__fixup_end(&dso->symbols[map->type]);
>>  
>> @@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
>>  	if (!dso__load_kcore(dso, map, filename))
>>  		return dso__split_kallsyms_for_kcore(dso, map, filter);
>>  	else
>> -		return dso__split_kallsyms(dso, map, filter);
>> +		return dso__split_kallsyms(dso, map, delta, filter);
>>  }
>>  
>>  static int dso__load_perf_map(struct dso *dso, struct map *map,
>> -- 
>> 1.7.11.7
> 
> 

Re: [PATCH V2 8/9] perf tools: Adjust kallsyms for relocated kernel

[Arnaldo Carvalho de Melo]

Summary: I'm applying this patch as it is reported to fix problems, but
answering anyway to outline my reasoning, that I eventually should put
in place to simplify the reloc handling:

Em Thu, Jan 30, 2014 at 10:10:50AM +0200, Adrian Hunter escreveu:
> On 29/01/14 21:08, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jan 29, 2014 at 04:14:43PM +0200, Adrian Hunter escreveu:
> >> If the kernel is relocated at boot time, kallsyms
> >> will not match data recorded previously.  That
> >> does not matter for modules because they are
> >> corrected anyway.  It also does not matter if
> >> vmlinux is being used for symbols. But if perf
> >> tools has only kallsyms then the symbols will not
> >> match.  Fix by applying the delta gained by
> >> comparing the old and current addresses of the
> >> relocation reference symbol.
> > 
> > Don't we have map->reloc? Can't we use it here, i.e. be consistent and
> > have the unrelocated address + the relocation, and then, when using
> > map->{map,unmap} take that into account, as you did to the objdump
> > variants, no?
> 
> map->reloc is calculated from vmlinux.  No vmlinux, no map->reloc.

I see that this is how it is being used now, yes.

> e.g. we don't need to calculate objdump addresses if we have no
> object code anyway.

But map->reloc doesn't need to be used just for objdump purposes.

> Also without vmlinux we do not know what the actual relocation was, we
> just know the difference between where the kernel was located when the
> data was recorded and where it is now.  That is the "delta".

A relocation is a delta as well, no? I'm just trying to figure out why
can't we avoid adding this 'delta' thing and instead use map->reloc for
that.
 
> This is the case where all we have is kallsyms.  In that case
> perf tools uses identity maps (identity__map_ip).  An alternative
> would be to change to use map__[un]map_ip() and set map->pgoff to
> match the current kallsyms. i.e. map->pgoff = map->start + delta

Why not use map->reloc for that? When we don't find any delta, then it
is zero and all is as today, when we find the delta, be it from vmlinux
or from a difference in the ref_reloc_symbol, we set it up, no?
 
> So we need either to change the symbols to match the mapping or
> change the mapping to match the symbols.  I went with the former
> because we are already changing the module symbols so it seemed
> consistent.

Well, for modules what we do is apply a relocation in place, like you do
with this delta, no? I.e. what I'm saying is that it should work both
ways, but we should try to be consistent accross the board, i.e. fully
use map->reloc and stop changing the address we get from the original
symtab.

Anyway, that can be done in a followup patch as your patch has been
reported to fix real bugs, thanks.

- Arnaldo
 
> > 
> > I applied the others, testing now.
> > 
> > - Arnaldo
> >  
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
> >>  1 file changed, 38 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> >> index 4ac1f87..a9d758a 100644
> >> --- a/tools/perf/util/symbol.c
> >> +++ b/tools/perf/util/symbol.c
> >> @@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
> >>   * kernel range is broken in several maps, named [kernel].N, as we don't have
> >>   * the original ELF section names vmlinux have.
> >>   */
> >> -static int dso__split_kallsyms(struct dso *dso, struct map *map,
> >> +static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
> >>  			       symbol_filter_t filter)
> >>  {
> >>  	struct map_groups *kmaps = map__kmap(map)->kmaps;
> >> @@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
> >>  			char dso_name[PATH_MAX];
> >>  			struct dso *ndso;
> >>  
> >> +			if (delta) {
> >> +				/* Kernel was relocated at boot time */
> >> +				pos->start -= delta;
> >> +				pos->end -= delta;
> >> +			}
> >> +
> >>  			if (count == 0) {
> >>  				curr_map = map;
> >>  				goto filter_symbol;
> >> @@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
> >>  			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
> >>  			map_groups__insert(kmaps, curr_map);
> >>  			++kernel_range;
> >> +		} else if (delta) {
> >> +			/* Kernel was relocated at boot time */
> >> +			pos->start -= delta;
> >> +			pos->end -= delta;
> >>  		}
> >>  filter_symbol:
> >>  		if (filter && filter(curr_map, pos)) {
> >> @@ -1130,15 +1140,41 @@ out_err:
> >>  	return -EINVAL;
> >>  }
> >>  
> >> +/*
> >> + * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
> >> + * delta based on the relocation reference symbol.
> >> + */
> >> +static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
> >> +{
> >> +	struct kmap *kmap = map__kmap(map);
> >> +	u64 addr;
> >> +
> >> +	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
> >> +		return 0;
> >> +
> >> +	addr = kallsyms__get_function_start(filename,
> >> +					    kmap->ref_reloc_sym->name);
> >> +	if (!addr)
> >> +		return -1;
> >> +
> >> +	*delta = addr - kmap->ref_reloc_sym->addr;
> >> +	return 0;
> >> +}
> >> +
> >>  int dso__load_kallsyms(struct dso *dso, const char *filename,
> >>  		       struct map *map, symbol_filter_t filter)
> >>  {
> >> +	u64 delta = 0;
> >> +
> >>  	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
> >>  		return -1;
> >>  
> >>  	if (dso__load_all_kallsyms(dso, filename, map) < 0)
> >>  		return -1;
> >>  
> >> +	if (kallsyms__delta(map, filename, &delta))
> >> +		return -1;
> >> +
> >>  	symbols__fixup_duplicate(&dso->symbols[map->type]);
> >>  	symbols__fixup_end(&dso->symbols[map->type]);
> >>  
> >> @@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
> >>  	if (!dso__load_kcore(dso, map, filename))
> >>  		return dso__split_kallsyms_for_kcore(dso, map, filter);
> >>  	else
> >> -		return dso__split_kallsyms(dso, map, filter);
> >> +		return dso__split_kallsyms(dso, map, delta, filter);
> >>  }
> >>  
> >>  static int dso__load_perf_map(struct dso *dso, struct map *map,
> >> -- 
> >> 1.7.11.7
> > 
> > 

[tip:perf/urgent] perf tools: Adjust kallsyms for relocated kernel

[tip-bot for Adrian Hunter]

Commit-ID:  d9b62aba87a82939c73f451a166c7a21342350d6
Gitweb:     http://git.kernel.org/tip/d9b62aba87a82939c73f451a166c7a21342350d6
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:43 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:53 -0300

perf tools: Adjust kallsyms for relocated kernel

If the kernel is relocated at boot time, kallsyms will not match data
recorded previously.

That does not matter for modules because they are corrected anyway.  It
also does not matter if vmlinux is being used for symbols. But if perf
tools has only kallsyms then the symbols will not match.

Fix by applying the delta gained by comparing the old and current
addresses of the relocation reference symbol.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-9-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4ac1f87..a9d758a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
  * kernel range is broken in several maps, named [kernel].N, as we don't have
  * the original ELF section names vmlinux have.
  */
-static int dso__split_kallsyms(struct dso *dso, struct map *map,
+static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
 			       symbol_filter_t filter)
 {
 	struct map_groups *kmaps = map__kmap(map)->kmaps;
@@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			char dso_name[PATH_MAX];
 			struct dso *ndso;
 
+			if (delta) {
+				/* Kernel was relocated at boot time */
+				pos->start -= delta;
+				pos->end -= delta;
+			}
+
 			if (count == 0) {
 				curr_map = map;
 				goto filter_symbol;
@@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
 			map_groups__insert(kmaps, curr_map);
 			++kernel_range;
+		} else if (delta) {
+			/* Kernel was relocated at boot time */
+			pos->start -= delta;
+			pos->end -= delta;
 		}
 filter_symbol:
 		if (filter && filter(curr_map, pos)) {
@@ -1130,15 +1140,41 @@ out_err:
 	return -EINVAL;
 }
 
+/*
+ * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
+ * delta based on the relocation reference symbol.
+ */
+static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
+{
+	struct kmap *kmap = map__kmap(map);
+	u64 addr;
+
+	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
+		return 0;
+
+	addr = kallsyms__get_function_start(filename,
+					    kmap->ref_reloc_sym->name);
+	if (!addr)
+		return -1;
+
+	*delta = addr - kmap->ref_reloc_sym->addr;
+	return 0;
+}
+
 int dso__load_kallsyms(struct dso *dso, const char *filename,
 		       struct map *map, symbol_filter_t filter)
 {
+	u64 delta = 0;
+
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return -1;
 
 	if (dso__load_all_kallsyms(dso, filename, map) < 0)
 		return -1;
 
+	if (kallsyms__delta(map, filename, &delta))
+		return -1;
+
 	symbols__fixup_duplicate(&dso->symbols[map->type]);
 	symbols__fixup_end(&dso->symbols[map->type]);
 
@@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
 	if (!dso__load_kcore(dso, map, filename))
 		return dso__split_kallsyms_for_kcore(dso, map, filter);
 	else
-		return dso__split_kallsyms(dso, map, filter);
+		return dso__split_kallsyms(dso, map, delta, filter);
 }
 
 static int dso__load_perf_map(struct dso *dso, struct map *map,

[PATCH V2 9/9] perf buildid-cache: Check relocation when checking for existing kcore

[Adrian Hunter]

perf buildid-cache does not make another
copy of kcore if the buildid and modules
match an existing copy.  That does not
take into account the possibility that
the kernel has been relocated.  Extend
the check to check if the reference
relocation symbol matches too, otherwise
do make a copy.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-buildid-cache.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index cfede86..b22dbb1 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -63,11 +63,35 @@ static int build_id_cache__kcore_dir(char *dir, size_t sz)
 	return 0;
 }
 
+static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
+{
+	char from[PATH_MAX];
+	char to[PATH_MAX];
+	const char *name;
+	u64 addr1 = 0, addr2 = 0;
+	int i;
+
+	scnprintf(from, sizeof(from), "%s/kallsyms", from_dir);
+	scnprintf(to, sizeof(to), "%s/kallsyms", to_dir);
+
+	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+		addr1 = kallsyms__get_function_start(from, name);
+		if (addr1)
+			break;
+	}
+
+	if (name)
+		addr2 = kallsyms__get_function_start(to, name);
+
+	return addr1 == addr2;
+}
+
 static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 					  size_t to_dir_sz)
 {
 	char from[PATH_MAX];
 	char to[PATH_MAX];
+	char to_subdir[PATH_MAX];
 	struct dirent *dent;
 	int ret = -1;
 	DIR *d;
@@ -86,10 +110,11 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 			continue;
 		scnprintf(to, sizeof(to), "%s/%s/modules", to_dir,
 			  dent->d_name);
-		if (!compare_proc_modules(from, to)) {
-			scnprintf(to, sizeof(to), "%s/%s", to_dir,
-				  dent->d_name);
-			strlcpy(to_dir, to, to_dir_sz);
+		scnprintf(to_subdir, sizeof(to_subdir), "%s/%s",
+			  to_dir, dent->d_name);
+		if (!compare_proc_modules(from, to) &&
+		    same_kallsyms_reloc(from_dir, to_subdir)) {
+			strlcpy(to_dir, to_subdir, to_dir_sz);
 			ret = 0;
 			break;
 		}
-- 
1.7.11.7

Re: [PATCH V2 9/9] perf buildid-cache: Check relocation when checking for existing kcore

[Arnaldo Carvalho de Melo]

Em Wed, Jan 29, 2014 at 04:14:44PM +0200, Adrian Hunter escreveu:
> perf buildid-cache does not make another
> copy of kcore if the buildid and modules
> match an existing copy.  That does not

Humm, what is the problem? Having the ref reloc symbol we can fix things
up, no? I.e. just using map->reloc the old kcore copy should be ok to
use, no need to replace the kcore copy in the cache. Or am I missing
something?

- Arnaldo

> take into account the possibility that
> the kernel has been relocated.  Extend
> the check to check if the reference
> relocation symbol matches too, otherwise
> do make a copy.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/builtin-buildid-cache.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index cfede86..b22dbb1 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -63,11 +63,35 @@ static int build_id_cache__kcore_dir(char *dir, size_t sz)
>  	return 0;
>  }
>  
> +static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
> +{
> +	char from[PATH_MAX];
> +	char to[PATH_MAX];
> +	const char *name;
> +	u64 addr1 = 0, addr2 = 0;
> +	int i;
> +
> +	scnprintf(from, sizeof(from), "%s/kallsyms", from_dir);
> +	scnprintf(to, sizeof(to), "%s/kallsyms", to_dir);
> +
> +	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
> +		addr1 = kallsyms__get_function_start(from, name);
> +		if (addr1)
> +			break;
> +	}
> +
> +	if (name)
> +		addr2 = kallsyms__get_function_start(to, name);
> +
> +	return addr1 == addr2;
> +}
> +
>  static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
>  					  size_t to_dir_sz)
>  {
>  	char from[PATH_MAX];
>  	char to[PATH_MAX];
> +	char to_subdir[PATH_MAX];
>  	struct dirent *dent;
>  	int ret = -1;
>  	DIR *d;
> @@ -86,10 +110,11 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
>  			continue;
>  		scnprintf(to, sizeof(to), "%s/%s/modules", to_dir,
>  			  dent->d_name);
> -		if (!compare_proc_modules(from, to)) {
> -			scnprintf(to, sizeof(to), "%s/%s", to_dir,
> -				  dent->d_name);
> -			strlcpy(to_dir, to, to_dir_sz);
> +		scnprintf(to_subdir, sizeof(to_subdir), "%s/%s",
> +			  to_dir, dent->d_name);
> +		if (!compare_proc_modules(from, to) &&
> +		    same_kallsyms_reloc(from_dir, to_subdir)) {
> +			strlcpy(to_dir, to_subdir, to_dir_sz);
>  			ret = 0;
>  			break;
>  		}
> -- 
> 1.7.11.7

Re: [PATCH V2 9/9] perf buildid-cache: Check relocation when checking for existing kcore

[Adrian Hunter]

On 29/01/14 21:14, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jan 29, 2014 at 04:14:44PM +0200, Adrian Hunter escreveu:
>> perf buildid-cache does not make another
>> copy of kcore if the buildid and modules
>> match an existing copy.  That does not
> 
> Humm, what is the problem? Having the ref reloc symbol we can fix things
> up, no? I.e. just using map->reloc the old kcore copy should be ok to
> use, no need to replace the kcore copy in the cache. Or am I missing
> something?

The current implementation works on the basis that kcore
matches the perf data recorded.  This is just a fix for that.

I am afraid it is that way because it meets my needs.
I did not think of allowing for relocation because I
need to be able to walk the code.  Relocation was one
of the things I was trying to avoid.

For me making a copy of kcore is far superior because
it can be made to have the jump labels mostly the
right way around too. e.g. run a dummy perf record
while making the copy.

> 
> - Arnaldo
> 
>> take into account the possibility that
>> the kernel has been relocated.  Extend
>> the check to check if the reference
>> relocation symbol matches too, otherwise
>> do make a copy.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/builtin-buildid-cache.c | 33 +++++++++++++++++++++++++++++----
>>  1 file changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
>> index cfede86..b22dbb1 100644
>> --- a/tools/perf/builtin-buildid-cache.c
>> +++ b/tools/perf/builtin-buildid-cache.c
>> @@ -63,11 +63,35 @@ static int build_id_cache__kcore_dir(char *dir, size_t sz)
>>  	return 0;
>>  }
>>  
>> +static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
>> +{
>> +	char from[PATH_MAX];
>> +	char to[PATH_MAX];
>> +	const char *name;
>> +	u64 addr1 = 0, addr2 = 0;
>> +	int i;
>> +
>> +	scnprintf(from, sizeof(from), "%s/kallsyms", from_dir);
>> +	scnprintf(to, sizeof(to), "%s/kallsyms", to_dir);
>> +
>> +	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
>> +		addr1 = kallsyms__get_function_start(from, name);
>> +		if (addr1)
>> +			break;
>> +	}
>> +
>> +	if (name)
>> +		addr2 = kallsyms__get_function_start(to, name);
>> +
>> +	return addr1 == addr2;
>> +}
>> +
>>  static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
>>  					  size_t to_dir_sz)
>>  {
>>  	char from[PATH_MAX];
>>  	char to[PATH_MAX];
>> +	char to_subdir[PATH_MAX];
>>  	struct dirent *dent;
>>  	int ret = -1;
>>  	DIR *d;
>> @@ -86,10 +110,11 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
>>  			continue;
>>  		scnprintf(to, sizeof(to), "%s/%s/modules", to_dir,
>>  			  dent->d_name);
>> -		if (!compare_proc_modules(from, to)) {
>> -			scnprintf(to, sizeof(to), "%s/%s", to_dir,
>> -				  dent->d_name);
>> -			strlcpy(to_dir, to, to_dir_sz);
>> +		scnprintf(to_subdir, sizeof(to_subdir), "%s/%s",
>> +			  to_dir, dent->d_name);
>> +		if (!compare_proc_modules(from, to) &&
>> +		    same_kallsyms_reloc(from_dir, to_subdir)) {
>> +			strlcpy(to_dir, to_subdir, to_dir_sz);
>>  			ret = 0;
>>  			break;
>>  		}
>> -- 
>> 1.7.11.7
> 
> 

Re: [PATCH V2 9/9] perf buildid-cache: Check relocation when checking for existing kcore

[Arnaldo Carvalho de Melo]

Em Thu, Jan 30, 2014 at 11:34:38AM +0200, Adrian Hunter escreveu:
> On 29/01/14 21:14, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Jan 29, 2014 at 04:14:44PM +0200, Adrian Hunter escreveu:
> >> perf buildid-cache does not make another
> >> copy of kcore if the buildid and modules
> >> match an existing copy.  That does not

> > Humm, what is the problem? Having the ref reloc symbol we can fix things
> > up, no? I.e. just using map->reloc the old kcore copy should be ok to
> > use, no need to replace the kcore copy in the cache. Or am I missing
> > something?

> The current implementation works on the basis that kcore matches the
> perf data recorded.  This is just a fix for that.

> I am afraid it is that way because it meets my needs.

> I did not think of allowing for relocation because I need to be able
> to walk the code.  Relocation was one of the things I was trying to
> avoid.
 
> For me making a copy of kcore is far superior because it can be made
> to have the jump labels mostly the right way around too. e.g. run a
> dummy perf record while making the copy.

Yes, it is superior, no question about it, I'm just trying to figure out
how this fits into the build-id cache thing, i.e. it should have files
keyed by its build-id, that are inserted, but not replaced, since it
expects its contents to be constant.

So you have a need to get the matching kcore at the time you did the
record, because we're dealing with self modifying code, the kernel (soon
if not already, userspace as well)...

So at least we need to make this abundantly clear to users, that what is
in the build-id cache may be the latest snapshot of some DSO that had a
build-id at, well, build time.

We need to add support for looking up in the binary where are places
that are modifiable and then mark those in the UI using some visual cue.

Till then, at least a paragraph in the documentation stating what
happens is needed, I'll look into it.

And then right now this is just for kcore, that is clearly marked as
such in the build-id cache, IIRC it is in a separate directory, etc,
right?

- Arnaldo

Re: [PATCH V2 9/9] perf buildid-cache: Check relocation when checking for existing kcore

[Adrian Hunter]

On 30/01/2014 4:18 p.m., Arnaldo Carvalho de Melo wrote:
> Em Thu, Jan 30, 2014 at 11:34:38AM +0200, Adrian Hunter escreveu:
>> On 29/01/14 21:14, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Jan 29, 2014 at 04:14:44PM +0200, Adrian Hunter escreveu:
>>>> perf buildid-cache does not make another
>>>> copy of kcore if the buildid and modules
>>>> match an existing copy.  That does not
>
>>> Humm, what is the problem? Having the ref reloc symbol we can fix things
>>> up, no? I.e. just using map->reloc the old kcore copy should be ok to
>>> use, no need to replace the kcore copy in the cache. Or am I missing
>>> something?
>
>> The current implementation works on the basis that kcore matches the
>> perf data recorded.  This is just a fix for that.
>
>> I am afraid it is that way because it meets my needs.
>
>> I did not think of allowing for relocation because I need to be able
>> to walk the code.  Relocation was one of the things I was trying to
>> avoid.
>
>> For me making a copy of kcore is far superior because it can be made
>> to have the jump labels mostly the right way around too. e.g. run a
>> dummy perf record while making the copy.
>
> Yes, it is superior, no question about it, I'm just trying to figure out
> how this fits into the build-id cache thing, i.e. it should have files
> keyed by its build-id, that are inserted, but not replaced, since it
> expects its contents to be constant.
>
> So you have a need to get the matching kcore at the time you did the
> record, because we're dealing with self modifying code, the kernel (soon
> if not already, userspace as well)...
>
> So at least we need to make this abundantly clear to users, that what is
> in the build-id cache may be the latest snapshot of some DSO that had a
> build-id at, well, build time.
>
> We need to add support for looking up in the binary where are places
> that are modifiable and then mark those in the UI using some visual cue.
>
> Till then, at least a paragraph in the documentation stating what
> happens is needed, I'll look into it.
>
> And then right now this is just for kcore, that is clearly marked as
> such in the build-id cache, IIRC it is in a separate directory, etc,
> right?

Yes, perf buidid-cache creates a directory in the cache of the form:

	[kernel.kcore]/<build-id>/<YYYYmmddHHMMSShh>

which contains 3 files: kcore, kallsyms and modules.

[tip:perf/urgent] perf buildid-cache: Check relocation when checking for existing kcore

[tip-bot for Adrian Hunter]

Commit-ID:  d3b70220292c40d3b499797fd2f33f608fc35edb
Gitweb:     http://git.kernel.org/tip/d3b70220292c40d3b499797fd2f33f608fc35edb
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 29 Jan 2014 16:14:44 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 31 Jan 2014 17:21:54 -0300

perf buildid-cache: Check relocation when checking for existing kcore

perf buildid-cache does not make another copy of kcore if the buildid
and modules match an existing copy.

That does not take into account the possibility that the kernel has been
relocated.

Extend the check to check if the reference relocation symbol matches
too, otherwise do make a copy.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1391004884-10334-10-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-buildid-cache.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index cfede86..b22dbb1 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -63,11 +63,35 @@ static int build_id_cache__kcore_dir(char *dir, size_t sz)
 	return 0;
 }
 
+static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
+{
+	char from[PATH_MAX];
+	char to[PATH_MAX];
+	const char *name;
+	u64 addr1 = 0, addr2 = 0;
+	int i;
+
+	scnprintf(from, sizeof(from), "%s/kallsyms", from_dir);
+	scnprintf(to, sizeof(to), "%s/kallsyms", to_dir);
+
+	for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+		addr1 = kallsyms__get_function_start(from, name);
+		if (addr1)
+			break;
+	}
+
+	if (name)
+		addr2 = kallsyms__get_function_start(to, name);
+
+	return addr1 == addr2;
+}
+
 static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 					  size_t to_dir_sz)
 {
 	char from[PATH_MAX];
 	char to[PATH_MAX];
+	char to_subdir[PATH_MAX];
 	struct dirent *dent;
 	int ret = -1;
 	DIR *d;
@@ -86,10 +110,11 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 			continue;
 		scnprintf(to, sizeof(to), "%s/%s/modules", to_dir,
 			  dent->d_name);
-		if (!compare_proc_modules(from, to)) {
-			scnprintf(to, sizeof(to), "%s/%s", to_dir,
-				  dent->d_name);
-			strlcpy(to_dir, to, to_dir_sz);
+		scnprintf(to_subdir, sizeof(to_subdir), "%s/%s",
+			  to_dir, dent->d_name);
+		if (!compare_proc_modules(from, to) &&
+		    same_kallsyms_reloc(from_dir, to_subdir)) {
+			strlcpy(to_dir, to_subdir, to_dir_sz);
 			ret = 0;
 			break;
 		}