linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 0/9] perf tools: kcore improvements
@ 2013-10-08  8:45 Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules Adrian Hunter
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Hi

Here are some improvements for using kcore (version 5).  There are 3
improvements:

	- validate that kcore matches the perf.data modules
	- workaround objdump difficulties with kcore
	- add kcore to the build-id cache

Changes in V5:
	perf tools: make a separate function to parse /proc/modules
		Use scnprintf not snprintf
	perf tools: validate kcore module addresses
		Fix check for mi->name not allocated
	perf buildid-cache: add ability to add kcore to the cache
		Use scnprintf not snprintf
	perf tools: add ability to find kcore in build-id cache
		Use scnprintf not snprintf

Changes in V4:
	perf tools: fix path unpopulated in machine__create_modules()
		Dropped because it has been applied
	perf buildid-cache: add ability to add kcore to the cache
		Tweaked Documentation/perf-buildid-cache.txt
	perf tools: add ability to find kcore in build-id cache
		Changed to check read access to /proc/kcore before
		skipping the buildid cache

Changes in V3:
	perf tools: workaround objdump difficulties with kcore
		change strncpy to strlcpy
	perf buildid-cache: add ability to add kcore to the cache
		change strncpy to strlcpy
	perf tools: add ability to find kcore in build-id cache
		change strncpy to strlcpy
Changes in V2:
	perf tools: fix buildid cache handling of kallsyms with kcore
		Dropped because it has been applied
	perf tools: fix path unpopulated in machine__create_modules()
		Use 'modules' pointer

Adrian Hunter (9):
      perf tools: make a separate function to parse /proc/modules
      perf tools: validate kcore module addresses
      perf tools: workaround objdump difficulties with kcore
      perf tools: add map__find_other_map_symbol()
      perf tools: fix annotate_browser__callq()
      perf tools: find kcore symbols on other maps
      perf tools: add copyfile_mode()
      perf buildid-cache: add ability to add kcore to the cache
      perf tools: add ability to find kcore in build-id cache

 tools/perf/Documentation/perf-buildid-cache.txt |  13 +
 tools/perf/builtin-buildid-cache.c              | 148 +++++-
 tools/perf/ui/browsers/annotate.c               |  10 +-
 tools/perf/util/annotate.c                      |  36 +-
 tools/perf/util/machine.c                       |  67 +--
 tools/perf/util/map.c                           |  27 ++
 tools/perf/util/map.h                           |   2 +
 tools/perf/util/symbol-elf.c                    | 579 ++++++++++++++++++++++++
 tools/perf/util/symbol-minimal.c                |  15 +
 tools/perf/util/symbol.c                        | 442 +++++++++++++++---
 tools/perf/util/symbol.h                        |  20 +
 tools/perf/util/util.c                          |  18 +-
 tools/perf/util/util.h                          |   1 +
 13 files changed, 1244 insertions(+), 134 deletions(-)


Regards
Adrian


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-15  5:31   ` [tip:perf/core] perf symbols: Make a separate function to parse / proc/modules tip-bot for Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 2/9] perf tools: validate kcore module addresses Adrian Hunter
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Make a separate function to parse /proc/modules
so that it can be reused.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/machine.c | 67 +++++++++++++----------------------------------
 tools/perf/util/symbol.c  | 58 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h  |  3 +++
 3 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 40083df..a36419e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -776,12 +776,22 @@ static int machine__set_modules_path(struct machine *machine)
 	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path);
 }
 
-static int machine__create_modules(struct machine *machine)
+static int machine__create_module(void *arg, const char *name, u64 start)
 {
-	char *line = NULL;
-	size_t n;
-	FILE *file;
+	struct machine *machine = arg;
 	struct map *map;
+
+	map = machine__new_module(machine, start, name);
+	if (map == NULL)
+		return -1;
+
+	dso__kernel_module_get_build_id(map->dso, machine->root_dir);
+
+	return 0;
+}
+
+static int machine__create_modules(struct machine *machine)
+{
 	const char *modules;
 	char path[PATH_MAX];
 
@@ -795,56 +805,15 @@ static int machine__create_modules(struct machine *machine)
 	if (symbol__restricted_filename(path, "/proc/modules"))
 		return -1;
 
-	file = fopen(modules, "r");
-	if (file == NULL)
+	if (modules__parse(modules, machine, machine__create_module))
 		return -1;
 
-	while (!feof(file)) {
-		char name[PATH_MAX];
-		u64 start;
-		char *sep;
-		int line_len;
-
-		line_len = getline(&line, &n, file);
-		if (line_len < 0)
-			break;
-
-		if (!line)
-			goto out_failure;
-
-		line[--line_len] = '\0'; /* \n */
-
-		sep = strrchr(line, 'x');
-		if (sep == NULL)
-			continue;
-
-		hex2u64(sep + 1, &start);
-
-		sep = strchr(line, ' ');
-		if (sep == NULL)
-			continue;
-
-		*sep = '\0';
-
-		snprintf(name, sizeof(name), "[%s]", line);
-		map = machine__new_module(machine, start, name);
-		if (map == NULL)
-			goto out_delete_line;
-		dso__kernel_module_get_build_id(map->dso, machine->root_dir);
-	}
+	if (!machine__set_modules_path(machine))
+		return 0;
 
-	free(line);
-	fclose(file);
+	pr_debug("Problems setting modules path maps, continuing anyway...\n");
 
-	if (machine__set_modules_path(machine) < 0) {
-		pr_debug("Problems setting modules path maps, continuing anyway...\n");
-	}
 	return 0;
-
-out_delete_line:
-	free(line);
-out_failure:
-	return -1;
 }
 
 int machine__create_kernel_maps(struct machine *machine)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 48c3879..5fd9513 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -500,6 +500,64 @@ out_failure:
 	return -1;
 }
 
+int modules__parse(const char *filename, void *arg,
+		   int (*process_module)(void *arg, const char *name,
+					 u64 start))
+{
+	char *line = NULL;
+	size_t n;
+	FILE *file;
+	int err = 0;
+
+	file = fopen(filename, "r");
+	if (file == NULL)
+		return -1;
+
+	while (1) {
+		char name[PATH_MAX];
+		u64 start;
+		char *sep;
+		ssize_t line_len;
+
+		line_len = getline(&line, &n, file);
+		if (line_len < 0) {
+			if (feof(file))
+				break;
+			err = -1;
+			goto out;
+		}
+
+		if (!line) {
+			err = -1;
+			goto out;
+		}
+
+		line[--line_len] = '\0'; /* \n */
+
+		sep = strrchr(line, 'x');
+		if (sep == NULL)
+			continue;
+
+		hex2u64(sep + 1, &start);
+
+		sep = strchr(line, ' ');
+		if (sep == NULL)
+			continue;
+
+		*sep = '\0';
+
+		scnprintf(name, sizeof(name), "[%s]", line);
+
+		err = process_module(arg, name, start);
+		if (err)
+			break;
+	}
+out:
+	free(line);
+	fclose(file);
+	return err;
+}
+
 struct process_kallsyms_args {
 	struct map *map;
 	struct dso *dso;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 2d4ee9a..a2543f0 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -223,6 +223,9 @@ int sysfs__read_build_id(const char *filename, void *bf, size_t size);
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
 					  char type, u64 start));
+int modules__parse(const char *filename, void *arg,
+		   int (*process_module)(void *arg, const char *name,
+					 u64 start));
 int filename__read_debuglink(const char *filename, char *debuglink,
 			     size_t size);
 
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 2/9] perf tools: validate kcore module addresses
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore Adrian Hunter
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Before using kcore we need to check that modules are
in memory at the same addresses that they were when
data was recorded.

This is done because, while we could remap symbols
to different addresses, the object code linkages
would still be different which would provide an
erroneous view of the object code.

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

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5fd9513..2a2c581 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -798,51 +798,201 @@ bool symbol__restricted_filename(const char *filename,
 	return restricted;
 }
 
-struct kcore_mapfn_data {
-	struct dso *dso;
-	enum map_type type;
-	struct list_head maps;
+struct module_info {
+	struct rb_node rb_node;
+	char *name;
+	u64 start;
 };
 
-static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
+static void add_module(struct module_info *mi, struct rb_root *modules)
 {
-	struct kcore_mapfn_data *md = data;
-	struct map *map;
+	struct rb_node **p = &modules->rb_node;
+	struct rb_node *parent = NULL;
+	struct module_info *m;
 
-	map = map__new2(start, md->dso, md->type);
-	if (map == NULL)
+	while (*p != NULL) {
+		parent = *p;
+		m = rb_entry(parent, struct module_info, rb_node);
+		if (strcmp(mi->name, m->name) < 0)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+	rb_link_node(&mi->rb_node, parent, p);
+	rb_insert_color(&mi->rb_node, modules);
+}
+
+static void delete_modules(struct rb_root *modules)
+{
+	struct module_info *mi;
+	struct rb_node *next = rb_first(modules);
+
+	while (next) {
+		mi = rb_entry(next, struct module_info, rb_node);
+		next = rb_next(&mi->rb_node);
+		rb_erase(&mi->rb_node, modules);
+		free(mi->name);
+		free(mi);
+	}
+}
+
+static struct module_info *find_module(const char *name,
+				       struct rb_root *modules)
+{
+	struct rb_node *n = modules->rb_node;
+
+	while (n) {
+		struct module_info *m;
+		int cmp;
+
+		m = rb_entry(n, struct module_info, rb_node);
+		cmp = strcmp(name, m->name);
+		if (cmp < 0)
+			n = n->rb_left;
+		else if (cmp > 0)
+			n = n->rb_right;
+		else
+			return m;
+	}
+
+	return NULL;
+}
+
+static int __read_proc_modules(void *arg, const char *name, u64 start)
+{
+	struct rb_root *modules = arg;
+	struct module_info *mi;
+
+	mi = zalloc(sizeof(struct module_info));
+	if (!mi)
 		return -ENOMEM;
 
-	map->end = map->start + len;
-	map->pgoff = pgoff;
+	mi->name = strdup(name);
+	mi->start = start;
 
-	list_add(&map->node, &md->maps);
+	if (!mi->name) {
+		free(mi);
+		return -ENOMEM;
+	}
+
+	add_module(mi, modules);
+
+	return 0;
+}
+
+static int read_proc_modules(const char *filename, struct rb_root *modules)
+{
+	if (symbol__restricted_filename(filename, "/proc/modules"))
+		return -1;
+
+	if (modules__parse(filename, modules, __read_proc_modules)) {
+		delete_modules(modules);
+		return -1;
+	}
 
 	return 0;
 }
 
+static int do_validate_kcore_modules(const char *filename, struct map *map,
+				  struct map_groups *kmaps)
+{
+	struct rb_root modules = RB_ROOT;
+	struct map *old_map;
+	int err;
+
+	err = read_proc_modules(filename, &modules);
+	if (err)
+		return err;
+
+	old_map = map_groups__first(kmaps, map->type);
+	while (old_map) {
+		struct map *next = map_groups__next(old_map);
+		struct module_info *mi;
+
+		if (old_map == map || old_map->start == map->start) {
+			/* The kernel map */
+			old_map = next;
+			continue;
+		}
+
+		/* Module must be in memory at the same address */
+		mi = find_module(old_map->dso->short_name, &modules);
+		if (!mi || mi->start != old_map->start) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		old_map = next;
+	}
+out:
+	delete_modules(&modules);
+	return err;
+}
+
 /*
- * If kallsyms is referenced by name then we look for kcore in the same
+ * If kallsyms is referenced by name then we look for filename in the same
  * directory.
  */
-static bool kcore_filename_from_kallsyms_filename(char *kcore_filename,
-						  const char *kallsyms_filename)
+static bool filename_from_kallsyms_filename(char *filename,
+					    const char *basename,
+					    const char *kallsyms_filename)
 {
 	char *name;
 
-	strcpy(kcore_filename, kallsyms_filename);
-	name = strrchr(kcore_filename, '/');
+	strcpy(filename, kallsyms_filename);
+	name = strrchr(filename, '/');
 	if (!name)
 		return false;
 
-	if (!strcmp(name, "/kallsyms")) {
-		strcpy(name, "/kcore");
+	name += 1;
+
+	if (!strcmp(name, "kallsyms")) {
+		strcpy(name, basename);
 		return true;
 	}
 
 	return false;
 }
 
+static int validate_kcore_modules(const char *kallsyms_filename,
+				  struct map *map)
+{
+	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	char modules_filename[PATH_MAX];
+
+	if (!filename_from_kallsyms_filename(modules_filename, "modules",
+					     kallsyms_filename))
+		return -EINVAL;
+
+	if (do_validate_kcore_modules(modules_filename, map, kmaps))
+		return -EINVAL;
+
+	return 0;
+}
+
+struct kcore_mapfn_data {
+	struct dso *dso;
+	enum map_type type;
+	struct list_head maps;
+};
+
+static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
+{
+	struct kcore_mapfn_data *md = data;
+	struct map *map;
+
+	map = map__new2(start, md->dso, md->type);
+	if (map == NULL)
+		return -ENOMEM;
+
+	map->end = map->start + len;
+	map->pgoff = pgoff;
+
+	list_add(&map->node, &md->maps);
+
+	return 0;
+}
+
 static int dso__load_kcore(struct dso *dso, struct map *map,
 			   const char *kallsyms_filename)
 {
@@ -859,8 +1009,12 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	if (map != machine->vmlinux_maps[map->type])
 		return -EINVAL;
 
-	if (!kcore_filename_from_kallsyms_filename(kcore_filename,
-						   kallsyms_filename))
+	if (!filename_from_kallsyms_filename(kcore_filename, "kcore",
+					     kallsyms_filename))
+		return -EINVAL;
+
+	/* All modules must be present at their original addresses */
+	if (validate_kcore_modules(kallsyms_filename, map))
 		return -EINVAL;
 
 	md.dso = dso;
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 2/9] perf tools: validate kcore module addresses Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08 14:02   ` Jiri Olsa
  2013-10-08 15:56   ` Arnaldo Carvalho de Melo
  2013-10-08  8:45 ` [PATCH V5 4/9] perf tools: add map__find_other_map_symbol() Adrian Hunter
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

