From: Ming Lei <[email protected]>
On ARM, module addresss space is ahead of kernel space,
so the module symbols are handled before kernel symbol
in dso__split_kallsyms, then cause one map is created
for each kernel symbol.
This patch fixes the issue by restoring to original kernel
map in dso__split_kallsyms() to avoid create unnecessary maps
for kernel symbols when starting to handle kenel symbol maps
but after module symbol maps are handled over.
Cc: Ian Munsie <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Tom Zanussi <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
---
tools/perf/util/symbol.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0500895..c5b4ccb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -520,6 +520,53 @@ static int dso__load_all_kallsyms(struct dso *self, const char *filename,
return kallsyms__parse(filename, &args, map__process_kallsym_symbol);
}
+static int map_for_module(struct map *map, struct machine *self)
+{
+ char *line = NULL;
+ FILE *file;
+ const char *modules;
+ char name[PATH_MAX];
+ int ret = 0;
+
+ if (machine__is_default_guest(self))
+ modules = symbol_conf.default_guest_modules;
+ else {
+ sprintf(name, "%s/proc/modules", self->root_dir);
+ modules = name;
+ }
+
+ file = fopen(modules, "r");
+ if (file == NULL)
+ return ret;
+
+ /*strip [] of map->dso->short_name*/
+ strncpy(name, map->dso->short_name + 1,
+ strlen(map->dso->short_name) - 2);
+ name[strlen(map->dso->short_name) - 3] = '\0';
+
+ while (!feof(file)) {
+ int line_len;
+ size_t n;
+
+ line_len = getline(&line, &n, file);
+ if (line_len < 0)
+ break;
+
+ if (!line)
+ break;
+
+ line[line_len-1] = '\0'; /* \n */
+
+ if (strstr(line, name)) {
+ ret = 1;
+ break;
+ }
+ }
+
+ free(line);
+ fclose(file);
+ return ret;
+}
/*
* Split the symbols into maps, making sure there are no overlaps, i.e. the
* kernel range is broken in several maps, named [kernel].N, as we don't have
@@ -590,6 +637,16 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
char dso_name[PATH_MAX];
struct dso *dso;
+ /*
+ * If module symbol is ahead of kernel symbol like ARM,
+ * restore to original kernel map to avoid create many
+ * unnecessary maps for kernel symbols.
+ */
+ if (map_for_module(curr_map, machine)) {
+ curr_map = map;
+ goto operate_map;
+ }
+
if (self->kernel == DSO_TYPE_GUEST_KERNEL)
snprintf(dso_name, sizeof(dso_name),
"[guest.kernel].%d",
@@ -616,6 +673,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
++kernel_range;
}
+operate_map:
if (filter && filter(curr_map, pos)) {
discard_symbol: rb_erase(&pos->rb_node, root);
symbol__delete(pos);
--
1.7.3
Em Wed, Nov 24, 2010 at 07:35:02PM +0800, [email protected] escreveu:
> From: Ming Lei <[email protected]>
> On ARM, module addresss space is ahead of kernel space, so the module
> symbols are handled before kernel symbol in dso__split_kallsyms, then
> cause one map is created for each kernel symbol.
> This patch fixes the issue by restoring to original kernel map in
> dso__split_kallsyms() to avoid create unnecessary maps for kernel
> symbols when starting to handle kenel symbol maps but after module
> symbol maps are handled over.
Can you try with the following patch instead?
- Arnaldo
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b39f499..a7518d2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -530,7 +530,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
struct machine *machine = kmaps->machine;
struct map *curr_map = map;
struct symbol *pos;
- int count = 0;
+ int count = 0, moved = 0;
struct rb_root *root = &self->symbols[map->type];
struct rb_node *next = rb_first(root);
int kernel_range = 0;
@@ -588,6 +588,11 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
char dso_name[PATH_MAX];
struct dso *dso;
+ if (count == 0) {
+ curr_map = map;
+ goto filter_symbol;
+ }
+
if (self->kernel == DSO_TYPE_GUEST_KERNEL)
snprintf(dso_name, sizeof(dso_name),
"[guest.kernel].%d",
@@ -613,7 +618,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
map_groups__insert(kmaps, curr_map);
++kernel_range;
}
-
+filter_symbol:
if (filter && filter(curr_map, pos)) {
discard_symbol: rb_erase(&pos->rb_node, root);
symbol__delete(pos);
@@ -621,8 +626,9 @@ discard_symbol: rb_erase(&pos->rb_node, root);
if (curr_map != map) {
rb_erase(&pos->rb_node, root);
symbols__insert(&curr_map->dso->symbols[curr_map->type], pos);
- }
- count++;
+ ++moved;
+ } else
+ ++count;
}
}
@@ -632,7 +638,7 @@ discard_symbol: rb_erase(&pos->rb_node, root);
dso__set_loaded(curr_map->dso, curr_map->type);
}
- return count;
+ return count + moved;
}
int dso__load_kallsyms(struct dso *self, const char *filename,
2010/11/24 Arnaldo Carvalho de Melo <[email protected]>:
> Em Wed, Nov 24, 2010 at 07:35:02PM +0800, [email protected] escreveu:
>> From: Ming Lei <[email protected]>
>
>> On ARM, module addresss space is ahead of kernel space, so the module
>> symbols are handled before kernel symbol in dso__split_kallsyms, then
>> cause one map is created for each kernel symbol.
>
>> This patch fixes the issue by restoring to original kernel map in
>> dso__split_kallsyms() to avoid create unnecessary maps for kernel
>> symbols when starting to handle kenel symbol maps but after module
>> symbol maps are handled over.
>
> Can you try with the following patch instead?
Fine to me.
Reported-and-tested-by: Ming Lei <[email protected]>
>
> - Arnaldo
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index b39f499..a7518d2 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -530,7 +530,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
> ? ? ? ?struct machine *machine = kmaps->machine;
> ? ? ? ?struct map *curr_map = map;
> ? ? ? ?struct symbol *pos;
> - ? ? ? int count = 0;
> + ? ? ? int count = 0, moved = 0;
> ? ? ? ?struct rb_root *root = &self->symbols[map->type];
> ? ? ? ?struct rb_node *next = rb_first(root);
> ? ? ? ?int kernel_range = 0;
> @@ -588,6 +588,11 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
> ? ? ? ? ? ? ? ? ? ? ? ?char dso_name[PATH_MAX];
> ? ? ? ? ? ? ? ? ? ? ? ?struct dso *dso;
>
> + ? ? ? ? ? ? ? ? ? ? ? if (count == 0) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? curr_map = map;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? goto filter_symbol;
> + ? ? ? ? ? ? ? ? ? ? ? }
> +
> ? ? ? ? ? ? ? ? ? ? ? ?if (self->kernel == DSO_TYPE_GUEST_KERNEL)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?snprintf(dso_name, sizeof(dso_name),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"[guest.kernel].%d",
> @@ -613,7 +618,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
> ? ? ? ? ? ? ? ? ? ? ? ?map_groups__insert(kmaps, curr_map);
> ? ? ? ? ? ? ? ? ? ? ? ?++kernel_range;
> ? ? ? ? ? ? ? ?}
> -
> +filter_symbol:
> ? ? ? ? ? ? ? ?if (filter && filter(curr_map, pos)) {
> ?discard_symbol: ? ? ? ? ? ? ? ?rb_erase(&pos->rb_node, root);
> ? ? ? ? ? ? ? ? ? ? ? ?symbol__delete(pos);
> @@ -621,8 +626,9 @@ discard_symbol: ? ? ? ? ? ? rb_erase(&pos->rb_node, root);
> ? ? ? ? ? ? ? ? ? ? ? ?if (curr_map != map) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rb_erase(&pos->rb_node, root);
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?symbols__insert(&curr_map->dso->symbols[curr_map->type], pos);
> - ? ? ? ? ? ? ? ? ? ? ? }
> - ? ? ? ? ? ? ? ? ? ? ? count++;
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ++moved;
> + ? ? ? ? ? ? ? ? ? ? ? } else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ++count;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> @@ -632,7 +638,7 @@ discard_symbol: ? ? ? ? ? ? rb_erase(&pos->rb_node, root);
> ? ? ? ? ? ? ? ?dso__set_loaded(curr_map->dso, curr_map->type);
> ? ? ? ?}
>
> - ? ? ? return count;
> + ? ? ? return count + moved;
> ?}
>
> ?int dso__load_kallsyms(struct dso *self, const char *filename,
>
thanks,
--
Lei Ming
2010/11/25 Ming Lei <[email protected]>:
> 2010/11/24 Arnaldo Carvalho de Melo <[email protected]>:
>> Em Wed, Nov 24, 2010 at 07:35:02PM +0800, [email protected] escreveu:
>>> From: Ming Lei <[email protected]>
>>
>>> On ARM, module addresss space is ahead of kernel space, so the module
>>> symbols are handled before kernel symbol in dso__split_kallsyms, then
>>> cause one map is created for each kernel symbol.
>>
>>> This patch fixes the issue by restoring to original kernel map in
>>> dso__split_kallsyms() to avoid create unnecessary maps for kernel
>>> symbols when starting to handle kenel symbol maps but after module
>>> symbol maps are handled over.
>>
>> Can you try with the following patch instead?
>
> Fine to me.
>
> Reported-and-tested-by: Ming Lei <[email protected]>
Arnaldo, could you queue this one and the patch below
http://marc.info/?l=linux-kernel&m=129068448714210&w=2
into your tree? Without the two, perf tool can't work well on ARM
if there are modules loaded.
thanks,
--
Lei Ming
Commit-ID: 8a9533123f43f2cdb3eb601c17ff2ad336882eff
Gitweb: http://git.kernel.org/tip/8a9533123f43f2cdb3eb601c17ff2ad336882eff
Author: Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Mon, 29 Nov 2010 12:44:15 -0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 30 Nov 2010 14:47:51 -0200
perf symbols: Fix kallsyms kernel/module map splitting
On ARM, module addresss space is ahead of kernel space, so the module
symbols are handled before kernel symbol in dso__split_kallsyms, then
was causing one map to be created for each kernel symbol.
Reported-by: Ming Lei <[email protected]>
Tested-by: Ming Lei <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Ming Lei <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/symbol.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0500895..2af4d7d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -532,7 +532,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
struct machine *machine = kmaps->machine;
struct map *curr_map = map;
struct symbol *pos;
- int count = 0;
+ int count = 0, moved = 0;
struct rb_root *root = &self->symbols[map->type];
struct rb_node *next = rb_first(root);
int kernel_range = 0;
@@ -590,6 +590,11 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
char dso_name[PATH_MAX];
struct dso *dso;
+ if (count == 0) {
+ curr_map = map;
+ goto filter_symbol;
+ }
+
if (self->kernel == DSO_TYPE_GUEST_KERNEL)
snprintf(dso_name, sizeof(dso_name),
"[guest.kernel].%d",
@@ -615,7 +620,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map,
map_groups__insert(kmaps, curr_map);
++kernel_range;
}
-
+filter_symbol:
if (filter && filter(curr_map, pos)) {
discard_symbol: rb_erase(&pos->rb_node, root);
symbol__delete(pos);
@@ -623,8 +628,9 @@ discard_symbol: rb_erase(&pos->rb_node, root);
if (curr_map != map) {
rb_erase(&pos->rb_node, root);
symbols__insert(&curr_map->dso->symbols[curr_map->type], pos);
- }
- count++;
+ ++moved;
+ } else
+ ++count;
}
}
@@ -634,7 +640,7 @@ discard_symbol: rb_erase(&pos->rb_node, root);
dso__set_loaded(curr_map->dso, curr_map->type);
}
- return count;
+ return count + moved;
}
int dso__load_kallsyms(struct dso *self, const char *filename,