Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1446354 > unrolled thread
| Started by | Song Shan Gong <gongss@linux.vnet.ibm.com> |
|---|---|
| First post | 2016-07-19 13:40 +0200 |
| Last post | 2016-07-20 14:00 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[RFC PATCH V3]s390/perf:fix 'start' address of module's map Song Shan Gong <gongss@linux.vnet.ibm.com> - 2016-07-19 13:40 +0200
[PATCH] s390/perf: fix 'start' address of module's map Song Shan Gong <gongss@linux.vnet.ibm.com> - 2016-07-19 13:40 +0200
Re: [PATCH] s390/perf: fix 'start' address of module's map Jiri Olsa <jolsa@redhat.com> - 2016-07-20 14:00 +0200
| From | Song Shan Gong <gongss@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-07-19 13:40 +0200 |
| Subject | [RFC PATCH V3]s390/perf:fix 'start' address of module's map |
| Message-ID | <rWBxw-3r9-15@gated-at.bofh.it> |
Change log: From V2: 1. remove redundancy string 'module_name'; From V1: 1.change func name from 'fix__arch_module_baseaddr' to 'fix__arch_module_text_start'; 2.Parse '.text' start addr from 'sys' through 'sysfs__read_ull', not 'hex2u64()'; 2.Perfect code: check return value and allocated pointer by 'strdup'. Song Shan Gong (1): s390/perf: fix 'start' address of module's map tools/perf/arch/s390/util/Build | 2 ++ tools/perf/arch/s390/util/sym-handling.c | 27 +++++++++++++++++++++++++++ tools/perf/util/machine.c | 8 ++++++++ tools/perf/util/machine.h | 1 + 4 files changed, 38 insertions(+) create mode 100644 tools/perf/arch/s390/util/sym-handling.c -- 2.3.0
[toc] | [next] | [standalone]
| From | Song Shan Gong <gongss@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-07-19 13:40 +0200 |
| Subject | [PATCH] s390/perf: fix 'start' address of module's map |
| Message-ID | <rWBxw-3r9-25@gated-at.bofh.it> |
| In reply to | #1446354 |
At preset, when creating module's map, perf gets 'start' address by parsing
'/proc/modules', but it's module base address, isn't the start address of
'.text' section. In most archs, it's OK. But for s390, it places 'GOT' and
'PLT' relocations before '.text' section. So there exists an offset between
module base address and '.text' section, which will incur wrong symbol
resolution for modules.
Fix this bug by getting 'start' address of module's map from parsing
'/sys/module/[module name]/sections/.text', not from '/proc/modules'.
Signed-off-by: Song Shan Gong <gongss@linux.vnet.ibm.com>
---
tools/perf/arch/s390/util/Build | 2 ++
tools/perf/arch/s390/util/sym-handling.c | 19 +++++++++++++++++++
tools/perf/util/machine.c | 8 ++++++++
tools/perf/util/machine.h | 1 +
4 files changed, 30 insertions(+)
create mode 100644 tools/perf/arch/s390/util/sym-handling.c
diff --git a/tools/perf/arch/s390/util/Build b/tools/perf/arch/s390/util/Build
index 8a61372..5e322ed 100644
--- a/tools/perf/arch/s390/util/Build
+++ b/tools/perf/arch/s390/util/Build
@@ -2,3 +2,5 @@ libperf-y += header.o
libperf-y += kvm-stat.o
libperf-$(CONFIG_DWARF) += dwarf-regs.o
+
+libperf-y += sym-handling.o
diff --git a/tools/perf/arch/s390/util/sym-handling.c b/tools/perf/arch/s390/util/sym-handling.c
new file mode 100644
index 0000000..ebfc55d
--- /dev/null
+++ b/tools/perf/arch/s390/util/sym-handling.c
@@ -0,0 +1,19 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include "util.h"
+#include "machine.h"
+#include "api/fs/fs.h"
+
+int arch__fix_module_text_start(u64 *start, const char *name)
+{
+ char path[PATH_MAX];
+
+ snprintf(path, PATH_MAX, "module/%.*s/sections/.text",
+ (int)strlen(name) - 2, name + 1);
+
+ if (sysfs__read_ull(path, (unsigned long long *)start) < 0)
+ return -1;
+
+ return 0;
+}
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b177218..97cc9f0 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1091,12 +1091,20 @@ static int machine__set_modules_path(struct machine *machine)
return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
}
+int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
+ const char *name __maybe_unused)
+{
+ return 0;
+}
static int machine__create_module(void *arg, const char *name, u64 start)
{
struct machine *machine = arg;
struct map *map;
+ if (arch__fix_module_text_start(&start, name) < 0)
+ return -1;
+
map = machine__findnew_module_map(machine, start, name);
if (map == NULL)
return -1;
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 41ac9cf..20739f7 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -216,6 +216,7 @@ struct symbol *machine__find_kernel_function_by_name(struct machine *machine,
struct map *machine__findnew_module_map(struct machine *machine, u64 start,
const char *filename);
+int arch__fix_module_text_start(u64 *start, const char *name);
int __machine__load_kallsyms(struct machine *machine, const char *filename,
enum map_type type, bool no_kcore, symbol_filter_t filter);
--
2.3.0
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2016-07-20 14:00 +0200 |
| Subject | Re: [PATCH] s390/perf: fix 'start' address of module's map |
| Message-ID | <rWYkq-Vx-19@gated-at.bofh.it> |
| In reply to | #1446358 |
On Tue, Jul 19, 2016 at 07:31:58PM +0800, Song Shan Gong wrote:
> At preset, when creating module's map, perf gets 'start' address by parsing
> '/proc/modules', but it's module base address, isn't the start address of
> '.text' section. In most archs, it's OK. But for s390, it places 'GOT' and
> 'PLT' relocations before '.text' section. So there exists an offset between
> module base address and '.text' section, which will incur wrong symbol
> resolution for modules.
>
> Fix this bug by getting 'start' address of module's map from parsing
> '/sys/module/[module name]/sections/.text', not from '/proc/modules'.
>
> Signed-off-by: Song Shan Gong <gongss@linux.vnet.ibm.com>
> ---
> tools/perf/arch/s390/util/Build | 2 ++
> tools/perf/arch/s390/util/sym-handling.c | 19 +++++++++++++++++++
> tools/perf/util/machine.c | 8 ++++++++
> tools/perf/util/machine.h | 1 +
> 4 files changed, 30 insertions(+)
> create mode 100644 tools/perf/arch/s390/util/sym-handling.c
>
> diff --git a/tools/perf/arch/s390/util/Build b/tools/perf/arch/s390/util/Build
> index 8a61372..5e322ed 100644
> --- a/tools/perf/arch/s390/util/Build
> +++ b/tools/perf/arch/s390/util/Build
> @@ -2,3 +2,5 @@ libperf-y += header.o
> libperf-y += kvm-stat.o
>
> libperf-$(CONFIG_DWARF) += dwarf-regs.o
> +
> +libperf-y += sym-handling.o
> diff --git a/tools/perf/arch/s390/util/sym-handling.c b/tools/perf/arch/s390/util/sym-handling.c
> new file mode 100644
> index 0000000..ebfc55d
> --- /dev/null
> +++ b/tools/perf/arch/s390/util/sym-handling.c
I wonder we should rather put this into arch/s390/util/machine.c
other than that:
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
> @@ -0,0 +1,19 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include "util.h"
> +#include "machine.h"
> +#include "api/fs/fs.h"
> +
> +int arch__fix_module_text_start(u64 *start, const char *name)
> +{
> + char path[PATH_MAX];
> +
> + snprintf(path, PATH_MAX, "module/%.*s/sections/.text",
> + (int)strlen(name) - 2, name + 1);
> +
> + if (sysfs__read_ull(path, (unsigned long long *)start) < 0)
> + return -1;
> +
> + return 0;
> +}
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index b177218..97cc9f0 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1091,12 +1091,20 @@ static int machine__set_modules_path(struct machine *machine)
>
> return map_groups__set_modules_path_dir(&machine->kmaps, modules_path, 0);
> }
> +int __weak arch__fix_module_text_start(u64 *start __maybe_unused,
> + const char *name __maybe_unused)
> +{
> + return 0;
> +}
>
> static int machine__create_module(void *arg, const char *name, u64 start)
> {
> struct machine *machine = arg;
> struct map *map;
>
> + if (arch__fix_module_text_start(&start, name) < 0)
> + return -1;
> +
> map = machine__findnew_module_map(machine, start, name);
> if (map == NULL)
> return -1;
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 41ac9cf..20739f7 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -216,6 +216,7 @@ struct symbol *machine__find_kernel_function_by_name(struct machine *machine,
>
> struct map *machine__findnew_module_map(struct machine *machine, u64 start,
> const char *filename);
> +int arch__fix_module_text_start(u64 *start, const char *name);
>
> int __machine__load_kallsyms(struct machine *machine, const char *filename,
> enum map_type type, bool no_kcore, symbol_filter_t filter);
> --
> 2.3.0
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web