objdump fails to annotate module symbols when looking
at kcore.  Workaround this by extracting object code
from kcore and putting it in a temporary file for
objdump to use instead.  The temporary file is created
to look like kcore but contains only the function
being disassembled.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/annotate.c       |  21 ++++
 tools/perf/util/symbol-elf.c     | 221 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol-minimal.c |   9 ++
 tools/perf/util/symbol.h         |  14 +++
 4 files changed, 265 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f7bdc01..46746b8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -879,6 +879,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 	FILE *file;
 	int err = 0;
 	char symfs_filename[PATH_MAX];
+	struct kcore_extract kce;
+	bool delete_extract = false;
 
 	if (filename) {
 		snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
@@ -940,6 +942,23 @@ fallback:
 	pr_debug("annotating [%p] %30s : [%p] %30s\n",
 		 dso, dso->long_name, sym, sym->name);
 
+	if (dso__is_kcore(dso)) {
+		kce.kcore_filename = symfs_filename;
+		kce.addr = map__rip_2objdump(map, sym->start);
+		kce.offs = sym->start;
+		kce.len = sym->end + 1 - sym->start;
+		if (!create_kcore_extract(&kce)) {
+			delete_extract = true;
+			strlcpy(symfs_filename, kce.extract_filename,
+				sizeof(symfs_filename));
+			if (free_filename) {
+				free(filename);
+				free_filename = false;
+			}
+			filename = symfs_filename;
+		}
+	}
+
 	snprintf(command, sizeof(command),
 		 "%s %s%s --start-address=0x%016" PRIx64
 		 " --stop-address=0x%016" PRIx64
@@ -972,6 +991,8 @@ fallback:
 
 	pclose(file);
 out_free_filename:
+	if (delete_extract)
+		delete_kcore_extract(&kce);
 	if (free_filename)
 		free(filename);
 	return err;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a7b9ab5..79b27fc7 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1002,6 +1002,227 @@ int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
 	return err;
 }
 
+static int copy_bytes(int from, off_t from_offs, int to, off_t to_offs, u64 len)
+{
+	char buf[page_size];
+	ssize_t r;
+	size_t n;
+
+	if (lseek(to, to_offs, SEEK_SET) != to_offs)
+		return -1;
+
+	if (lseek(from, from_offs, SEEK_SET) != from_offs)
+		return -1;
+
+	while (len) {
+		n = sizeof(buf);
+		if (len < n)
+			n = len;
+		/* Use read because mmap won't work on proc files */
+		r = read(from, buf, n);
+		if (r < 0)
+			return -1;
+		if (!r)
+			break;
+		n = r;
+		r = write(to, buf, n);
+		if (r < 0)
+			return -1;
+		if ((size_t)r != n)
+			return -1;
+		len -= n;
+	}
+	return 0;
+}
+
+struct kcore {
+	int fd;
+	int elfclass;
+	Elf *elf;
+	GElf_Ehdr ehdr;
+};
+
+static int kcore_open(const char *filename, struct kcore *kcore)
+{
+	GElf_Ehdr *ehdr;
+
+	kcore->fd = open(filename, O_RDONLY);
+	if (kcore->fd == -1)
+		return -1;
+
+	kcore->elf = elf_begin(kcore->fd, ELF_C_READ, NULL);
+	if (!kcore->elf)
+		goto out_close;
+
+	kcore->elfclass = gelf_getclass(kcore->elf);
+	if (kcore->elfclass == ELFCLASSNONE)
+		goto out_end;
+
+	ehdr = gelf_getehdr(kcore->elf, &kcore->ehdr);
+	if (!ehdr)
+		goto out_end;
+
+	return 0;
+
+out_end:
+	elf_end(kcore->elf);
+out_close:
+	close(kcore->fd);
+	return -1;
+}
+
+static int kcore_new(char *filename, struct kcore *kcore, int elfclass,
+		     bool temp)
+{
+	GElf_Ehdr *ehdr;
+
+	kcore->elfclass = elfclass;
+
+	if (temp)
+		kcore->fd = mkstemp(filename);
+	else
+		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
+	if (kcore->fd == -1)
+		return -1;
+
+	kcore->elf = elf_begin(kcore->fd, ELF_C_WRITE, NULL);
+	if (!kcore->elf)
+		goto out_close;
+
+	if (!gelf_newehdr(kcore->elf, elfclass))
+		goto out_end;
+
+	ehdr = gelf_getehdr(kcore->elf, &kcore->ehdr);
+	if (!ehdr)
+		goto out_end;
+
+	return 0;
+
+out_end:
+	elf_end(kcore->elf);
+out_close:
+	close(kcore->fd);
+	unlink(filename);
+	return -1;
+}
+
+static void kcore_close(struct kcore *kcore)
+{
+	elf_end(kcore->elf);
+	close(kcore->fd);
+}
+
+static int kcore_copy_hdr(struct kcore *from, struct kcore *to, size_t count)
+{
+	GElf_Ehdr *ehdr = &to->ehdr;
+	GElf_Ehdr *kehdr = &from->ehdr;
+
+	memcpy(ehdr->e_ident, kehdr->e_ident, EI_NIDENT);
+	ehdr->e_type      = kehdr->e_type;
+	ehdr->e_machine   = kehdr->e_machine;
+	ehdr->e_version   = kehdr->e_version;
+	ehdr->e_entry     = 0;
+	ehdr->e_shoff     = 0;
+	ehdr->e_flags     = kehdr->e_flags;
+	ehdr->e_phnum     = count;
+	ehdr->e_shentsize = 0;
+	ehdr->e_shnum     = 0;
+	ehdr->e_shstrndx  = 0;
+
+	if (from->elfclass == ELFCLASS32) {
+		ehdr->e_phoff     = sizeof(Elf32_Ehdr);
+		ehdr->e_ehsize    = sizeof(Elf32_Ehdr);
+		ehdr->e_phentsize = sizeof(Elf32_Phdr);
+	} else {
+		ehdr->e_phoff     = sizeof(Elf64_Ehdr);
+		ehdr->e_ehsize    = sizeof(Elf64_Ehdr);
+		ehdr->e_phentsize = sizeof(Elf64_Phdr);
+	}
+
+	if (!gelf_update_ehdr(to->elf, ehdr))
+		return -1;
+
+	if (!gelf_newphdr(to->elf, count))
+		return -1;
+
+	return 0;
+}
+
+static int kcore_add_phdr(struct kcore *kcore, int index, off_t offset,
+			  u64 addr, u64 len)
+{
+	GElf_Phdr gphdr;
+	GElf_Phdr *phdr;
+
+	phdr = gelf_getphdr(kcore->elf, index, &gphdr);
+	if (!phdr)
+		return -1;
+
+	phdr->p_type	= PT_LOAD;
+	phdr->p_flags	= PF_R | PF_W | PF_X;
+	phdr->p_offset	= offset;
+	phdr->p_vaddr	= addr;
+	phdr->p_paddr	= 0;
+	phdr->p_filesz	= len;
+	phdr->p_memsz	= len;
+	phdr->p_align	= page_size;
+
+	if (!gelf_update_phdr(kcore->elf, index, phdr))
+		return -1;
+
+	return 0;
+}
+
+static off_t kcore_write(struct kcore *kcore)
+{
+	return elf_update(kcore->elf, ELF_C_WRITE);
+}
+
+int create_kcore_extract(struct kcore_extract *kce)
+{
+	struct kcore kcore;
+	struct kcore extract;
+	size_t count = 1;
+	int index = 0, err = -1;
+	off_t offset = page_size, sz;
+
+	if (kcore_open(kce->kcore_filename, &kcore))
+		return -1;
+
+	strcpy(kce->extract_filename, PERF_KCORE_EXTRACT);
+	if (kcore_new(kce->extract_filename, &extract, kcore.elfclass, true))
+		goto out_kcore_close;
+
+	if (kcore_copy_hdr(&kcore, &extract, count))
+		goto out_extract_close;
+
+	if (kcore_add_phdr(&extract, index, offset, kce->addr, kce->len))
+		goto out_extract_close;
+
+	sz = kcore_write(&extract);
+	if (sz < 0 || sz > offset)
+		goto out_extract_close;
+
+	if (copy_bytes(kcore.fd, kce->offs, extract.fd, offset, kce->len))
+		goto out_extract_close;
+
+	err = 0;
+
+out_extract_close:
+	kcore_close(&extract);
+	if (err)
+		unlink(kce->extract_filename);
+out_kcore_close:
+	kcore_close(&kcore);
+
+	return err;
+}
+
+void delete_kcore_extract(struct kcore_extract *kce)
+{
+	unlink(kce->extract_filename);
+}
+
 void symbol__elf_init(void)
 {
 	elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 3a802c3..0330163 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -308,6 +308,15 @@ int file__read_maps(int fd __maybe_unused, bool exe __maybe_unused,
 	return -1;
 }
 
+int create_kcore_extract(struct kcore_extract *kce __maybe_unused)
+{
+	return -1;
+}
+
+void delete_kcore_extract(struct kcore_extract *kce __maybe_unused)
+{
+}
+
 void symbol__elf_init(void)
 {
 }
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index a2543f0..54f3ed0 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -256,4 +256,18 @@ typedef int (*mapfn_t)(u64 start, u64 len, u64 pgoff, void *data);
 int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
 		    bool *is_64_bit);
 
+#define PERF_KCORE_EXTRACT "/tmp/perf-kcore-XXXXXX"
+
+struct kcore_extract {
+	char *kcore_filename;
+	u64 addr;
+	u64 offs;
+	u64 len;
+	char extract_filename[sizeof(PERF_KCORE_EXTRACT)];
+	int fd;
+};
+
+int create_kcore_extract(struct kcore_extract *kce);
+void delete_kcore_extract(struct kcore_extract *kce);
+
 #endif /* __PERF_SYMBOL */
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 4/9] perf tools: add map__find_other_map_symbol()
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (2 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 5/9] perf tools: fix annotate_browser__callq() Adrian Hunter
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Add a function to find a symbol using an ip that
might be on a different map.

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

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4f6680d..beedeef 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -213,6 +213,33 @@ struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
 	return dso__find_symbol_by_name(map->dso, map->type, name);
 }
 
