[PATCH 2/2] perf:tools: figure out start address of kernel map from /proc/kallsyms

[PATCH 2/2] perf:tools: figure out start address of kernel map from /proc/kallsyms

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

On ARM, module symbol start address is ahead of kernel symbol start
address, so we can't suppose that the start address of kenerl map
always is zero, otherwise may cause incorrect .start and .end of kernel map
(caused by fixup) when there are modules loaded, then map_groups__find
may return incorrect map for symbol query.

This patch always figures out the start address of kernel map from
/proc/kallsyms if the file is available, so fix the issues on ARM for
module loaded case.

This patch fixes the following issues on ARM when modules are loaded:

	- vmlinux symbol can't be found by kallsyms maps doing 'perf test'
	- module symbols are parsed mistakenlly when doing 'perf top'/'perf report'

Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Zanussi <tzanussi@gmail.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 tools/perf/util/symbol.c |   60 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index c5b4ccb..1a8895c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2183,14 +2183,72 @@ static struct dso *machine__create_kernel(struct machine *self)
 	return kernel;
 }
 
+/*figure out the start address of kernel map from /proc/kallsyms*/
+static u64 __machine_get_kernel_start_addr(struct machine *self)
+{
+	char *line = NULL;
+	size_t n;
+	u64 start_addr = 0;
+	FILE *file;
+	const char *filename;
+	char path[PATH_MAX];
+
+	if (machine__is_default_guest(self)) {
+		filename = symbol_conf.default_guest_kallsyms;
+		if (!filename)
+			goto out_failure;
+	} else {
+		sprintf(path, "%s/proc/kallsyms", self->root_dir);
+		filename = path;
+	}
+
+	file = fopen(filename, "r");
+
+	if (file == NULL)
+		goto out_failure;
+
+	while (!feof(file)) {
+		u64 start = 0;
+		int line_len, len;
+
+		line_len = getline(&line, &n, file);
+		if (line_len < 0 || !line)
+			break;
+
+		line[--line_len] = '\0'; /* \n */
+
+		/*if the line is for module symbol, skip it*/
+		if (strchr(line, '['))
+			continue;
+
+		len = hex2u64(line, &start);
+
+		len++;
+		if (len + 2 >= line_len)
+			continue;
+
+		start_addr = start;
+		break;
+	}
+
+	free(line);
+	fclose(file);
+	return start_addr;
+
+out_failure:
+	return start_addr;
+}
+
 int __machine__create_kernel_maps(struct machine *self, struct dso *kernel)
 {
 	enum map_type type;
+	u64 start;
 
+	start = __machine_get_kernel_start_addr(self);
 	for (type = 0; type < MAP__NR_TYPES; ++type) {
 		struct kmap *kmap;
 
-		self->vmlinux_maps[type] = map__new2(0, kernel, type);
+		self->vmlinux_maps[type] = map__new2(start, kernel, type);
 		if (self->vmlinux_maps[type] == NULL)
 			return -1;
 
-- 
1.7.3

Re: [PATCH 2/2] perf:tools: figure out start address of kernel map from /proc/kallsyms

[Arnaldo Carvalho de Melo]

Em Wed, Nov 24, 2010 at 07:35:33PM +0800, tom.leiming@gmail.com escreveu:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> On ARM, module symbol start address is ahead of kernel symbol start
> address, so we can't suppose that the start address of kenerl map
> always is zero, otherwise may cause incorrect .start and .end of kernel map
> (caused by fixup) when there are modules loaded, then map_groups__find
> may return incorrect map for symbol query.
> 
> This patch always figures out the start address of kernel map from
> /proc/kallsyms if the file is available, so fix the issues on ARM for
> module loaded case.
> 
> This patch fixes the following issues on ARM when modules are loaded:
> 
> 	- vmlinux symbol can't be found by kallsyms maps doing 'perf test'
> 	- module symbols are parsed mistakenlly when doing 'perf top'/'perf report'
> 
> Cc: Ian Munsie <imunsie@au1.ibm.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Tom Zanussi <tzanussi@gmail.com>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> ---
>  tools/perf/util/symbol.c |   60 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 59 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index c5b4ccb..1a8895c 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2183,14 +2183,72 @@ static struct dso *machine__create_kernel(struct machine *self)
>  	return kernel;
>  }
>  
> +/*figure out the start address of kernel map from /proc/kallsyms*/
> +static u64 __machine_get_kernel_start_addr(struct machine *self)
> +{
> +	char *line = NULL;
> +	size_t n;
> +	u64 start_addr = 0;
> +	FILE *file;
> +	const char *filename;
> +	char path[PATH_MAX];
> +
> +	if (machine__is_default_guest(self)) {
> +		filename = symbol_conf.default_guest_kallsyms;
> +		if (!filename)
> +			goto out_failure;
> +	} else {
> +		sprintf(path, "%s/proc/kallsyms", self->root_dir);
> +		filename = path;
> +	}
> +
> +	file = fopen(filename, "r");

Can you please use kallsyms__parse()? Please take a look at
event__synthesize_kernel_mmap as it also uses it.

> +	if (file == NULL)
> +		goto out_failure;
> +
> +	while (!feof(file)) {
> +		u64 start = 0;
> +		int line_len, len;
> +
> +		line_len = getline(&line, &n, file);
> +		if (line_len < 0 || !line)
> +			break;
> +
> +		line[--line_len] = '\0'; /* \n */
> +
> +		/*if the line is for module symbol, skip it*/
> +		if (strchr(line, '['))
> +			continue;
> +
> +		len = hex2u64(line, &start);
> +
> +		len++;
> +		if (len + 2 >= line_len)
> +			continue;
> +
> +		start_addr = start;
> +		break;
> +	}
> +
> +	free(line);
> +	fclose(file);

Out failure could be here, avoiding one return case.

> +	return start_addr;
> +
> +out_failure:
> +	return start_addr;
> +}
> +
>  int __machine__create_kernel_maps(struct machine *self, struct dso *kernel)
>  {
>  	enum map_type type;
> +	u64 start;
>  
> +	start = __machine_get_kernel_start_addr(self);
>  	for (type = 0; type < MAP__NR_TYPES; ++type) {
>  		struct kmap *kmap;
>  
> -		self->vmlinux_maps[type] = map__new2(0, kernel, type);
> +		self->vmlinux_maps[type] = map__new2(start, kernel, type);
>  		if (self->vmlinux_maps[type] == NULL)
>  			return -1;
>  
> -- 
> 1.7.3

Re: [PATCH 2/2] perf:tools: figure out start address of kernel map from /proc/kallsyms

[Ming Lei]

2010/11/24 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
> Can you please use kallsyms__parse()? Please take a look at
> event__synthesize_kernel_mmap as it also uses it.

Good idea, but kallsyms__parse does not pass a parameter
which indicates if it is a module or kernel symbol to process handler.

So could you agree on adding one parameter which indicates if
the current symbol is module symbol to process handler?

If so, I can use kallsyms__parse.

thanks,
-- 
Lei Ming

Re: [PATCH 2/2] perf:tools: figure out start address of kernel map from /proc/kallsyms

[Arnaldo Carvalho de Melo]

Em Wed, Nov 24, 2010 at 10:38:29PM +0800, Ming Lei escreveu:
> 2010/11/24 Arnaldo Carvalho de Melo <acme@ghostprotocols.net>:
> > Can you please use kallsyms__parse()? Please take a look at
> > event__synthesize_kernel_mmap as it also uses it.
> 
> Good idea, but kallsyms__parse does not pass a parameter
> which indicates if it is a module or kernel symbol to process handler.
> 
> So could you agree on adding one parameter which indicates if
> the current symbol is module symbol to process handler?
> 
> If so, I can use kallsyms__parse.

It passes, for a line like:

ffffffffa00361ce t serio_raw_connect	[serio_raw]

The process callback will receive:

process_symbol(arg=callback arg,
               symbol_name="serio_raw_connect    [serio_raw]",
               symbol_type='T', start=0xffffffffa00361ce)

That is how we figure out if it is a module when splitting the kallsyms
into modules, as kallsyms__parse is also used in:

	dso__load_kallsyms
          dso__load_all_kallsyms
             kallsyms__parse

- Arnaldo