Hi,
During playing with the Stephane's memory access sampling series [1],
I needed to have these patches to make perf mem work properly.
It still gets a segfult when analyzing system wide sample data, and
needs more work on dealing with kernel's percpu symbols and rodata
symbols in user app, it worked well for my toy application at least
sometimes. ;-) I'll continue to chasing it down but before doing it
I'd like to share what I have now.
Thanks,
Namhyung
[1] https://lkml.org/lkml/2012/11/5/485
Namhyung Kim (6):
perf tools: Synthesize data mmap events for threads
perf tools: Set kernel data mapping length
perf tools: Fix detection of stack area
perf tools: Ignore ABS symbols when loading data maps
perf tools: Fix output of symbol_daddr offset
perf tools: Free {branch,mem}_info when freeing hist_entry
tools/perf/util/event.c | 78 ++++++++++++++++++++------------------------
tools/perf/util/hist.c | 2 ++
tools/perf/util/machine.c | 22 ++++++++-----
tools/perf/util/map.c | 2 +-
tools/perf/util/sort.c | 2 +-
tools/perf/util/symbol-elf.c | 3 ++
6 files changed, 55 insertions(+), 54 deletions(-)
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Current perf_event__synthesize_mmap_events() only deals with
executable mappings. With upcoming memory access sampling,
non-executable data mappings are needed also.
While at it, convert parsing code to use sscanf which makes
the code cleaner IMHO.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/event.c | 78 ++++++++++++++++++++++---------------------------
1 file changed, 35 insertions(+), 43 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index ca9ca285406a..068acf606b40 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -193,55 +193,47 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
event->header.misc = PERF_RECORD_MISC_USER;
while (1) {
- char bf[BUFSIZ], *pbf = bf;
- int n;
+ char bf[BUFSIZ];
+ char prot[5], *pprot = prot;
size_t size;
+ char exec_name[PATH_MAX], *pexec = exec_name;
+ char anonstr[] = "//anon";
+
if (fgets(bf, sizeof(bf), fp) == NULL)
break;
+ /* ensure null termination since stack will be reused */
+ strcpy(exec_name, "");
+
/* 00400000-0040c000 r-xp 00000000 fd:01 41038 /bin/cat */
- n = hex2u64(pbf, &event->mmap.start);
- if (n < 0)
- continue;
- pbf += n + 1;
- n = hex2u64(pbf, &event->mmap.len);
- if (n < 0)
+ sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
+ &event->mmap.start, &event->mmap.len, pprot,
+ &event->mmap.pgoff, pexec);
+
+ if (prot[2] == 'x' && !strcmp(exec_name, ""))
+ strcpy(exec_name, anonstr);
+
+ /* ignore non-executable anon mappings */
+ if (!strcmp(exec_name, ""))
continue;
- pbf += n + 3;
- if (*pbf == 'x') { /* vm_exec */
- char anonstr[] = "//anon\n";
- char *execname = strchr(bf, '/');
-
- /* Catch VDSO */
- if (execname == NULL)
- execname = strstr(bf, "[vdso]");
-
- /* Catch anonymous mmaps */
- if ((execname == NULL) && !strstr(bf, "["))
- execname = anonstr;
-
- if (execname == NULL)
- continue;
-
- pbf += 3;
- n = hex2u64(pbf, &event->mmap.pgoff);
-
- size = strlen(execname);
- execname[size - 1] = '\0'; /* Remove \n */
- memcpy(event->mmap.filename, execname, size);
- size = PERF_ALIGN(size, sizeof(u64));
- event->mmap.len -= event->mmap.start;
- event->mmap.header.size = (sizeof(event->mmap) -
- (sizeof(event->mmap.filename) - size));
- memset(event->mmap.filename + size, 0, machine->id_hdr_size);
- event->mmap.header.size += machine->id_hdr_size;
- event->mmap.pid = tgid;
- event->mmap.tid = pid;
-
- if (process(tool, event, &synth_sample, machine) != 0) {
- rc = -1;
- break;
- }
+
+ if (prot[2] != 'x')
+ event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;
+
+ size = strlen(exec_name) + 1;
+ memcpy(event->mmap.filename, exec_name, size);
+ size = PERF_ALIGN(size, sizeof(u64));
+ event->mmap.len -= event->mmap.start;
+ event->mmap.header.size = (sizeof(event->mmap) -
+ (sizeof(event->mmap.filename) - size));
+ memset(event->mmap.filename + size, 0, machine->id_hdr_size);
+ event->mmap.header.size += machine->id_hdr_size;
+ event->mmap.pid = tgid;
+ event->mmap.tid = pid;
+
+ if (process(tool, event, &synth_sample, machine) != 0) {
+ rc = -1;
+ break;
}
}
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Output of /proc/<pid>/maps contains helpful information to anonymous
mappings like stack, heap, ... For the case of stack, it can show
multiple stack area for each thread in the process:
$ cat /proc/$(pidof gnome-shell)/maps | grep stack
7fe019946000-7fe01a146000 rw-p 00000000 00:00 0 [stack:1624]
7fe040e32000-7fe041632000 rw-p 00000000 00:00 0 [stack:1451]
7fe041643000-7fe041e43000 rw-p 00000000 00:00 0 [stack:1450]
7fe04204b000-7fe04284b000 rw-p 00000000 00:00 0 [stack:1449]
7fe042a7e000-7fe04327e000 rw-p 00000000 00:00 0 [stack:1446]
7fe0432ff000-7fe043aff000 rw-p 00000000 00:00 0 [stack:1445]
7fe043b00000-7fe044300000 rw-p 00000000 00:00 0 [stack:1444]
7fe044301000-7fe044b01000 rw-p 00000000 00:00 0 [stack:1443]
7fe044b02000-7fe045302000 rw-p 00000000 00:00 0 [stack:1442]
7fe045303000-7fe045b03000 rw-p 00000000 00:00 0 [stack:1441]
7fe045b04000-7fe046304000 rw-p 00000000 00:00 0 [stack:1440]
7fe046305000-7fe046b05000 rw-p 00000000 00:00 0 [stack:1439]
7fe046b06000-7fe047306000 rw-p 00000000 00:00 0 [stack:1438]
7fff4b16f000-7fff4b190000 rw-p 00000000 00:00 0 [stack]
However perf only knew about the main thread's. Fix it.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/map.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9b40c444039c..579187865f08 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -24,7 +24,7 @@ static inline int is_anon_memory(const char *filename)
static inline int is_no_dso_memory(const char *filename)
{
- return !strcmp(filename, "[stack]") ||
+ return !strncmp(filename, "[stack", 6) ||
!strcmp(filename, "[heap]");
}
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Those data should be free along with the associated hist_entry,
otherwise they'll be leaked.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/hist.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 52fe4e24502b..50030133fb3b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -470,6 +470,8 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
void hist_entry__free(struct hist_entry *he)
{
+ free(he->branch_info);
+ free(he->mem_info);
free(he);
}
--
1.7.11.7
From: Namhyung Kim <[email protected]>
The symbol addresses in a dso have relative offsets from the start of
a mapping. So in order to ouput correct offset value from @ip, one of
them should be converted.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/sort.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index e2e466d24c16..166480ca2865 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -186,7 +186,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
if (map->type == MAP__VARIABLE) {
ret += repsep_snprintf(bf + ret, size - ret, "%s", sym->name);
ret += repsep_snprintf(bf + ret, size - ret, "+0x%llx",
- ip - sym->start);
+ ip - map->unmap_ip(map, sym->start));
ret += repsep_snprintf(bf + ret, size - ret, "%-*s",
width - ret, "");
} else {
--
1.7.11.7
From: Namhyung Kim <[email protected]>
When loading symbols in a data mapping, ABS symbols (which has a value
of SHN_ABS in its st_shndx) failed at elf_getscn(). And it marks the
loading as a failure so already loaded symbols cannot be fixed up.
I'm not sure what should be done. Just ignore them for now. :)
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/symbol-elf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index db0cc92cf2ea..00cf128e26f4 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -719,6 +719,9 @@ int dso__load_sym(struct dso *dso, struct map *map,
used_opd = true;
}
+ if (sym.st_shndx == SHN_ABS)
+ continue;
+
sec = elf_getscn(runtime_ss->elf, sym.st_shndx);
if (!sec)
goto out_elf_end;
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Currently only text (function) mapping was set, so that the kernel
data addresses couldn't parsed correctly. Fix it.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/machine.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 34d8dfeaa2b2..fc471704af12 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -84,15 +84,19 @@ int machine__process_lost_event(struct machine *machine __maybe_unused,
static void machine__set_kernel_mmap_len(struct machine *machine,
union perf_event *event)
{
- machine->vmlinux_maps[MAP__FUNCTION]->start = event->mmap.start;
- machine->vmlinux_maps[MAP__FUNCTION]->end = (event->mmap.start +
- event->mmap.len);
- /*
- * Be a bit paranoid here, some perf.data file came with
- * a zero sized synthesized MMAP event for the kernel.
- */
- if (machine->vmlinux_maps[MAP__FUNCTION]->end == 0)
- machine->vmlinux_maps[MAP__FUNCTION]->end = ~0ULL;
+ int i;
+
+ for (i = 0; i < MAP__NR_TYPES; i++) {
+ machine->vmlinux_maps[i]->start = event->mmap.start;
+ machine->vmlinux_maps[i]->end = (event->mmap.start +
+ event->mmap.len);
+ /*
+ * Be a bit paranoid here, some perf.data file came with
+ * a zero sized synthesized MMAP event for the kernel.
+ */
+ if (machine->vmlinux_maps[i]->end == 0)
+ machine->vmlinux_maps[i]->end = ~0ULL;
+ }
}
static int machine__process_kernel_mmap_event(struct machine *machine,
--
1.7.11.7
Em Wed, Nov 07, 2012 at 04:27:09PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <[email protected]>
>
> Current perf_event__synthesize_mmap_events() only deals with
> executable mappings. With upcoming memory access sampling,
Not "upcoming", "recently added", since your patch needs the patch, in
Stephane series, that introduces PERF_RECORD_MISC_MMAP_DATA.
> non-executable data mappings are needed also.
>
> While at it, convert parsing code to use sscanf which makes
> the code cleaner IMHO.
Please split the patch in two, one that does the
PERF_RECORD_MISC_MMAP_DATA and other that converts from hex2u64 to
sscanf.
- Arnaldo
> Cc: Stephane Eranian <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/util/event.c | 78 ++++++++++++++++++++++---------------------------
> 1 file changed, 35 insertions(+), 43 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index ca9ca285406a..068acf606b40 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -193,55 +193,47 @@ static int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> event->header.misc = PERF_RECORD_MISC_USER;
>
> while (1) {
> - char bf[BUFSIZ], *pbf = bf;
> - int n;
> + char bf[BUFSIZ];
> + char prot[5], *pprot = prot;
> size_t size;
> + char exec_name[PATH_MAX], *pexec = exec_name;
> + char anonstr[] = "//anon";
> +
> if (fgets(bf, sizeof(bf), fp) == NULL)
> break;
>
> + /* ensure null termination since stack will be reused */
> + strcpy(exec_name, "");
> +
> /* 00400000-0040c000 r-xp 00000000 fd:01 41038 /bin/cat */
> - n = hex2u64(pbf, &event->mmap.start);
> - if (n < 0)
> - continue;
> - pbf += n + 1;
> - n = hex2u64(pbf, &event->mmap.len);
> - if (n < 0)
> + sscanf(bf, "%"PRIx64"-%"PRIx64" %s %"PRIx64" %*x:%*x %*u %s\n",
> + &event->mmap.start, &event->mmap.len, pprot,
> + &event->mmap.pgoff, pexec);
> +
> + if (prot[2] == 'x' && !strcmp(exec_name, ""))
> + strcpy(exec_name, anonstr);
> +
> + /* ignore non-executable anon mappings */
> + if (!strcmp(exec_name, ""))
> continue;
> - pbf += n + 3;
> - if (*pbf == 'x') { /* vm_exec */
> - char anonstr[] = "//anon\n";
> - char *execname = strchr(bf, '/');
> -
> - /* Catch VDSO */
> - if (execname == NULL)
> - execname = strstr(bf, "[vdso]");
> -
> - /* Catch anonymous mmaps */
> - if ((execname == NULL) && !strstr(bf, "["))
> - execname = anonstr;
> -
> - if (execname == NULL)
> - continue;
> -
> - pbf += 3;
> - n = hex2u64(pbf, &event->mmap.pgoff);
> -
> - size = strlen(execname);
> - execname[size - 1] = '\0'; /* Remove \n */
> - memcpy(event->mmap.filename, execname, size);
> - size = PERF_ALIGN(size, sizeof(u64));
> - event->mmap.len -= event->mmap.start;
> - event->mmap.header.size = (sizeof(event->mmap) -
> - (sizeof(event->mmap.filename) - size));
> - memset(event->mmap.filename + size, 0, machine->id_hdr_size);
> - event->mmap.header.size += machine->id_hdr_size;
> - event->mmap.pid = tgid;
> - event->mmap.tid = pid;
> -
> - if (process(tool, event, &synth_sample, machine) != 0) {
> - rc = -1;
> - break;
> - }
> +
> + if (prot[2] != 'x')
> + event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;
> +
> + size = strlen(exec_name) + 1;
> + memcpy(event->mmap.filename, exec_name, size);
> + size = PERF_ALIGN(size, sizeof(u64));
> + event->mmap.len -= event->mmap.start;
> + event->mmap.header.size = (sizeof(event->mmap) -
> + (sizeof(event->mmap.filename) - size));
> + memset(event->mmap.filename + size, 0, machine->id_hdr_size);
> + event->mmap.header.size += machine->id_hdr_size;
> + event->mmap.pid = tgid;
> + event->mmap.tid = pid;
> +
> + if (process(tool, event, &synth_sample, machine) != 0) {
> + rc = -1;
> + break;
> }
> }
>
> --
> 1.7.11.7
Commit-ID: 4552cf0f774ae3d24bf31e91324586274a552a66
Gitweb: http://git.kernel.org/tip/4552cf0f774ae3d24bf31e91324586274a552a66
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 7 Nov 2012 16:27:10 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Nov 2012 11:56:16 -0300
perf machine: Set kernel data mapping length
Currently only text (function) mapping was set, so that the kernel data
addresses couldn't parsed correctly. Fix it.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/machine.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 502eec0..4c6754a 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -84,15 +84,19 @@ int machine__process_lost_event(struct machine *machine __maybe_unused,
static void machine__set_kernel_mmap_len(struct machine *machine,
union perf_event *event)
{
- machine->vmlinux_maps[MAP__FUNCTION]->start = event->mmap.start;
- machine->vmlinux_maps[MAP__FUNCTION]->end = (event->mmap.start +
- event->mmap.len);
- /*
- * Be a bit paranoid here, some perf.data file came with
- * a zero sized synthesized MMAP event for the kernel.
- */
- if (machine->vmlinux_maps[MAP__FUNCTION]->end == 0)
- machine->vmlinux_maps[MAP__FUNCTION]->end = ~0ULL;
+ int i;
+
+ for (i = 0; i < MAP__NR_TYPES; i++) {
+ machine->vmlinux_maps[i]->start = event->mmap.start;
+ machine->vmlinux_maps[i]->end = (event->mmap.start +
+ event->mmap.len);
+ /*
+ * Be a bit paranoid here, some perf.data file came with
+ * a zero sized synthesized MMAP event for the kernel.
+ */
+ if (machine->vmlinux_maps[i]->end == 0)
+ machine->vmlinux_maps[i]->end = ~0ULL;
+ }
}
static int machine__process_kernel_mmap_event(struct machine *machine,
Commit-ID: 1e82574d1db1451f137cb520f21b9176f05284c9
Gitweb: http://git.kernel.org/tip/1e82574d1db1451f137cb520f21b9176f05284c9
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 7 Nov 2012 16:27:11 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Nov 2012 11:59:32 -0300
perf tools: Fix detection of stack area
Output of /proc/<pid>/maps contains helpful information to anonymous
mappings like stack, heap, ... For the case of stack, it can show
multiple stack area for each thread in the process:
$ cat /proc/$(pidof gnome-shell)/maps | grep stack
7fe019946000-7fe01a146000 rw-p 00000000 00:00 0 [stack:1624]
7fe040e32000-7fe041632000 rw-p 00000000 00:00 0 [stack:1451]
7fe041643000-7fe041e43000 rw-p 00000000 00:00 0 [stack:1450]
7fe04204b000-7fe04284b000 rw-p 00000000 00:00 0 [stack:1449]
7fe042a7e000-7fe04327e000 rw-p 00000000 00:00 0 [stack:1446]
7fe0432ff000-7fe043aff000 rw-p 00000000 00:00 0 [stack:1445]
7fe043b00000-7fe044300000 rw-p 00000000 00:00 0 [stack:1444]
7fe044301000-7fe044b01000 rw-p 00000000 00:00 0 [stack:1443]
7fe044b02000-7fe045302000 rw-p 00000000 00:00 0 [stack:1442]
7fe045303000-7fe045b03000 rw-p 00000000 00:00 0 [stack:1441]
7fe045b04000-7fe046304000 rw-p 00000000 00:00 0 [stack:1440]
7fe046305000-7fe046b05000 rw-p 00000000 00:00 0 [stack:1439]
7fe046b06000-7fe047306000 rw-p 00000000 00:00 0 [stack:1438]
7fff4b16f000-7fff4b190000 rw-p 00000000 00:00 0 [stack]
However perf only knew about the main thread's. Fix it.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/map.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9b40c44..5791878 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -24,7 +24,7 @@ static inline int is_anon_memory(const char *filename)
static inline int is_no_dso_memory(const char *filename)
{
- return !strcmp(filename, "[stack]") ||
+ return !strncmp(filename, "[stack", 6) ||
!strcmp(filename, "[heap]");
}
Commit-ID: 580e338d7e9dc4947cba2e1021e78e76ebe0869e
Gitweb: http://git.kernel.org/tip/580e338d7e9dc4947cba2e1021e78e76ebe0869e
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 7 Nov 2012 16:27:14 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Nov 2012 12:05:12 -0300
perf hists: Free branch_info when freeing hist_entry
Those data should be free along with the associated hist_entry,
otherwise it'll be leaked.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ committer note: mem_info is not yet in perf/core, free just branch_info ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 277947a..a1b823f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -410,6 +410,7 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)
void hist_entry__free(struct hist_entry *he)
{
+ free(he->branch_info);
free(he);
}