+struct symbol *map__find_other_map_symbol(struct map **map_ptr, u64 *ip_ptr,
+					  symbol_filter_t filter)
+{
+	struct map *map = *map_ptr;
+	u64 ip = *ip_ptr;
+	struct map *sym_map = NULL;
+	struct symbol *sym;
+
+	if (ip >= map->start && ip <= map->end)
+		sym_map = map;
+	else if (map->groups)
+		sym_map = map_groups__find(map->groups, map->type, ip);
+
+	if (!sym_map)
+		return NULL;
+
+	ip = sym_map->map_ip(sym_map, ip);
+
+	sym = map__find_symbol(sym_map, ip, filter);
+	if (sym) {
+		*map_ptr = sym_map;
+		*ip_ptr = ip;
+	}
+
+	return sym;
+}
+
 struct map *map__clone(struct map *map)
 {
 	return memdup(map, sizeof(*map));
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 4886ca2..b7b494c 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -106,6 +106,8 @@ struct symbol *map__find_symbol(struct map *map,
 				u64 addr, symbol_filter_t filter);
 struct symbol *map__find_symbol_by_name(struct map *map, const char *name,
 					symbol_filter_t filter);
+struct symbol *map__find_other_map_symbol(struct map **map_ptr, u64 *ip_ptr,
+					  symbol_filter_t filter);
 void map__fixup_start(struct map *map);
 void map__fixup_end(struct map *map);
 
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 5/9] perf tools: fix annotate_browser__callq()
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (3 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 4/9] perf tools: add map__find_other_map_symbol() Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 6/9] perf tools: find kcore symbols on other maps Adrian Hunter
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

When following a call, annotate_browser__callq()
uses the current symbol's map to look up the
target ip.  That will not work if the target ip
is on a map with a different mapping (i.e.
start - pgoff is different).

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/ui/browsers/annotate.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 08545ae..d9edb35 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -445,14 +445,16 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	struct symbol *sym = ms->sym;
 	struct annotation *notes;
 	struct symbol *target;
+	struct map *map;
 	u64 ip;
 	char title[SYM_TITLE_MAX_SIZE];
 
 	if (!ins__is_call(dl->ins))
 		return false;
 
-	ip = ms->map->map_ip(ms->map, dl->ops.target.addr);
-	target = map__find_symbol(ms->map, ip, NULL);
+	map = ms->map;
+	ip = dl->ops.target.addr;
+	target = map__find_other_map_symbol(&map, &ip, NULL);
 	if (target == NULL) {
 		ui_helpline__puts("The called function was not found.");
 		return true;
@@ -469,8 +471,8 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	}
 
 	pthread_mutex_unlock(&notes->lock);
-	symbol__tui_annotate(target, ms->map, evsel, hbt);
-	sym_title(sym, ms->map, title, sizeof(title));
+	symbol__tui_annotate(target, map, evsel, hbt);
+	sym_title(sym, map, title, sizeof(title));
 	ui_browser__show_title(&browser->b, title);
 	return true;
 }
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 6/9] perf tools: find kcore symbols on other maps
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (4 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 5/9] perf tools: fix annotate_browser__callq() Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 7/9] perf tools: add copyfile_mode() Adrian Hunter
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Use the new map__find_other_map_symbol() to
find kcore symbols on other maps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/annotate.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 46746b8..6cb7277 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -825,20 +825,15 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 		dl->ops.target.offset = dl->ops.target.addr -
 					map__rip_2objdump(map, sym->start);
 
-	/*
-	 * kcore has no symbols, so add the call target name if it is on the
-	 * same map.
-	 */
+	/* kcore has no symbols, so add the call target name */
 	if (dl->ins && ins__is_call(dl->ins) && !dl->ops.target.name) {
+		struct map *map_ptr = map;
 		struct symbol *s;
 		u64 ip = dl->ops.target.addr;
 
-		if (ip >= map->start && ip <= map->end) {
-			ip = map->map_ip(map, ip);
-			s = map__find_symbol(map, ip, NULL);
-			if (s && s->start == ip)
-				dl->ops.target.name = strdup(s->name);
-		}
+		s = map__find_other_map_symbol(&map_ptr, &ip, NULL);
+		if (s && s->start == ip)
+			dl->ops.target.name = strdup(s->name);
 	}
 
 	disasm__add(&notes->src->source, dl);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 7/9] perf tools: add copyfile_mode()
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (5 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 6/9] perf tools: find kcore symbols on other maps Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 8/9] perf buildid-cache: add ability to add kcore to the cache Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 9/9] perf tools: add ability to find kcore in build-id cache Adrian Hunter
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Add a function to copy a file specifying the
permissions to use for the created file.

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

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 141317e..2d6c42c 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -55,17 +55,20 @@ int mkdir_p(char *path, mode_t mode)
 	return (stat(path, &st) && mkdir(path, mode)) ? -1 : 0;
 }
 
-static int slow_copyfile(const char *from, const char *to)
+static int slow_copyfile(const char *from, const char *to, mode_t mode)
 {
-	int err = 0;
+	int err = -1;
 	char *line = NULL;
 	size_t n;
 	FILE *from_fp = fopen(from, "r"), *to_fp;
+	mode_t old_umask;
 
 	if (from_fp == NULL)
 		goto out;
 
+	old_umask = umask(mode ^ 0777);
 	to_fp = fopen(to, "w");
+	umask(old_umask);
 	if (to_fp == NULL)
 		goto out_fclose_from;
 
@@ -82,7 +85,7 @@ out:
 	return err;
 }
 
-int copyfile(const char *from, const char *to)
+int copyfile_mode(const char *from, const char *to, mode_t mode)
 {
 	int fromfd, tofd;
 	struct stat st;
@@ -93,13 +96,13 @@ int copyfile(const char *from, const char *to)
 		goto out;
 
 	if (st.st_size == 0) /* /proc? do it slowly... */
-		return slow_copyfile(from, to);
+		return slow_copyfile(from, to, mode);
 
 	fromfd = open(from, O_RDONLY);
 	if (fromfd < 0)
 		goto out;
 
-	tofd = creat(to, 0755);
+	tofd = creat(to, mode);
 	if (tofd < 0)
 		goto out_close_from;
 
@@ -121,6 +124,11 @@ out:
 	return err;
 }
 
+int copyfile(const char *from, const char *to)
+{
+	return copyfile_mode(from, to, 0755);
+}
+
 unsigned long convert_unit(unsigned long value, char *unit)
 {
 	*unit = ' ';
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 1f06ba4..42dfba7 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -243,6 +243,7 @@ static inline int sane_case(int x, int high)
 
 int mkdir_p(char *path, mode_t mode);
 int copyfile(const char *from, const char *to);
+int copyfile_mode(const char *from, const char *to, mode_t mode);
 
 s64 perf_atoll(const char *str);
 char **argv_split(const char *str, int *argcp);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 8/9] perf buildid-cache: add ability to add kcore to the cache
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (6 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 7/9] perf tools: add copyfile_mode() Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  2013-10-08  8:45 ` [PATCH V5 9/9] perf tools: add ability to find kcore in build-id cache Adrian Hunter
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

kcore can be used to view the running kernel object code.
However, kcore changes as modules are loaded and unloaded,
and when the kernel decides to modify its own code.
Consequently it is useful to create a copy of kcore at a
particular time.  Unlike vmlinux, kcore is not unique
for a given build-id.  And in addition, the kallsyms
and modules files are also needed.  The tool therefore
creates a directory:

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

which contains: kcore, kallsyms and modules.

Note that the copied kcore contains only code sections.
See the kcore_copy() function for how that is determined.

The tool will not make additional copies of kcore if there
is already one with the same modules at the same addresses.

Currently, perf tools will not look for kcore in the cache.
That is addressed in another patch.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-buildid-cache.txt |  13 +
 tools/perf/builtin-buildid-cache.c              | 148 +++++++++-
 tools/perf/util/symbol-elf.c                    | 358 ++++++++++++++++++++++++
 tools/perf/util/symbol-minimal.c                |   6 +
 tools/perf/util/symbol.c                        |  41 +++
 tools/perf/util/symbol.h                        |   3 +
 6 files changed, 568 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index e9a8349..fd77d81 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -21,6 +21,19 @@ OPTIONS
 -a::
 --add=::
         Add specified file to the cache.
+-k::
+--kcore::
+        Add specified kcore file to the cache. For the current host that is
+        /proc/kcore which requires root permissions to read. Be aware that
+        running 'perf buildid-cache' as root may update root's build-id cache
+        not the user's. Use the -v option to see where the file is created.
+        Note that the copied file contains only code sections not the whole core
+        image. Note also that files "kallsyms" and "modules" must also be in the
+        same directory and are also copied.  All 3 files are created with read
+        permissions for root only. kcore will not be added if there is already a
+        kcore in the cache (with the same build-id) that has the same modules at
+        the same addresses. Use the -v option to see if a copy of kcore is
+        actually made.
 -r::
 --remove=::
         Remove specified file from the cache.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index c96c8fa..8140b7b 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -6,6 +6,11 @@
  * Copyright (C) 2010, Red Hat Inc.
  * Copyright (C) 2010, Arnaldo Carvalho de Melo <acme@redhat.com>
  */
+#include <sys/types.h>
+#include <sys/time.h>
+#include <time.h>
+#include <dirent.h>
+#include <unistd.h>
 #include "builtin.h"
 #include "perf.h"
 #include "util/cache.h"
@@ -17,6 +22,140 @@
 #include "util/session.h"
 #include "util/symbol.h"
 
+static int build_id_cache__kcore_buildid(const char *proc_dir, char *sbuildid)
+{
+	char root_dir[PATH_MAX];
+	char notes[PATH_MAX];
+	u8 build_id[BUILD_ID_SIZE];
+	char *p;
+
+	strlcpy(root_dir, proc_dir, sizeof(root_dir));
+
+	p = strrchr(root_dir, '/');
+	if (!p)
+		return -1;
+	*p = '\0';
+
+	scnprintf(notes, sizeof(notes), "%s/sys/kernel/notes", root_dir);
+
+	if (sysfs__read_build_id(notes, build_id, sizeof(build_id)))
+		return -1;
+
+	build_id__sprintf(build_id, sizeof(build_id), sbuildid);
+
+	return 0;
+}
+
+static int build_id_cache__kcore_dir(char *dir, size_t sz)
+{
+	struct timeval tv;
+	struct tm tm;
+	char dt[32];
+
+	if (gettimeofday(&tv, NULL) || !localtime_r(&tv.tv_sec, &tm))
+		return -1;
+
+	if (!strftime(dt, sizeof(dt), "%Y%m%d%H%M%S", &tm))
+		return -1;
+
+	scnprintf(dir, sz, "%s%02u", dt, (unsigned)tv.tv_usec / 10000);
+
+	return 0;
+}
+
+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];
+	struct dirent *dent;
+	int ret = -1;
+	DIR *d;
+
+	d = opendir(to_dir);
+	if (!d)
+		return -1;
+
+	scnprintf(from, sizeof(from), "%s/modules", from_dir);
+
+	while (1) {
+		dent = readdir(d);
+		if (!dent)
+			break;
+		if (dent->d_type != DT_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);
+			ret = 0;
+			break;
+		}
+	}
+
+	closedir(d);
+
+	return ret;
+}
+
+static int build_id_cache__add_kcore(const char *filename, const char *debugdir)
+{
+	char dir[32], sbuildid[BUILD_ID_SIZE * 2 + 1];
+	char from_dir[PATH_MAX], to_dir[PATH_MAX];
+	char *p;
+
+	strlcpy(from_dir, filename, sizeof(from_dir));
+
+	p = strrchr(from_dir, '/');
+	if (!p || strcmp(p + 1, "kcore"))
+		return -1;
+	*p = '\0';
+
+	if (build_id_cache__kcore_buildid(from_dir, sbuildid))
+		return -1;
+
+	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s",
+		  debugdir, sbuildid);
+
+	if (!build_id_cache__kcore_existing(from_dir, to_dir, sizeof(to_dir))) {
+		pr_debug("same kcore found in %s\n", to_dir);
+		return 0;
+	}
+
+	if (build_id_cache__kcore_dir(dir, sizeof(dir)))
+		return -1;
+
+	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s/%s",
+		  debugdir, sbuildid, dir);
+
+	if (mkdir_p(to_dir, 0755))
+		return -1;
+
+	if (kcore_copy(from_dir, to_dir)) {
+		/* Remove YYYYmmddHHMMSShh directory */
+		if (!rmdir(to_dir)) {
+			p = strrchr(to_dir, '/');
+			if (p)
+				*p = '\0';
+			/* Try to remove buildid directory */
+			if (!rmdir(to_dir)) {
+				p = strrchr(to_dir, '/');
+				if (p)
+					*p = '\0';
+				/* Try to remove [kernel.kcore] directory */
+				rmdir(to_dir);
+			}
+		}
+		return -1;
+	}
+
+	pr_debug("kcore added to build-id cache directory %s\n", to_dir);
+
+	return 0;
+}
+
 static int build_id_cache__add_file(const char *filename, const char *debugdir)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -130,11 +269,14 @@ int cmd_buildid_cache(int argc, const char **argv,
 	char const *add_name_list_str = NULL,
 		   *remove_name_list_str = NULL,
 		   *missing_filename = NULL,
-		   *update_name_list_str = NULL;
+		   *update_name_list_str = NULL,
+		   *kcore_filename;
 
 	const struct option buildid_cache_options[] = {
 	OPT_STRING('a', "add", &add_name_list_str,
 		   "file list", "file(s) to add"),
+	OPT_STRING('k', "kcore", &kcore_filename,
+		   "file", "kcore file to add"),
 	OPT_STRING('r', "remove", &remove_name_list_str, "file list",
 		    "file(s) to remove"),
 	OPT_STRING('M', "missing", &missing_filename, "file",
@@ -217,5 +359,9 @@ int cmd_buildid_cache(int argc, const char **argv,
 		}
 	}
 
+	if (kcore_filename &&
+	    build_id_cache__add_kcore(kcore_filename, debugdir))
+		pr_warning("Couldn't add %s\n", kcore_filename);
+
 	return ret;
 }
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 79b27fc7..bb51330 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1178,6 +1178,364 @@ static off_t kcore_write(struct kcore *kcore)
 	return elf_update(kcore->elf, ELF_C_WRITE);
 }
 
+struct phdr_data {
+	off_t offset;
+	u64 addr;
+	u64 len;
+};
+
+struct kcore_copy_info {
+	u64 stext;
+	u64 etext;
+	u64 first_symbol;
+	u64 last_symbol;
+	u64 first_module;
+	u64 last_module_symbol;
+	struct phdr_data kernel_map;
+	struct phdr_data modules_map;
+};
+
+static int kcore_copy__process_kallsyms(void *arg, const char *name, char type,
+					u64 start)
+{
+	struct kcore_copy_info *kci = arg;
+
+	if (!symbol_type__is_a(type, MAP__FUNCTION))
+		return 0;
+
+	if (strchr(name, '[')) {
+		if (start > kci->last_module_symbol)
+			kci->last_module_symbol = start;
+		return 0;
+	}
+
+	if (!kci->first_symbol || start < kci->first_symbol)
+		kci->first_symbol = start;
+
+	if (!kci->last_symbol || start > kci->last_symbol)
+		kci->last_symbol = start;
+
+	if (!strcmp(name, "_stext")) {
+		kci->stext = start;
+		return 0;
+	}
+
+	if (!strcmp(name, "_etext")) {
+		kci->etext = start;
+		return 0;
+	}
+
+	return 0;
+}
+
+static int kcore_copy__parse_kallsyms(const char *dir,
+				      struct kcore_copy_info *kci)
+{
+	char kallsyms_filename[PATH_MAX];
+
+	scnprintf(kallsyms_filename, PATH_MAX, "%s/kallsyms", dir);
+
+	if (symbol__restricted_filename(kallsyms_filename, "/proc/kallsyms"))
+		return -1;
+
+	if (kallsyms__parse(kallsyms_filename, kci,
+			    kcore_copy__process_kallsyms) < 0)
+		return -1;
+
+	return 0;
+}
+
+static int kcore_copy__process_modules(void *arg,
+				       const char *name __maybe_unused,
+				       u64 start)
+{
+	struct kcore_copy_info *kci = arg;
+
+	if (!kci->first_module || start < kci->first_module)
+		kci->first_module = start;
+
+	return 0;
+}
+
+static int kcore_copy__parse_modules(const char *dir,
+				     struct kcore_copy_info *kci)
+{
+	char modules_filename[PATH_MAX];
+
+	scnprintf(modules_filename, PATH_MAX, "%s/modules", dir);
+
+	if (symbol__restricted_filename(modules_filename, "/proc/modules"))
+		return -1;
+
+	if (modules__parse(modules_filename, kci,
+			   kcore_copy__process_modules) < 0)
+		return -1;
+
+	return 0;
+}
+
+static void kcore_copy__map(struct phdr_data *p, u64 start, u64 end, u64 pgoff,
+			    u64 s, u64 e)
+{
+	if (p->addr || s < start || s >= end)
+		return;
+
+	p->addr = s;
+	p->offset = (s - start) + pgoff;
+	p->len = e < end ? e - s : end - s;
+}
+
+static int kcore_copy__read_map(u64 start, u64 len, u64 pgoff, void *data)
+{
+	struct kcore_copy_info *kci = data;
+	u64 end = start + len;
+
+	kcore_copy__map(&kci->kernel_map, start, end, pgoff, kci->stext,
+			kci->etext);
+
+	kcore_copy__map(&kci->modules_map, start, end, pgoff, kci->first_module,
+			kci->last_module_symbol);
+
+	return 0;
+}
+
+static int kcore_copy__read_maps(Elf *elf, struct kcore_copy_info *kci)
+{
+	if (elf_read_maps(elf, true, kcore_copy__read_map, kci) < 0)
+		return -1;
+
+	return 0;
+}
+
+static int kcore_copy__calc_maps(const char *dir, Elf *elf,
+				 struct kcore_copy_info *kci)
+{
+	if (kcore_copy__parse_kallsyms(dir, kci))
+		return -1;
+
+	if (kcore_copy__parse_modules(dir, kci))
+		return -1;
+
+	if (kci->stext)
+		kci->stext = round_down(kci->stext, page_size);
+	else
+		kci->stext = round_down(kci->first_symbol, page_size);
+
+	if (kci->etext) {
+		kci->etext = round_up(kci->etext, page_size);
+	} else if (kci->last_symbol) {
+		kci->etext = round_up(kci->last_symbol, page_size);
+		kci->etext += page_size;
+	}
+
+	kci->first_module = round_down(kci->first_module, page_size);
+
+	if (kci->last_module_symbol) {
+		kci->last_module_symbol = round_up(kci->last_module_symbol,
+						   page_size);
+		kci->last_module_symbol += page_size;
+	}
+
+	if (!kci->stext || !kci->etext)
+		return -1;
+
+	if (kci->first_module && !kci->last_module_symbol)
+		return -1;
+
+	return kcore_copy__read_maps(elf, kci);
+}
+
+static int kcore_copy__copy_file(const char *from_dir, const char *to_dir,
+				 const char *name)
+{
+	char from_filename[PATH_MAX];
+	char to_filename[PATH_MAX];
+
+	scnprintf(from_filename, PATH_MAX, "%s/%s", from_dir, name);
+	scnprintf(to_filename, PATH_MAX, "%s/%s", to_dir, name);
+
+	return copyfile_mode(from_filename, to_filename, 0400);
+}
+
+static int kcore_copy__unlink(const char *dir, const char *name)
+{
+	char filename[PATH_MAX];
+
+	scnprintf(filename, PATH_MAX, "%s/%s", dir, name);
+
+	return unlink(filename);
+}
+
+static int kcore_copy__compare_fds(int from, int to)
+{
+	char buf_from[page_size];
+	char buf_to[page_size];
+	ssize_t ret;
+	size_t len, n;
+
+	while (1) {
+		/* Use read because mmap won't work on proc files */
+		ret = read(from, buf_from, page_size);
+		if (ret < 0)
+			return -1;
+
+		if (!ret)
+			return 0;
+
+		n = ret;
+		len = ret;
+		while (n) {
+			ret = read(to, buf_to, n);
+			if (ret <= 0)
+				return -1;
+			n -= ret;
+		}
+
+		if (memcmp(buf_from, buf_to, len))
+			return -1;
+	}
+}
+
+static int kcore_copy__compare_files(const char *from_filename,
+				     const char *to_filename)
+{
+	int from, to, err = -1;
+
+	from = open(from_filename, O_RDONLY);
+	if (from < 0)
+		return -1;
+
+	to = open(to_filename, O_RDONLY);
+	if (to < 0)
+		goto out_close_from;
+
+	err = kcore_copy__compare_fds(from, to);
+
+	close(to);
+out_close_from:
+	close(from);
+	return err;
+}
+
+static int kcore_copy__compare_file(const char *from_dir, const char *to_dir,
+				    const char *name)
+{
+	char from_filename[PATH_MAX];
+	char to_filename[PATH_MAX];
+
+	scnprintf(from_filename, PATH_MAX, "%s/%s", from_dir, name);
+	scnprintf(to_filename, PATH_MAX, "%s/%s", to_dir, name);
+
+	return kcore_copy__compare_files(from_filename, to_filename);
+}
+
+/**
+ * kcore_copy - copy kallsyms, modules and kcore from one directory to another.
+ * @from_dir: from directory
+ * @to_dir: to directory
+ *
+ * This function copies kallsyms, modules and kcore files from one directory to
+ * another.  kallsyms and modules are copied entirely.  Only code segments are
+ * copied from kcore.  It is assumed that two segments suffice: one for the
+ * kernel proper and one for all the modules.  The code segments are determined
+ * from kallsyms and modules files.  The kernel map starts at _stext or the
+ * lowest function symbol, and ends at _etext or the highest function symbol.
+ * The module map starts at the lowest module address and ends at the highest
+ * module symbol.  Start addresses are rounded down to the nearest page.  End
+ * addresses are rounded up to the nearest page.  An extra page is added to the
+ * highest kernel symbol and highest module symbol to, hopefully, encompass that
+ * symbol too.  Because it contains only code sections, the resulting kcore is
+ * unusual.  One significant peculiarity is that the mapping (start -> pgoff)
+ * is not the same for the kernel map and the modules map.  That happens because
+ * the data is copied adjacently whereas the original kcore has gaps.  Finally,
+ * kallsyms and modules files are compared with their copies to check that
+ * modules have not been loaded or unloaded while the copies were taking place.
+ *
+ * Return: %0 on success, %-1 on failure.
+ */
+int kcore_copy(const char *from_dir, const char *to_dir)
+{
+	struct kcore kcore;
+	struct kcore extract;
+	size_t count = 2;
+	int index = 0, err = -1;
+	off_t offset = page_size, sz, modules_offset = 0;
+	struct kcore_copy_info kci = {0};
+	char kcore_filename[PATH_MAX];
+	char extract_filename[PATH_MAX];
+
+	if (kcore_copy__copy_file(from_dir, to_dir, "kallsyms"))
+		return -1;
+
+	if (kcore_copy__copy_file(from_dir, to_dir, "modules"))
+		goto out_unlink_kallsyms;
+
+	scnprintf(kcore_filename, PATH_MAX, "%s/kcore", from_dir);
+	scnprintf(extract_filename, PATH_MAX, "%s/kcore", to_dir);
+
+	if (kcore_open(kcore_filename, &kcore))
+		goto out_unlink_modules;
+
+	if (kcore_copy__calc_maps(from_dir, kcore.elf, &kci))
+		goto out_kcore_close;
+
+	if (kcore_new(extract_filename, &extract, kcore.elfclass, false))
+		goto out_kcore_close;
+
+	if (!kci.modules_map.addr)
+		count -= 1;
+
+	if (kcore_copy_hdr(&kcore, &extract, count))
+		goto out_extract_close;
+
+	if (kcore_add_phdr(&extract, index++, offset, kci.kernel_map.addr,
+			   kci.kernel_map.len))
+		goto out_extract_close;
+
+	if (kci.modules_map.addr) {
+		modules_offset = offset + kci.kernel_map.len;
+		if (kcore_add_phdr(&extract, index, modules_offset,
+				   kci.modules_map.addr, kci.modules_map.len))
+			goto out_extract_close;
+	}
+
+	sz = kcore_write(&extract);
+	if (sz < 0 || sz > offset)
+		goto out_extract_close;
+
+	if (copy_bytes(kcore.fd, kci.kernel_map.offset, extract.fd, offset,
+		       kci.kernel_map.len))
+		goto out_extract_close;
+
+	if (modules_offset && copy_bytes(kcore.fd, kci.modules_map.offset,
+					 extract.fd, modules_offset,
+					 kci.modules_map.len))
+		goto out_extract_close;
+
+	if (kcore_copy__compare_file(from_dir, to_dir, "modules"))
+		goto out_extract_close;
+
+	if (kcore_copy__compare_file(from_dir, to_dir, "kallsyms"))
+		goto out_extract_close;
+
+	err = 0;
+
+out_extract_close:
+	kcore_close(&extract);
+	if (err)
+		unlink(extract_filename);
+out_kcore_close:
+	kcore_close(&kcore);
+out_unlink_modules:
+	if (err)
+		kcore_copy__unlink(to_dir, "modules");
+out_unlink_kallsyms:
+	if (err)
+		kcore_copy__unlink(to_dir, "kallsyms");
+
+	return err;
+}
+
 int create_kcore_extract(struct kcore_extract *kce)
 {
 	struct kcore kcore;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 0330163..172d25c 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -317,6 +317,12 @@ void delete_kcore_extract(struct kcore_extract *kce __maybe_unused)
 {
 }
 
+int kcore_copy(const char *from_dir __maybe_unused,
+	       const char *to_dir __maybe_unused)
+{
+	return -1;
+}
+
 void symbol__elf_init(void)
 {
 }
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2a2c581..dc5783c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -893,6 +893,47 @@ static int read_proc_modules(const char *filename, struct rb_root *modules)
 	return 0;
 }
 
+int compare_proc_modules(const char *from, const char *to)
+{
+	struct rb_root from_modules = RB_ROOT;
+	struct rb_root to_modules = RB_ROOT;
+	struct rb_node *from_node, *to_node;
+	struct module_info *from_m, *to_m;
+	int ret = -1;
+
+	if (read_proc_modules(from, &from_modules))
+		return -1;
+
+	if (read_proc_modules(to, &to_modules))
+		goto out_delete_from;
+
+	from_node = rb_first(&from_modules);
+	to_node = rb_first(&to_modules);
+	while (from_node) {
+		if (!to_node)
+			break;
+
+		from_m = rb_entry(from_node, struct module_info, rb_node);
+		to_m = rb_entry(to_node, struct module_info, rb_node);
+
+		if (from_m->start != to_m->start ||
+		    strcmp(from_m->name, to_m->name))
+			break;
+
+		from_node = rb_next(from_node);
+		to_node = rb_next(to_node);
+	}
+
+	if (!from_node && !to_node)
+		ret = 0;
+
+	delete_modules(&to_modules);
+out_delete_from:
+	delete_modules(&from_modules);
+
+	return ret;
+}
+
 static int do_validate_kcore_modules(const char *filename, struct map *map,
 				  struct map_groups *kmaps)
 {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 54f3ed0..2f1a187 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -270,4 +270,7 @@ struct kcore_extract {
 int create_kcore_extract(struct kcore_extract *kce);
 void delete_kcore_extract(struct kcore_extract *kce);
 
+int kcore_copy(const char *from_dir, const char *to_dir);
+int compare_proc_modules(const char *from, const char *to);
+
 #endif /* __PERF_SYMBOL */
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH V5 9/9] perf tools: add ability to find kcore in build-id cache
  2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
                   ` (7 preceding siblings ...)
  2013-10-08  8:45 ` [PATCH V5 8/9] perf buildid-cache: add ability to add kcore to the cache Adrian Hunter
@ 2013-10-08  8:45 ` Adrian Hunter
  8 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-08  8:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

When no vmlinux is found, tools will use kallsyms and,
if possible, kcore.  Add the ability to find kcore in
the build-id cache.

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

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index dc5783c..9049f62 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1442,6 +1442,105 @@ out:
 	return err;
 }
 
+static int find_matching_kcore(struct map *map, char *dir, size_t dir_sz)
+{
+	char kallsyms_filename[PATH_MAX];
+	struct dirent *dent;
+	int ret = -1;
+	DIR *d;
+
+	d = opendir(dir);
+	if (!d)
+		return -1;
+
+	while (1) {
+		dent = readdir(d);
+		if (!dent)
+			break;
+		if (dent->d_type != DT_DIR)
+			continue;
+		scnprintf(kallsyms_filename, sizeof(kallsyms_filename),
+			  "%s/%s/kallsyms", dir, dent->d_name);
+		if (!validate_kcore_modules(kallsyms_filename, map)) {
+			strlcpy(dir, kallsyms_filename, dir_sz);
+			ret = 0;
+			break;
+		}
+	}
+
+	closedir(d);
+
+	return ret;
+}
+
+static char *dso__find_kallsyms(struct dso *dso, struct map *map)
+{
+	u8 host_build_id[BUILD_ID_SIZE];
+	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
+	bool is_host = false;
+	char path[PATH_MAX];
+
+	if (!dso->has_build_id) {
+		/*
+		 * Last resort, if we don't have a build-id and couldn't find
+		 * any vmlinux file, try the running kernel kallsyms table.
+		 */
+		goto proc_kallsyms;
+	}
+
+	if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
+				 sizeof(host_build_id)) == 0)
+		is_host = dso__build_id_equal(dso, host_build_id);
+
+	build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+
+	/* Use /proc/kallsyms if possible */
+	if (is_host) {
+		DIR *d;
+		int fd;
+
+		/* If no cached kcore go with /proc/kallsyms */
+		scnprintf(path, sizeof(path), "%s/[kernel.kcore]/%s",
+			  buildid_dir, sbuild_id);
+		d = opendir(path);
+		if (!d)
+			goto proc_kallsyms;
+		closedir(d);
+
+		/*
+		 * Do not check the build-id cache, until we know we cannot use
+		 * /proc/kcore.
+		 */
+		fd = open("/proc/kcore", O_RDONLY);
+		if (fd != -1) {
+			close(fd);
+			/* If module maps match go with /proc/kallsyms */
+			if (!validate_kcore_modules("/proc/kallsyms", map))
+				goto proc_kallsyms;
+		}
+
+		/* Find kallsyms in build-id cache with kcore */
+		if (!find_matching_kcore(map, path, sizeof(path)))
+			return strdup(path);
+
+		goto proc_kallsyms;
+	}
+
+	scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
+		  buildid_dir, sbuild_id);
+
+	if (access(path, F_OK)) {
+		pr_err("No kallsyms or vmlinux with build-id %s was found\n",
+		       sbuild_id);
+		return NULL;
+	}
+
+	return strdup(path);
+
+proc_kallsyms:
+	return strdup("/proc/kallsyms");
+}
+
 static int dso__load_kernel_sym(struct dso *dso, struct map *map,
 				symbol_filter_t filter)
 {
@@ -1490,51 +1589,11 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map,
 	if (symbol_conf.symfs[0] != 0)
 		return -1;
 
-	/*
-	 * Say the kernel DSO was created when processing the build-id header table,
-	 * we have a build-id, so check if it is the same as the running kernel,
-	 * using it if it is.
-	 */
-	if (dso->has_build_id) {
-		u8 kallsyms_build_id[BUILD_ID_SIZE];
-		char sbuild_id[BUILD_ID_SIZE * 2 + 1];
-
-		if (sysfs__read_build_id("/sys/kernel/notes", kallsyms_build_id,
-					 sizeof(kallsyms_build_id)) == 0) {
-			if (dso__build_id_equal(dso, kallsyms_build_id)) {
-				kallsyms_filename = "/proc/kallsyms";
-				goto do_kallsyms;
-			}
-		}
-		/*
-		 * Now look if we have it on the build-id cache in
-		 * $HOME/.debug/[kernel.kallsyms].
-		 */
-		build_id__sprintf(dso->build_id, sizeof(dso->build_id),
-				  sbuild_id);
-
-		if (asprintf(&kallsyms_allocated_filename,
-			     "%s/.debug/[kernel.kallsyms]/%s",
-			     getenv("HOME"), sbuild_id) == -1) {
-			pr_err("Not enough memory for kallsyms file lookup\n");
-			return -1;
-		}
-
-		kallsyms_filename = kallsyms_allocated_filename;
+	kallsyms_allocated_filename = dso__find_kallsyms(dso, map);
+	if (!kallsyms_allocated_filename)
+		return -1;
 
-		if (access(kallsyms_filename, F_OK)) {
-			pr_err("No kallsyms or vmlinux with build-id %s "
-			       "was found\n", sbuild_id);
-			free(kallsyms_allocated_filename);
-			return -1;
-		}
-	} else {
-		/*
-		 * Last resort, if we don't have a build-id and couldn't find
-		 * any vmlinux file, try the running kernel kallsyms table.
-		 */
-		kallsyms_filename = "/proc/kallsyms";
-	}
+	kallsyms_filename = kallsyms_allocated_filename;
 
 do_kallsyms:
 	err = dso__load_kallsyms(dso, kallsyms_filename, map, filter);
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-08  8:45 ` [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore Adrian Hunter
@ 2013-10-08 14:02   ` Jiri Olsa
  2013-10-09  7:33     ` Adrian Hunter
  2013-10-08 15:56   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2013-10-08 14:02 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:
> objdump fails to annotate module symbols when looking
> at kcore.  Workaround this by extracting object code
> from kcore and putting it in a temporary file for
> objdump to use instead.  The temporary file is created
> to look like kcore but contains only the function
> being disassembled.

Excited to ses this one, but looks like I'm hitting some
issue.  All annotation starts for me like this:

                                                                                                       ▒
       │              Disassembly of section load0:                                                            ▒
       │                                                                                                       ▒
       │              ffffffff815eee80 <load0>:                                                                ◆
  9.33 │ffffffff815eee80:   data32 data32 data32 xchg %ax,%ax                                                  


which does not seem right

thanks,
jirka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-08  8:45 ` [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore Adrian Hunter
  2013-10-08 14:02   ` Jiri Olsa
@ 2013-10-08 15:56   ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2013-10-08 15:56 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
	Stephane Eranian

Em Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter escreveu:
> objdump fails to annotate module symbols when looking
> at kcore.  Workaround this by extracting object code
> from kcore and putting it in a temporary file for
> objdump to use instead.  The temporary file is created
> to look like kcore but contains only the function
> being disassembled.

While waiting for the discussion with Jiri to continue... See below,
applies for other patches in this series.

"perf symbols: Make a separate function to parse /proc/modules"

was applied, pushed to acme/perf/core now.

Thanks,

- Arnaldo
 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/util/annotate.c       |  21 ++++
>  tools/perf/util/symbol-elf.c     | 221 +++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/symbol-minimal.c |   9 ++
>  tools/perf/util/symbol.h         |  14 +++
>  4 files changed, 265 insertions(+)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index f7bdc01..46746b8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -879,6 +879,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
>  	FILE *file;
>  	int err = 0;
>  	char symfs_filename[PATH_MAX];
> +	struct kcore_extract kce;
> +	bool delete_extract = false;
>  
>  	if (filename) {
>  		snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
> @@ -940,6 +942,23 @@ fallback:
>  	pr_debug("annotating [%p] %30s : [%p] %30s\n",
>  		 dso, dso->long_name, sym, sym->name);
>  
> +	if (dso__is_kcore(dso)) {
> +		kce.kcore_filename = symfs_filename;
> +		kce.addr = map__rip_2objdump(map, sym->start);
> +		kce.offs = sym->start;
> +		kce.len = sym->end + 1 - sym->start;
> +		if (!create_kcore_extract(&kce)) {
> +			delete_extract = true;
> +			strlcpy(symfs_filename, kce.extract_filename,
> +				sizeof(symfs_filename));
> +			if (free_filename) {
> +				free(filename);
> +				free_filename = false;
> +			}
> +			filename = symfs_filename;
> +		}
> +	}
> +
>  	snprintf(command, sizeof(command),
>  		 "%s %s%s --start-address=0x%016" PRIx64
>  		 " --stop-address=0x%016" PRIx64
> @@ -972,6 +991,8 @@ fallback:
>  
>  	pclose(file);
>  out_free_filename:
> +	if (delete_extract)
> +		delete_kcore_extract(&kce);
>  	if (free_filename)
>  		free(filename);
>  	return err;
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index a7b9ab5..79b27fc7 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1002,6 +1002,227 @@ int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
>  	return err;
>  }
>  
> +static int copy_bytes(int from, off_t from_offs, int to, off_t to_offs, u64 len)
> +{
> +	char buf[page_size];
> +	ssize_t r;
> +	size_t n;
> +
> +	if (lseek(to, to_offs, SEEK_SET) != to_offs)
> +		return -1;
> +
> +	if (lseek(from, from_offs, SEEK_SET) != from_offs)
> +		return -1;
> +
> +	while (len) {
> +		n = sizeof(buf);
> +		if (len < n)
> +			n = len;
> +		/* Use read because mmap won't work on proc files */
> +		r = read(from, buf, n);
> +		if (r < 0)
> +			return -1;
> +		if (!r)
> +			break;
> +		n = r;
> +		r = write(to, buf, n);
> +		if (r < 0)
> +			return -1;
> +		if ((size_t)r != n)
> +			return -1;
> +		len -= n;
> +	}
> +	return 0;
> +}
> +
> +struct kcore {
> +	int fd;
> +	int elfclass;
> +	Elf *elf;
> +	GElf_Ehdr ehdr;
> +};
> +
> +static int kcore_open(const char *filename, struct kcore *kcore)

The style, since you almost matched it, is to separate the class (kcore)
from the method (open) with double underscores, and also to have as the
first parameter the class instance being acted upon (kcore), so:

static int kcore__open(struct kcore *kcore, const char *filename)

> +{
> +	GElf_Ehdr *ehdr;
> +
> +	kcore->fd = open(filename, O_RDONLY);
> +	if (kcore->fd == -1)
> +		return -1;
> +
> +	kcore->elf = elf_begin(kcore->fd, ELF_C_READ, NULL);
> +	if (!kcore->elf)
> +		goto out_close;
> +
> +	kcore->elfclass = gelf_getclass(kcore->elf);
> +	if (kcore->elfclass == ELFCLASSNONE)
> +		goto out_end;
> +
> +	ehdr = gelf_getehdr(kcore->elf, &kcore->ehdr);
> +	if (!ehdr)
> +		goto out_end;
> +
> +	return 0;

Other than the above comment, the error handling is done in the expected
style, with forward gotos, etc, great!

> +out_end:
> +	elf_end(kcore->elf);
> +out_close:
> +	close(kcore->fd);
> +	return -1;
> +}
> +
> +static int kcore_new(char *filename, struct kcore *kcore, int elfclass,
> +		     bool temp)
> +{
> +	GElf_Ehdr *ehdr;

Normally when we receive an instance to just init its members, we call
it "init", so this method would be:

static int kcore__init(struct kcore *kcore, const char *filename,
		       int elfclass, bool temp)

See, for instance, machine__init(), etc

> +	kcore->elfclass = elfclass;
> +
> +	if (temp)
> +		kcore->fd = mkstemp(filename);
> +	else
> +		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
> +	if (kcore->fd == -1)
> +		return -1;
> +
> +	kcore->elf = elf_begin(kcore->fd, ELF_C_WRITE, NULL);
> +	if (!kcore->elf)
> +		goto out_close;
> +
> +	if (!gelf_newehdr(kcore->elf, elfclass))
> +		goto out_end;
> +
> +	ehdr = gelf_getehdr(kcore->elf, &kcore->ehdr);
> +	if (!ehdr)
> +		goto out_end;
> +
> +	return 0;
> +
> +out_end:
> +	elf_end(kcore->elf);
> +out_close:
> +	close(kcore->fd);
> +	unlink(filename);
> +	return -1;
> +}
> +
> +static void kcore_close(struct kcore *kcore)

static void kcore__close(struct kcore *kcore)

> +{
> +	elf_end(kcore->elf);
> +	close(kcore->fd);
> +}
> +
> +static int kcore_copy_hdr(struct kcore *from, struct kcore *to, size_t count)

just the __

> +{
> +	GElf_Ehdr *ehdr = &to->ehdr;
> +	GElf_Ehdr *kehdr = &from->ehdr;
> +
> +	memcpy(ehdr->e_ident, kehdr->e_ident, EI_NIDENT);
> +	ehdr->e_type      = kehdr->e_type;
> +	ehdr->e_machine   = kehdr->e_machine;
> +	ehdr->e_version   = kehdr->e_version;
> +	ehdr->e_entry     = 0;
> +	ehdr->e_shoff     = 0;
> +	ehdr->e_flags     = kehdr->e_flags;
> +	ehdr->e_phnum     = count;
> +	ehdr->e_shentsize = 0;
> +	ehdr->e_shnum     = 0;
> +	ehdr->e_shstrndx  = 0;
> +
> +	if (from->elfclass == ELFCLASS32) {
> +		ehdr->e_phoff     = sizeof(Elf32_Ehdr);
> +		ehdr->e_ehsize    = sizeof(Elf32_Ehdr);
> +		ehdr->e_phentsize = sizeof(Elf32_Phdr);
> +	} else {
> +		ehdr->e_phoff     = sizeof(Elf64_Ehdr);
> +		ehdr->e_ehsize    = sizeof(Elf64_Ehdr);
> +		ehdr->e_phentsize = sizeof(Elf64_Phdr);
> +	}
> +
> +	if (!gelf_update_ehdr(to->elf, ehdr))
> +		return -1;
> +
> +	if (!gelf_newphdr(to->elf, count))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int kcore_add_phdr(struct kcore *kcore, int index, off_t offset,
> +			  u64 addr, u64 len)

ditto, and here the kcore pointer is the first, perfect.

> +{
> +	GElf_Phdr gphdr;
> +	GElf_Phdr *phdr;
> +
> +	phdr = gelf_getphdr(kcore->elf, index, &gphdr);
> +	if (!phdr)
> +		return -1;
> +
> +	phdr->p_type	= PT_LOAD;
> +	phdr->p_flags	= PF_R | PF_W | PF_X;
> +	phdr->p_offset	= offset;
> +	phdr->p_vaddr	= addr;
> +	phdr->p_paddr	= 0;
> +	phdr->p_filesz	= len;
> +	phdr->p_memsz	= len;
> +	phdr->p_align	= page_size;
> +
> +	if (!gelf_update_phdr(kcore->elf, index, phdr))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static off_t kcore_write(struct kcore *kcore)
> +{
> +	return elf_update(kcore->elf, ELF_C_WRITE);
> +}
> +
> +int create_kcore_extract(struct kcore_extract *kce)
> +{
> +	struct kcore kcore;
> +	struct kcore extract;
> +	size_t count = 1;
> +	int index = 0, err = -1;
> +	off_t offset = page_size, sz;
> +
> +	if (kcore_open(kce->kcore_filename, &kcore))
> +		return -1;
> +
> +	strcpy(kce->extract_filename, PERF_KCORE_EXTRACT);
> +	if (kcore_new(kce->extract_filename, &extract, kcore.elfclass, true))
> +		goto out_kcore_close;
> +
> +	if (kcore_copy_hdr(&kcore, &extract, count))
> +		goto out_extract_close;
> +
> +	if (kcore_add_phdr(&extract, index, offset, kce->addr, kce->len))
> +		goto out_extract_close;
> +
> +	sz = kcore_write(&extract);
> +	if (sz < 0 || sz > offset)
> +		goto out_extract_close;
> +
> +	if (copy_bytes(kcore.fd, kce->offs, extract.fd, offset, kce->len))
> +		goto out_extract_close;
> +
> +	err = 0;
> +
> +out_extract_close:
> +	kcore_close(&extract);
> +	if (err)
> +		unlink(kce->extract_filename);
> +out_kcore_close:
> +	kcore_close(&kcore);
> +
> +	return err;
> +}
> +
> +void delete_kcore_extract(struct kcore_extract *kce)
> +{
> +	unlink(kce->extract_filename);
> +}
> +
>  void symbol__elf_init(void)
>  {
>  	elf_version(EV_CURRENT);
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 3a802c3..0330163 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -308,6 +308,15 @@ int file__read_maps(int fd __maybe_unused, bool exe __maybe_unused,
>  	return -1;
>  }
>  
> +int create_kcore_extract(struct kcore_extract *kce __maybe_unused)
> +{
> +	return -1;
> +}
> +
> +void delete_kcore_extract(struct kcore_extract *kce __maybe_unused)
> +{
> +}
> +
>  void symbol__elf_init(void)
>  {
>  }
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index a2543f0..54f3ed0 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -256,4 +256,18 @@ typedef int (*mapfn_t)(u64 start, u64 len, u64 pgoff, void *data);
>  int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
>  		    bool *is_64_bit);
>  
> +#define PERF_KCORE_EXTRACT "/tmp/perf-kcore-XXXXXX"
> +
> +struct kcore_extract {
> +	char *kcore_filename;
> +	u64 addr;
> +	u64 offs;
> +	u64 len;
> +	char extract_filename[sizeof(PERF_KCORE_EXTRACT)];
> +	int fd;
> +};
> +
> +int create_kcore_extract(struct kcore_extract *kce);
> +void delete_kcore_extract(struct kcore_extract *kce);
> +
>  #endif /* __PERF_SYMBOL */
> -- 
> 1.7.11.7

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-08 14:02   ` Jiri Olsa
@ 2013-10-09  7:33     ` Adrian Hunter
  2013-10-09 10:12       ` Jiri Olsa
  0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2013-10-09  7:33 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On 08/10/13 17:02, Jiri Olsa wrote:
> On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:
>> objdump fails to annotate module symbols when looking
>> at kcore.  Workaround this by extracting object code
>> from kcore and putting it in a temporary file for
>> objdump to use instead.  The temporary file is created
>> to look like kcore but contains only the function
>> being disassembled.
> 
> Excited to ses this one, but looks like I'm hitting some
> issue.  All annotation starts for me like this:
> 
>                                                                                                        ▒
>        │              Disassembly of section load0:                                                            ▒
>        │                                                                                                       ▒
>        │              ffffffff815eee80 <load0>:                                                                ◆
>   9.33 │ffffffff815eee80:   data32 data32 data32 xchg %ax,%ax                                                  
> 
> 
> which does not seem right

Can you tell me the commits of the kernel and perf tools you
were using, plus the commands and what symbol it was?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-09  7:33     ` Adrian Hunter
@ 2013-10-09 10:12       ` Jiri Olsa
  2013-10-09 10:38         ` Adrian Hunter
  0 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2013-10-09 10:12 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On Wed, Oct 09, 2013 at 10:33:25AM +0300, Adrian Hunter wrote:
> On 08/10/13 17:02, Jiri Olsa wrote:
> > On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:
> >> objdump fails to annotate module symbols when looking
> >> at kcore.  Workaround this by extracting object code
> >> from kcore and putting it in a temporary file for
> >> objdump to use instead.  The temporary file is created
> >> to look like kcore but contains only the function
> >> being disassembled.
> > 
> > Excited to ses this one, but looks like I'm hitting some
> > issue.  All annotation starts for me like this:
> > 
> >                                                                                                        ▒
> >        │              Disassembly of section load0:                                                            ▒
> >        │                                                                                                       ▒
> >        │              ffffffff815eee80 <load0>:                                                                ◆
> >   9.33 │ffffffff815eee80:   data32 data32 data32 xchg %ax,%ax                                                  
> > 
> > 
> > which does not seem right
> 
> Can you tell me the commits of the kernel and perf tools you
> were using, plus the commands and what symbol it was?

kernel: 3.9.10-100.fc17.x86_64
perf:   latest acme's perf/core (06de626 perf evlist: Fix perf_evlist__mmap_read event overflow )
        plus your V5 patches

commands:
  sudo ./perf record -e cycles:k -a 
  sudo ./perf report

---
Samples: 2K of event 'cycles:k', Event count (approx.): 445188286                                               
 14.73%          swapper  [kernel.kallsyms]   [k] intel_idle                                                   ◆
  3.19%                X  [kernel.kallsyms]   [k] smp_call_function_many                                       ▒
  1.58%                X  [kernel.kallsyms]   [k] i915_gem_write_fence__ipi                                    ▒
  1.58%          swapper  [kernel.kallsyms]   [k] iwl_trans_pcie_read32                                        ▒


annotation of 1st 4 symbols:

---
intel_idle  /proc/kcore                                                                                         
       │                                                                                                       ▒
       │                                                                                                       ▒
       │                                                                                                       ▒
       │     Disassembly of section load0:                                                                     ▒
       │                                                                                                       ▒
       │     ffffffff8135f490 <load0>:                                                                         ▒
  1.18 │       data32 data32 data32 xchg %ax,%ax                                                               ▒


---
smp_call_function_many  /proc/kcore                                                                             
       │                                                                                                       ◆
       │                                                                                                       ▒
       │                                                                                                       ▒
       │     Disassembly of section load0:                                                                     ▒
       │                                                                                                       ▒
       │     ffffffff810bc270 <load0>:                                                                         ▒
       │       data32 data32 data32 xchg %ax,%ax                                                               ▒

---
i915_gem_write_fence__ipi  /proc/kcore                                                                          
       │
       │
       │
       │    Disassembly of section load0:
       │
       │    ffffffffa0086630 <load0>:
       │      data32 data32 data32 xchg %ax,%ax

---
iwl_trans_pcie_read32  /proc/kcore                                                                              
       │
       │
       │
       │    Disassembly of section load0:
       │
       │    ffffffffa0414a50 <load0>:
       │      data32 data32 data32 xchg %ax,%ax


the rest of the instruction decode differs.. just the first
line is same for all

addresses seem ok:

[jolsa@krava perf]$ egrep 'ffffffff8135f490|ffffffff810bc270|ffffffffa0086630|ffffffffa0414a50' /proc/kallsyms 
ffffffff810bc270 T smp_call_function_many
ffffffff8135f490 t intel_idle
ffffffffa0414a50 t iwl_trans_pcie_read32        [iwlwifi]
ffffffffa0086630 t i915_gem_write_fence__ipi    [i915]

so.. the name of the section, name of the <function> plus the first
instruction decode seem wrong.. I can see that in every symbol I
annotate in the report and in annotate command as well.

jirka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-09 10:12       ` Jiri Olsa
@ 2013-10-09 10:38         ` Adrian Hunter
  2013-10-09 12:16           ` Jiri Olsa
  0 siblings, 1 reply; 18+ messages in thread
From: Adrian Hunter @ 2013-10-09 10:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On 09/10/13 13:12, Jiri Olsa wrote:
> On Wed, Oct 09, 2013 at 10:33:25AM +0300, Adrian Hunter wrote:
>> On 08/10/13 17:02, Jiri Olsa wrote:
>>> On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:
>>>> objdump fails to annotate module symbols when looking
>>>> at kcore.  Workaround this by extracting object code
>>>> from kcore and putting it in a temporary file for
>>>> objdump to use instead.  The temporary file is created
>>>> to look like kcore but contains only the function
>>>> being disassembled.
>>>
>>> Excited to ses this one, but looks like I'm hitting some
>>> issue.  All annotation starts for me like this:
>>>
>>>                                                                                                        ▒
>>>        │              Disassembly of section load0:                                                            ▒
>>>        │                                                                                                       ▒
>>>        │              ffffffff815eee80 <load0>:                                                                ◆
>>>   9.33 │ffffffff815eee80:   data32 data32 data32 xchg %ax,%ax                                                  
>>>
>>>
>>> which does not seem right
>>
>> Can you tell me the commits of the kernel and perf tools you
>> were using, plus the commands and what symbol it was?
> 
> kernel: 3.9.10-100.fc17.x86_64
> perf:   latest acme's perf/core (06de626 perf evlist: Fix perf_evlist__mmap_read event overflow )
>         plus your V5 patches
> 
> commands:
>   sudo ./perf record -e cycles:k -a 
>   sudo ./perf report
> 
> ---
> Samples: 2K of event 'cycles:k', Event count (approx.): 445188286                                               
>  14.73%          swapper  [kernel.kallsyms]   [k] intel_idle                                                   ◆
>   3.19%                X  [kernel.kallsyms]   [k] smp_call_function_many                                       ▒
>   1.58%                X  [kernel.kallsyms]   [k] i915_gem_write_fence__ipi                                    ▒
>   1.58%          swapper  [kernel.kallsyms]   [k] iwl_trans_pcie_read32                                        ▒
> 
> 
> annotation of 1st 4 symbols:
> 
> ---
> intel_idle  /proc/kcore                                                                                         
>        │                                                                                                       ▒
>        │                                                                                                       ▒
>        │                                                                                                       ▒
>        │     Disassembly of section load0:                                                                     ▒
>        │                                                                                                       ▒
>        │     ffffffff8135f490 <load0>:                                                                         ▒
>   1.18 │       data32 data32 data32 xchg %ax,%ax                                                               ▒
> 
> 
> ---
> smp_call_function_many  /proc/kcore                                                                             
>        │                                                                                                       ◆
>        │                                                                                                       ▒
>        │                                                                                                       ▒
>        │     Disassembly of section load0:                                                                     ▒
>        │                                                                                                       ▒
>        │     ffffffff810bc270 <load0>:                                                                         ▒
>        │       data32 data32 data32 xchg %ax,%ax                                                               ▒
> 
> ---
> i915_gem_write_fence__ipi  /proc/kcore                                                                          
>        │
>        │
>        │
>        │    Disassembly of section load0:
>        │
>        │    ffffffffa0086630 <load0>:
>        │      data32 data32 data32 xchg %ax,%ax
> 
> ---
> iwl_trans_pcie_read32  /proc/kcore                                                                              
>        │
>        │
>        │
>        │    Disassembly of section load0:
>        │
>        │    ffffffffa0414a50 <load0>:
>        │      data32 data32 data32 xchg %ax,%ax
> 
> 
> the rest of the instruction decode differs.. just the first
> line is same for all
> 
> addresses seem ok:
> 
> [jolsa@krava perf]$ egrep 'ffffffff8135f490|ffffffff810bc270|ffffffffa0086630|ffffffffa0414a50' /proc/kallsyms 
> ffffffff810bc270 T smp_call_function_many
> ffffffff8135f490 t intel_idle
> ffffffffa0414a50 t iwl_trans_pcie_read32        [iwlwifi]
> ffffffffa0086630 t i915_gem_write_fence__ipi    [i915]
> 
> so.. the name of the section, name of the <function> plus the first
> instruction decode seem wrong.. I can see that in every symbol I
> annotate in the report and in annotate command as well.

If you use the --asm-raw option you can see the bytes:

	66 66 66 90 

That looks like a "nop" e.g. K8_NOP4 in arch/x86/include/asm/nops.h

	/*
	 * Define nops for use with alternative() and for tracing.
	 *
	 * *_NOP5_ATOMIC must be a single instruction.
	 */

	#define NOP_DS_PREFIX 0x3e

	/* generic versions from gas
	   1: nop
	   the following instructions are NOT nops in 64-bit mode,
	   for 64-bit mode use K8 or P6 nops instead
	   2: movl %esi,%esi
	   3: leal 0x00(%esi),%esi
	   4: leal 0x00(,%esi,1),%esi
	   6: leal 0x00000000(%esi),%esi
	   7: leal 0x00000000(,%esi,1),%esi
	*/
	#define GENERIC_NOP1 0x90
	#define GENERIC_NOP2 0x89,0xf6
	#define GENERIC_NOP3 0x8d,0x76,0x00
	#define GENERIC_NOP4 0x8d,0x74,0x26,0x00
	#define GENERIC_NOP5 GENERIC_NOP1,GENERIC_NOP4
	#define GENERIC_NOP6 0x8d,0xb6,0x00,0x00,0x00,0x00
	#define GENERIC_NOP7 0x8d,0xb4,0x26,0x00,0x00,0x00,0x00
	#define GENERIC_NOP8 GENERIC_NOP1,GENERIC_NOP7
	#define GENERIC_NOP5_ATOMIC NOP_DS_PREFIX,GENERIC_NOP4

	/* Opteron 64bit nops
	   1: nop
	   2: osp nop
	   3: osp osp nop
	   4: osp osp osp nop
	*/
	#define K8_NOP1 GENERIC_NOP1
	#define K8_NOP2 0x66,K8_NOP1
	#define K8_NOP3 0x66,K8_NOP2
	#define K8_NOP4 0x66,K8_NOP3
	#define K8_NOP5 K8_NOP3,K8_NOP2
	#define K8_NOP6 K8_NOP3,K8_NOP3
	#define K8_NOP7 K8_NOP4,K8_NOP3
	#define K8_NOP8 K8_NOP4,K8_NOP4
	#define K8_NOP5_ATOMIC 0x66,K8_NOP4

I think what you see is correct.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-09 10:38         ` Adrian Hunter
@ 2013-10-09 12:16           ` Jiri Olsa
  2013-10-09 12:43             ` Adrian Hunter
  0 siblings, 1 reply; 18+ messages in thread
From: Jiri Olsa @ 2013-10-09 12:16 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On Wed, Oct 09, 2013 at 01:38:04PM +0300, Adrian Hunter wrote:
> On 09/10/13 13:12, Jiri Olsa wrote:
> > On Wed, Oct 09, 2013 at 10:33:25AM +0300, Adrian Hunter wrote:
> >> On 08/10/13 17:02, Jiri Olsa wrote:
> >>> On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:

SNIP

> > 
> > so.. the name of the section, name of the <function> plus the first
> > instruction decode seem wrong.. I can see that in every symbol I
> > annotate in the report and in annotate command as well.
> 
> If you use the --asm-raw option you can see the bytes:
> 
> 	66 66 66 90 
> 
> That looks like a "nop" e.g. K8_NOP4 in arch/x86/include/asm/nops.h
> 

hum, wierd for all those functions to start with nop

ok, how about the '<load0>' function name?

thanks,
jirka

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore
  2013-10-09 12:16           ` Jiri Olsa
@ 2013-10-09 12:43             ` Adrian Hunter
  0 siblings, 0 replies; 18+ messages in thread
From: Adrian Hunter @ 2013-10-09 12:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, linux-kernel,
	David Ahern, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

On 09/10/13 15:16, Jiri Olsa wrote:
> On Wed, Oct 09, 2013 at 01:38:04PM +0300, Adrian Hunter wrote:
>> On 09/10/13 13:12, Jiri Olsa wrote:
>>> On Wed, Oct 09, 2013 at 10:33:25AM +0300, Adrian Hunter wrote:
>>>> On 08/10/13 17:02, Jiri Olsa wrote:
>>>>> On Tue, Oct 08, 2013 at 11:45:50AM +0300, Adrian Hunter wrote:
> 
> SNIP
> 
>>>
>>> so.. the name of the section, name of the <function> plus the first
>>> instruction decode seem wrong.. I can see that in every symbol I
>>> annotate in the report and in annotate command as well.
>>
>> If you use the --asm-raw option you can see the bytes:
>>
>> 	66 66 66 90 
>>
>> That looks like a "nop" e.g. K8_NOP4 in arch/x86/include/asm/nops.h
>>
> 
> hum, wierd for all those functions to start with nop

I guess it is for ftrace.

> 
> ok, how about the '<load0>' function name?

That is because kcore (and its copies) have no symbols
so objdump has nothing meaningful to put there.  The function
name and file appear at the top of the screen instead.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf symbols: Make a separate function to parse / proc/modules
  2013-10-08  8:45 ` [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules Adrian Hunter
@ 2013-10-15  5:31   ` tip-bot for Adrian Hunter
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Adrian Hunter @ 2013-10-15  5:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra,
	efault, namhyung, jolsa, fweisbec, adrian.hunter, dsahern, tglx

Commit-ID:  316d70d6dbde540b275289563cbddd9f0c903fc6
Gitweb:     http://git.kernel.org/tip/316d70d6dbde540b275289563cbddd9f0c903fc6
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Tue, 8 Oct 2013 11:45:48 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 11 Oct 2013 12:17:57 -0300

perf symbols: Make a separate function to parse /proc/modules

Make a separate function to parse /proc/modules so that it can be
reused.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.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/1381221956-16699-2-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 67 +++++++++++++----------------------------------
 tools/perf/util/symbol.c  | 58 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h  |  3 +++
 3 files changed, 79 insertions(+), 49 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 901397a..6b861ae 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -793,12 +793,22 @@ static int machine__set_modules_path(struct machine *machine)
 	return map_groups__set_modules_path_dir(&machine->kmaps, modules_path);
 }
 
-static int machine__create_modules(struct machine *machine)
+static int machine__create_module(void *arg, const char *name, u64 start)
 {
-	char *line = NULL;
-	size_t n;
-	FILE *file;
+	struct machine *machine = arg;
 	struct map *map;
+
+	map = machine__new_module(machine, start, name);
+	if (map == NULL)
+		return -1;
+
+	dso__kernel_module_get_build_id(map->dso, machine->root_dir);
+
+	return 0;
+}
+
+static int machine__create_modules(struct machine *machine)
+{
 	const char *modules;
 	char path[PATH_MAX];
 
@@ -812,56 +822,15 @@ static int machine__create_modules(struct machine *machine)
 	if (symbol__restricted_filename(modules, "/proc/modules"))
 		return -1;
 
-	file = fopen(modules, "r");
-	if (file == NULL)
+	if (modules__parse(modules, machine, machine__create_module))
 		return -1;
 
-	while (!feof(file)) {
-		char name[PATH_MAX];
-		u64 start;
-		char *sep;
-		int line_len;
-
-		line_len = getline(&line, &n, file);
-		if (line_len < 0)
-			break;
-
-		if (!line)
-			goto out_failure;
-
-		line[--line_len] = '\0'; /* \n */
-
-		sep = strrchr(line, 'x');
-		if (sep == NULL)
-			continue;
-
-		hex2u64(sep + 1, &start);
-
-		sep = strchr(line, ' ');
-		if (sep == NULL)
-			continue;
-
-		*sep = '\0';
-
-		snprintf(name, sizeof(name), "[%s]", line);
-		map = machine__new_module(machine, start, name);
-		if (map == NULL)
-			goto out_delete_line;
-		dso__kernel_module_get_build_id(map->dso, machine->root_dir);
-	}
+	if (!machine__set_modules_path(machine))
+		return 0;
 
-	free(line);
-	fclose(file);
+	pr_debug("Problems setting modules path maps, continuing anyway...\n");
 
-	if (machine__set_modules_path(machine) < 0) {
-		pr_debug("Problems setting modules path maps, continuing anyway...\n");
-	}
 	return 0;
-
-out_delete_line:
-	free(line);
-out_failure:
-	return -1;
 }
 
 int machine__create_kernel_maps(struct machine *machine)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 48c3879..5fd9513 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -500,6 +500,64 @@ out_failure:
 	return -1;
 }
 
+int modules__parse(const char *filename, void *arg,
+		   int (*process_module)(void *arg, const char *name,
+					 u64 start))
+{
+	char *line = NULL;
+	size_t n;
+	FILE *file;
+	int err = 0;
+
+	file = fopen(filename, "r");
+	if (file == NULL)
+		return -1;
+
+	while (1) {
+		char name[PATH_MAX];
+		u64 start;
+		char *sep;
+		ssize_t line_len;
+
+		line_len = getline(&line, &n, file);
+		if (line_len < 0) {
+			if (feof(file))
+				break;
+			err = -1;
+			goto out;
+		}
+
+		if (!line) {
+			err = -1;
+			goto out;
+		}
+
+		line[--line_len] = '\0'; /* \n */
+
+		sep = strrchr(line, 'x');
+		if (sep == NULL)
+			continue;
+
+		hex2u64(sep + 1, &start);
+
+		sep = strchr(line, ' ');
+		if (sep == NULL)
+			continue;
+
+		*sep = '\0';
+
+		scnprintf(name, sizeof(name), "[%s]", line);
+
+		err = process_module(arg, name, start);
+		if (err)
+			break;
+	}
+out:
+	free(line);
+	fclose(file);
+	return err;
+}
+
 struct process_kallsyms_args {
 	struct map *map;
 	struct dso *dso;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 9b8b213..2d3eb43 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -223,6 +223,9 @@ int sysfs__read_build_id(const char *filename, void *bf, size_t size);
 int kallsyms__parse(const char *filename, void *arg,
 		    int (*process_symbol)(void *arg, const char *name,
 					  char type, u64 start));
+int modules__parse(const char *filename, void *arg,
+		   int (*process_module)(void *arg, const char *name,
+					 u64 start));
 int filename__read_debuglink(const char *filename, char *debuglink,
 			     size_t size);
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-10-15  5:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-08  8:45 [PATCH V5 0/9] perf tools: kcore improvements Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 1/9] perf tools: make a separate function to parse /proc/modules Adrian Hunter
2013-10-15  5:31   ` [tip:perf/core] perf symbols: Make a separate function to parse / proc/modules tip-bot for Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 2/9] perf tools: validate kcore module addresses Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 3/9] perf tools: workaround objdump difficulties with kcore Adrian Hunter
2013-10-08 14:02   ` Jiri Olsa
2013-10-09  7:33     ` Adrian Hunter
2013-10-09 10:12       ` Jiri Olsa
2013-10-09 10:38         ` Adrian Hunter
2013-10-09 12:16           ` Jiri Olsa
2013-10-09 12:43             ` Adrian Hunter
2013-10-08 15:56   ` Arnaldo Carvalho de Melo
2013-10-08  8:45 ` [PATCH V5 4/9] perf tools: add map__find_other_map_symbol() Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 5/9] perf tools: fix annotate_browser__callq() Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 6/9] perf tools: find kcore symbols on other maps Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 7/9] perf tools: add copyfile_mode() Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 8/9] perf buildid-cache: add ability to add kcore to the cache Adrian Hunter
2013-10-08  8:45 ` [PATCH V5 9/9] perf tools: add ability to find kcore in build-id cache Adrian Hunter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).