linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/7] perf/core refactorings and improvements
@ 2015-05-28  0:05 Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 1/7] perf tools: Introduce struct maps Arnaldo Carvalho de Melo
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Andi Kleen, Borislav Petkov, David Ahern, Don Zickus,
	Frederic Weisbecker, Jiri Olsa, Michael Petlan, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

Hi Ingo,

	Please consider applying,

- Arnaldo


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

* [PATCH 1/7] perf tools: Introduce struct maps
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 2/7] perf tools: Protect accesses the map rbtrees with a rw lock Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

That for now has the maps rbtree and the list for the dead maps, that
may be still referenced from some hist_entry, etc.

This paves the way for protecting the rbtree with a lock, then refcount
the maps and finally remove the removed_maps list, as it'll not ne
anymore needed.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-fl0fa6142pj8khj97fow3uw0@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/vmlinux-kallsyms.c |  2 +-
 tools/perf/util/event.c             |  2 +-
 tools/perf/util/map.c               | 64 +++++++++++++++++++++----------------
 tools/perf/util/map.h               | 16 ++++++----
 tools/perf/util/probe-event.c       |  2 +-
 tools/perf/util/symbol.c            |  4 +--
 6 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 94ac6924df65..b34c5fc829ae 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -26,7 +26,7 @@ int test__vmlinux_matches_kallsyms(void)
 	struct map *kallsyms_map, *vmlinux_map, *map;
 	struct machine kallsyms, vmlinux;
 	enum map_type type = MAP__FUNCTION;
-	struct rb_root *maps = &vmlinux.kmaps.maps[type];
+	struct maps *maps = &vmlinux.kmaps.maps[type];
 	u64 mem_start, mem_end;
 
 	/*
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9d3bba175423..c1925968a8af 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -331,7 +331,7 @@ int perf_event__synthesize_modules(struct perf_tool *tool,
 	int rc = 0;
 	struct map *pos;
 	struct map_groups *kmaps = &machine->kmaps;
-	struct rb_root *maps = &kmaps->maps[MAP__FUNCTION];
+	struct maps *maps = &kmaps->maps[MAP__FUNCTION];
 	union perf_event *event = zalloc((sizeof(event->mmap) +
 					  machine->id_hdr_size));
 	if (event == NULL) {
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 898ab92a98dd..adf012c4d650 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -418,48 +418,58 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 	return ip + map->reloc;
 }
 
+static void maps__init(struct maps *maps)
+{
+	maps->entries = RB_ROOT;
+	INIT_LIST_HEAD(&maps->removed_maps);
+}
+
 void map_groups__init(struct map_groups *mg, struct machine *machine)
 {
 	int i;
 	for (i = 0; i < MAP__NR_TYPES; ++i) {
-		mg->maps[i] = RB_ROOT;
-		INIT_LIST_HEAD(&mg->removed_maps[i]);
+		maps__init(&mg->maps[i]);
 	}
 	mg->machine = machine;
 	atomic_set(&mg->refcnt, 1);
 }
 
-static void maps__delete(struct rb_root *maps)
+static void maps__purge(struct maps *maps)
 {
-	struct rb_node *next = rb_first(maps);
+	struct rb_root *root = &maps->entries;
+	struct rb_node *next = rb_first(root);
 
 	while (next) {
 		struct map *pos = rb_entry(next, struct map, rb_node);
 
 		next = rb_next(&pos->rb_node);
-		rb_erase(&pos->rb_node, maps);
+		rb_erase(&pos->rb_node, root);
 		map__delete(pos);
 	}
 }
 
-static void maps__delete_removed(struct list_head *maps)
+static void maps__purge_removed_maps(struct maps *maps)
 {
 	struct map *pos, *n;
 
-	list_for_each_entry_safe(pos, n, maps, node) {
+	list_for_each_entry_safe(pos, n, &maps->removed_maps, node) {
 		list_del(&pos->node);
 		map__delete(pos);
 	}
 }
 
+static void maps__exit(struct maps *maps)
+{
+	maps__purge(maps);
+	maps__purge_removed_maps(maps);
+}
+
 void map_groups__exit(struct map_groups *mg)
 {
 	int i;
 
-	for (i = 0; i < MAP__NR_TYPES; ++i) {
-		maps__delete(&mg->maps[i]);
-		maps__delete_removed(&mg->removed_maps[i]);
-	}
+	for (i = 0; i < MAP__NR_TYPES; ++i)
+		maps__exit(&mg->maps[i]);
 }
 
 bool map_groups__empty(struct map_groups *mg)
@@ -469,7 +479,7 @@ bool map_groups__empty(struct map_groups *mg)
 	for (i = 0; i < MAP__NR_TYPES; ++i) {
 		if (maps__first(&mg->maps[i]))
 			return false;
-		if (!list_empty(&mg->removed_maps[i]))
+		if (!list_empty(&mg->maps[i].removed_maps))
 			return false;
 	}
 
@@ -523,7 +533,7 @@ struct symbol *map_groups__find_symbol_by_name(struct map_groups *mg,
 {
 	struct rb_node *nd;
 
-	for (nd = rb_first(&mg->maps[type]); nd; nd = rb_next(nd)) {
+	for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) {
 		struct map *pos = rb_entry(nd, struct map, rb_node);
 		struct symbol *sym = map__find_symbol_by_name(pos, name, filter);
 
@@ -560,7 +570,7 @@ size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type,
 	size_t printed = fprintf(fp, "%s:\n", map_type__name[type]);
 	struct rb_node *nd;
 
-	for (nd = rb_first(&mg->maps[type]); nd; nd = rb_next(nd)) {
+	for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) {
 		struct map *pos = rb_entry(nd, struct map, rb_node);
 		printed += fprintf(fp, "Map:");
 		printed += map__fprintf(pos, fp);
@@ -587,7 +597,7 @@ static size_t __map_groups__fprintf_removed_maps(struct map_groups *mg,
 	struct map *pos;
 	size_t printed = 0;
 
-	list_for_each_entry(pos, &mg->removed_maps[type], node) {
+	list_for_each_entry(pos, &mg->maps[type].removed_maps, node) {
 		printed += fprintf(fp, "Map:");
 		printed += map__fprintf(pos, fp);
 		if (verbose > 1) {
@@ -617,7 +627,7 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
 int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
 				   FILE *fp)
 {
-	struct rb_root *root = &mg->maps[map->type];
+	struct rb_root *root = &mg->maps[map->type].entries;
 	struct rb_node *next = rb_first(root);
 	int err = 0;
 
@@ -671,7 +681,7 @@ move_map:
 		 * If we have references, just move them to a separate list.
 		 */
 		if (pos->referenced)
-			list_add_tail(&pos->node, &mg->removed_maps[map->type]);
+			list_add_tail(&pos->node, &mg->maps[map->type].removed_maps);
 		else
 			map__delete(pos);
 
@@ -689,7 +699,7 @@ int map_groups__clone(struct map_groups *mg,
 		      struct map_groups *parent, enum map_type type)
 {
 	struct map *map;
-	struct rb_root *maps = &parent->maps[type];
+	struct maps *maps = &parent->maps[type];
 
 	for (map = maps__first(maps); map; map = map__next(map)) {
 		struct map *new = map__clone(map);
@@ -700,9 +710,9 @@ int map_groups__clone(struct map_groups *mg,
 	return 0;
 }
 
-void maps__insert(struct rb_root *maps, struct map *map)
+void maps__insert(struct maps *maps, struct map *map)
 {
-	struct rb_node **p = &maps->rb_node;
+	struct rb_node **p = &maps->entries.rb_node;
 	struct rb_node *parent = NULL;
 	const u64 ip = map->start;
 	struct map *m;
@@ -717,17 +727,17 @@ void maps__insert(struct rb_root *maps, struct map *map)
 	}
 
 	rb_link_node(&map->rb_node, parent, p);
-	rb_insert_color(&map->rb_node, maps);
+	rb_insert_color(&map->rb_node, &maps->entries);
 }
 
-void maps__remove(struct rb_root *maps, struct map *map)
+void maps__remove(struct maps *maps, struct map *map)
 {
-	rb_erase(&map->rb_node, maps);
+	rb_erase(&map->rb_node, &maps->entries);
 }
 
-struct map *maps__find(struct rb_root *maps, u64 ip)
+struct map *maps__find(struct maps *maps, u64 ip)
 {
-	struct rb_node **p = &maps->rb_node;
+	struct rb_node **p = &maps->entries.rb_node;
 	struct rb_node *parent = NULL;
 	struct map *m;
 
@@ -745,9 +755,9 @@ struct map *maps__find(struct rb_root *maps, u64 ip)
 	return NULL;
 }
 
-struct map *maps__first(struct rb_root *maps)
+struct map *maps__first(struct maps *maps)
 {
-	struct rb_node *first = rb_first(maps);
+	struct rb_node *first = rb_first(&maps->entries);
 
 	if (first)
 		return rb_entry(first, struct map, rb_node);
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f2b27566d986..e3702fd468c5 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -58,9 +58,13 @@ struct kmap {
 	struct map_groups	*kmaps;
 };
 
+struct maps {
+	struct rb_root	 entries;
+	struct list_head removed_maps;
+};
+
 struct map_groups {
-	struct rb_root	 maps[MAP__NR_TYPES];
-	struct list_head removed_maps[MAP__NR_TYPES];
+	struct maps	 maps[MAP__NR_TYPES];
 	struct machine	 *machine;
 	atomic_t	 refcnt;
 };
@@ -162,10 +166,10 @@ void map__reloc_vmlinux(struct map *map);
 
 size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type,
 				  FILE *fp);
-void maps__insert(struct rb_root *maps, struct map *map);
-void maps__remove(struct rb_root *maps, struct map *map);
-struct map *maps__find(struct rb_root *maps, u64 addr);
-struct map *maps__first(struct rb_root *maps);
+void maps__insert(struct maps *maps, struct map *map);
+void maps__remove(struct maps *maps, struct map *map);
+struct map *maps__find(struct maps *maps, u64 addr);
+struct map *maps__first(struct maps *maps);
 struct map *map__next(struct map *map);
 void map_groups__init(struct map_groups *mg, struct machine *machine);
 void map_groups__exit(struct map_groups *mg);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 97da98481d89..32471d0839d1 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -163,7 +163,7 @@ static u64 kernel_get_symbol_address_by_name(const char *name, bool reloc)
 static struct map *kernel_get_module_map(const char *module)
 {
 	struct map_groups *grp = &host_machine->kmaps;
-	struct rb_root *maps = &grp->maps[MAP__FUNCTION];
+	struct maps *maps = &grp->maps[MAP__FUNCTION];
 	struct map *pos;
 
 	/* A file path -- this is an offline module */
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b9e3eb581884..c8a3e79c5da2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -202,7 +202,7 @@ void symbols__fixup_end(struct rb_root *symbols)
 
 void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
 {
-	struct rb_root *maps = &mg->maps[type];
+	struct maps *maps = &mg->maps[type];
 	struct map *next, *curr;
 
 	curr = maps__first(maps);
@@ -1520,7 +1520,7 @@ out:
 struct map *map_groups__find_by_name(struct map_groups *mg,
 				     enum map_type type, const char *name)
 {
-	struct rb_root *maps = &mg->maps[type];
+	struct maps *maps = &mg->maps[type];
 	struct map *map;
 
 	for (map = maps__first(maps); map; map = map__next(map)) {
-- 
2.1.0


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

* [PATCH 2/7] perf tools: Protect accesses the map rbtrees with a rw lock
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 1/7] perf tools: Introduce struct maps Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 3/7] perf tools: Check if a map is still in use when deleting it Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

To allow concurrent access, next step: refcount struct map instances, so
that we can ditch maps->removed_maps and stop leaking threads, maps,
then struct DSO needs the same treatment.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-o45w2w5dzrza38nzqxnqzhyf@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/map.c    | 122 +++++++++++++++++++++++++++++++++++------------
 tools/perf/util/map.h    |   2 +
 tools/perf/util/symbol.c |  17 +++++--
 3 files changed, 108 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index adf012c4d650..0905b07072da 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -16,6 +16,8 @@
 #include "machine.h"
 #include <linux/string.h>
 
+static void __maps__insert(struct maps *maps, struct map *map);
+
 const char *map_type__name[MAP__NR_TYPES] = {
 	[MAP__FUNCTION] = "Functions",
 	[MAP__VARIABLE] = "Variables",
@@ -421,6 +423,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 static void maps__init(struct maps *maps)
 {
 	maps->entries = RB_ROOT;
+	pthread_rwlock_init(&maps->lock, NULL);
 	INIT_LIST_HEAD(&maps->removed_maps);
 }
 
@@ -434,7 +437,7 @@ void map_groups__init(struct map_groups *mg, struct machine *machine)
 	atomic_set(&mg->refcnt, 1);
 }
 
-static void maps__purge(struct maps *maps)
+static void __maps__purge(struct maps *maps)
 {
 	struct rb_root *root = &maps->entries;
 	struct rb_node *next = rb_first(root);
@@ -448,7 +451,7 @@ static void maps__purge(struct maps *maps)
 	}
 }
 
-static void maps__purge_removed_maps(struct maps *maps)
+static void __maps__purge_removed_maps(struct maps *maps)
 {
 	struct map *pos, *n;
 
@@ -460,8 +463,10 @@ static void maps__purge_removed_maps(struct maps *maps)
 
 static void maps__exit(struct maps *maps)
 {
-	maps__purge(maps);
-	maps__purge_removed_maps(maps);
+	pthread_rwlock_wrlock(&maps->lock);
+	__maps__purge(maps);
+	__maps__purge_removed_maps(maps);
+	pthread_rwlock_unlock(&maps->lock);
 }
 
 void map_groups__exit(struct map_groups *mg)
@@ -531,20 +536,28 @@ struct symbol *map_groups__find_symbol_by_name(struct map_groups *mg,
 					       struct map **mapp,
 					       symbol_filter_t filter)
 {
+	struct maps *maps = &mg->maps[type];
+	struct symbol *sym;
 	struct rb_node *nd;
 
-	for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) {
+	pthread_rwlock_rdlock(&maps->lock);
+
+	for (nd = rb_first(&maps->entries); nd; nd = rb_next(nd)) {
 		struct map *pos = rb_entry(nd, struct map, rb_node);
-		struct symbol *sym = map__find_symbol_by_name(pos, name, filter);
+
+		sym = map__find_symbol_by_name(pos, name, filter);
 
 		if (sym == NULL)
 			continue;
 		if (mapp != NULL)
 			*mapp = pos;
-		return sym;
+		goto out;
 	}
 
-	return NULL;
+	sym = NULL;
+out:
+	pthread_rwlock_unlock(&maps->lock);
+	return sym;
 }
 
 int map_groups__find_ams(struct addr_map_symbol *ams, symbol_filter_t filter)
@@ -564,25 +577,35 @@ int map_groups__find_ams(struct addr_map_symbol *ams, symbol_filter_t filter)
 	return ams->sym ? 0 : -1;
 }
 
-size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type,
-				  FILE *fp)
+static size_t maps__fprintf(struct maps *maps, FILE *fp)
 {
-	size_t printed = fprintf(fp, "%s:\n", map_type__name[type]);
+	size_t printed = 0;
 	struct rb_node *nd;
 
-	for (nd = rb_first(&mg->maps[type].entries); nd; nd = rb_next(nd)) {
+	pthread_rwlock_rdlock(&maps->lock);
+
+	for (nd = rb_first(&maps->entries); nd; nd = rb_next(nd)) {
 		struct map *pos = rb_entry(nd, struct map, rb_node);
 		printed += fprintf(fp, "Map:");
 		printed += map__fprintf(pos, fp);
 		if (verbose > 2) {
-			printed += dso__fprintf(pos->dso, type, fp);
+			printed += dso__fprintf(pos->dso, pos->type, fp);
 			printed += fprintf(fp, "--\n");
 		}
 	}
 
+	pthread_rwlock_unlock(&maps->lock);
+
 	return printed;
 }
 
+size_t __map_groups__fprintf_maps(struct map_groups *mg, enum map_type type,
+				  FILE *fp)
+{
+	size_t printed = fprintf(fp, "%s:\n", map_type__name[type]);
+	return printed += maps__fprintf(&mg->maps[type], fp);
+}
+
 static size_t map_groups__fprintf_maps(struct map_groups *mg, FILE *fp)
 {
 	size_t printed = 0, i;
@@ -624,13 +647,17 @@ size_t map_groups__fprintf(struct map_groups *mg, FILE *fp)
 	return printed + map_groups__fprintf_removed_maps(mg, fp);
 }
 
-int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
-				   FILE *fp)
+static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
 {
-	struct rb_root *root = &mg->maps[map->type].entries;
-	struct rb_node *next = rb_first(root);
+	struct rb_root *root;
+	struct rb_node *next;
 	int err = 0;
 
+	pthread_rwlock_wrlock(&maps->lock);
+
+	root = &maps->entries;
+	next = rb_first(root);
+
 	while (next) {
 		struct map *pos = rb_entry(next, struct map, rb_node);
 		next = rb_next(&pos->rb_node);
@@ -658,7 +685,7 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
 			}
 
 			before->end = map->start;
-			map_groups__insert(mg, before);
+			__maps__insert(maps, before);
 			if (verbose >= 2)
 				map__fprintf(before, fp);
 		}
@@ -672,7 +699,7 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
 			}
 
 			after->start = map->end;
-			map_groups__insert(mg, after);
+			__maps__insert(maps, after);
 			if (verbose >= 2)
 				map__fprintf(after, fp);
 		}
@@ -681,15 +708,24 @@ move_map:
 		 * If we have references, just move them to a separate list.
 		 */
 		if (pos->referenced)
-			list_add_tail(&pos->node, &mg->maps[map->type].removed_maps);
+			list_add_tail(&pos->node, &maps->removed_maps);
 		else
 			map__delete(pos);
 
 		if (err)
-			return err;
+			goto out;
 	}
 
-	return 0;
+	err = 0;
+out:
+	pthread_rwlock_unlock(&maps->lock);
+	return err;
+}
+
+int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
+				   FILE *fp)
+{
+	return maps__fixup_overlappings(&mg->maps[map->type], map, fp);
 }
 
 /*
@@ -698,19 +734,26 @@ move_map:
 int map_groups__clone(struct map_groups *mg,
 		      struct map_groups *parent, enum map_type type)
 {
+	int err = -ENOMEM;
 	struct map *map;
 	struct maps *maps = &parent->maps[type];
 
+	pthread_rwlock_rdlock(&maps->lock);
+
 	for (map = maps__first(maps); map; map = map__next(map)) {
 		struct map *new = map__clone(map);
 		if (new == NULL)
-			return -ENOMEM;
+			goto out_unlock;
 		map_groups__insert(mg, new);
 	}
-	return 0;
+
+	err = 0;
+out_unlock:
+	pthread_rwlock_unlock(&maps->lock);
+	return err;
 }
 
-void maps__insert(struct maps *maps, struct map *map)
+static void __maps__insert(struct maps *maps, struct map *map)
 {
 	struct rb_node **p = &maps->entries.rb_node;
 	struct rb_node *parent = NULL;
@@ -730,17 +773,33 @@ void maps__insert(struct maps *maps, struct map *map)
 	rb_insert_color(&map->rb_node, &maps->entries);
 }
 
-void maps__remove(struct maps *maps, struct map *map)
+void maps__insert(struct maps *maps, struct map *map)
+{
+	pthread_rwlock_wrlock(&maps->lock);
+	__maps__insert(maps, map);
+	pthread_rwlock_unlock(&maps->lock);
+}
+
+static void __maps__remove(struct maps *maps, struct map *map)
 {
 	rb_erase(&map->rb_node, &maps->entries);
 }
 
+void maps__remove(struct maps *maps, struct map *map)
+{
+	pthread_rwlock_wrlock(&maps->lock);
+	__maps__remove(maps, map);
+	pthread_rwlock_unlock(&maps->lock);
+}
+
 struct map *maps__find(struct maps *maps, u64 ip)
 {
-	struct rb_node **p = &maps->entries.rb_node;
-	struct rb_node *parent = NULL;
+	struct rb_node **p, *parent = NULL;
 	struct map *m;
 
+	pthread_rwlock_rdlock(&maps->lock);
+
+	p = &maps->entries.rb_node;
 	while (*p != NULL) {
 		parent = *p;
 		m = rb_entry(parent, struct map, rb_node);
@@ -749,10 +808,13 @@ struct map *maps__find(struct maps *maps, u64 ip)
 		else if (ip >= m->end)
 			p = &(*p)->rb_right;
 		else
-			return m;
+			goto out;
 	}
 
-	return NULL;
+	m = NULL;
+out:
+	pthread_rwlock_unlock(&maps->lock);
+	return m;
 }
 
 struct map *maps__first(struct maps *maps)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index e3702fd468c5..6796f2785649 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -5,6 +5,7 @@
 #include <linux/compiler.h>
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include <pthread.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <linux/types.h>
@@ -60,6 +61,7 @@ struct kmap {
 
 struct maps {
 	struct rb_root	 entries;
+	pthread_rwlock_t lock;
 	struct list_head removed_maps;
 };
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index c8a3e79c5da2..8aae8b6b1cee 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -205,9 +205,11 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
 	struct maps *maps = &mg->maps[type];
 	struct map *next, *curr;
 
+	pthread_rwlock_wrlock(&maps->lock);
+
 	curr = maps__first(maps);
 	if (curr == NULL)
-		return;
+		goto out_unlock;
 
 	for (next = map__next(curr); next; next = map__next(curr)) {
 		curr->end = next->start;
@@ -219,6 +221,9 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type)
 	 * last map final address.
 	 */
 	curr->end = ~0ULL;
+
+out_unlock:
+	pthread_rwlock_unlock(&maps->lock);
 }
 
 struct symbol *symbol__new(u64 start, u64 len, u8 binding, const char *name)
@@ -1523,12 +1528,18 @@ struct map *map_groups__find_by_name(struct map_groups *mg,
 	struct maps *maps = &mg->maps[type];
 	struct map *map;
 
+	pthread_rwlock_rdlock(&maps->lock);
+
 	for (map = maps__first(maps); map; map = map__next(map)) {
 		if (map->dso && strcmp(map->dso->short_name, name) == 0)
-			return map;
+			goto out_unlock;
 	}
 
-	return NULL;
+	map = NULL;
+
+out_unlock:
+	pthread_rwlock_unlock(&maps->lock);
+	return map;
 }
 
 int dso__load_vmlinux(struct dso *dso, struct map *map,
-- 
2.1.0


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

* [PATCH 3/7] perf tools: Check if a map is still in use when deleting it
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 1/7] perf tools: Introduce struct maps Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 2/7] perf tools: Protect accesses the map rbtrees with a rw lock Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 4/7] perf tools: Reference count struct map Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

I.e. match RB_CLEAR_NODE() with RB_EMPTY_NODE(), to check that it isn't
in a rb tree at the time of its deletion.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-vumvhird765id11zbx00d2r8@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c | 4 ++++
 tools/perf/util/map.c         | 9 +++++----
 tools/perf/util/symbol.c      | 8 ++++----
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index b57a027fb200..c434e1264087 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -59,6 +59,10 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
 	    (al->sym == NULL ||
 	     strcmp(ann->sym_hist_filter, al->sym->name) != 0)) {
 		/* We're only interested in a symbol named sym_hist_filter */
+		/*
+		 * FIXME: why isn't this done in the symbol_filter when loading
+		 * the DSO?
+		 */
 		if (al->sym != NULL) {
 			rb_erase(&al->sym->rb_node,
 				 &al->map->dso->symbols[al->map->type]);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 0905b07072da..4d3a92d5dff3 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -225,6 +225,7 @@ struct map *map__new2(u64 start, struct dso *dso, enum map_type type)
 
 void map__delete(struct map *map)
 {
+	BUG_ON(!RB_EMPTY_NODE(&map->rb_node));
 	free(map);
 }
 
@@ -446,7 +447,7 @@ static void __maps__purge(struct maps *maps)
 		struct map *pos = rb_entry(next, struct map, rb_node);
 
 		next = rb_next(&pos->rb_node);
-		rb_erase(&pos->rb_node, root);
+		rb_erase_init(&pos->rb_node, root);
 		map__delete(pos);
 	}
 }
@@ -456,7 +457,7 @@ static void __maps__purge_removed_maps(struct maps *maps)
 	struct map *pos, *n;
 
 	list_for_each_entry_safe(pos, n, &maps->removed_maps, node) {
-		list_del(&pos->node);
+		list_del_init(&pos->node);
 		map__delete(pos);
 	}
 }
@@ -671,7 +672,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 			map__fprintf(pos, fp);
 		}
 
-		rb_erase(&pos->rb_node, root);
+		rb_erase_init(&pos->rb_node, root);
 		/*
 		 * Now check if we need to create new maps for areas not
 		 * overlapped by the new map:
@@ -782,7 +783,7 @@ void maps__insert(struct maps *maps, struct map *map)
 
 static void __maps__remove(struct maps *maps, struct map *map)
 {
-	rb_erase(&map->rb_node, &maps->entries);
+	rb_erase_init(&map->rb_node, &maps->entries);
 }
 
 void maps__remove(struct maps *maps, struct map *map)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8aae8b6b1cee..743a9b360e3d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -659,14 +659,14 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 		curr_map = map_groups__find(kmaps, map->type, pos->start);
 
 		if (!curr_map || (filter && filter(curr_map, pos))) {
-			rb_erase(&pos->rb_node, root);
+			rb_erase_init(&pos->rb_node, root);
 			symbol__delete(pos);
 		} else {
 			pos->start -= curr_map->start - curr_map->pgoff;
 			if (pos->end)
 				pos->end -= curr_map->start - curr_map->pgoff;
 			if (curr_map != map) {
-				rb_erase(&pos->rb_node, root);
+				rb_erase_init(&pos->rb_node, root);
 				symbols__insert(
 					&curr_map->dso->symbols[curr_map->type],
 					pos);
@@ -1173,7 +1173,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	/* Add new maps */
 	while (!list_empty(&md.maps)) {
 		new_map = list_entry(md.maps.next, struct map, node);
-		list_del(&new_map->node);
+		list_del_init(&new_map->node);
 		if (new_map == replacement_map) {
 			map->start	= new_map->start;
 			map->end	= new_map->end;
@@ -1211,7 +1211,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 out_err:
 	while (!list_empty(&md.maps)) {
 		map = list_entry(md.maps.next, struct map, node);
-		list_del(&map->node);
+		list_del_init(&map->node);
 		map__delete(map);
 	}
 	close(fd);
-- 
2.1.0


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

* [PATCH 4/7] perf tools: Reference count struct map
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2015-05-28  0:05 ` [PATCH 3/7] perf tools: Check if a map is still in use when deleting it Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 5/7] perf tools: Add hint for 'Too many events are opened.' error message Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We have pointers to struct map instances in several places, like in the
hist_entry instances, so we need a way to know when we can destroy them,
otherwise we may either keep leaking them or end up referencing deleted
instances.

Start fixing it by reference counting them.

This patch puts the reference count for struct map in place, replacing
direct map__delete() calls with map__put() ones and then grabbing a
reference count when adding it to the maps struct where maps for a
struct thread are kept.

Next we'll grab reference counts when setting pointers to struct map
instances, in places like in the hist_entry code.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-wi19xczk0t2a41r1i2chuio5@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c     |  3 ++-
 tools/perf/util/map.c         | 21 +++++++++++++++------
 tools/perf/util/map.h         | 11 +++++++++++
 tools/perf/util/probe-event.c |  6 +++---
 tools/perf/util/symbol-elf.c  |  2 ++
 tools/perf/util/symbol.c      |  7 +++++--
 6 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 6bf845758ae3..0c0e61cce577 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -759,7 +759,6 @@ void machine__destroy_kernel_maps(struct machine *machine)
 				kmap->ref_reloc_sym = NULL;
 		}
 
-		map__delete(machine->vmlinux_maps[type]);
 		machine->vmlinux_maps[type] = NULL;
 	}
 }
@@ -1247,6 +1246,7 @@ int machine__process_mmap2_event(struct machine *machine,
 
 	thread__insert_map(thread, map);
 	thread__put(thread);
+	map__put(map);
 	return 0;
 
 out_problem_map:
@@ -1297,6 +1297,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
 
 	thread__insert_map(thread, map);
 	thread__put(thread);
+	map__put(map);
 	return 0;
 
 out_problem_map:
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 4d3a92d5dff3..af572322586d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -139,6 +139,7 @@ void map__init(struct map *map, enum map_type type,
 	map->groups   = NULL;
 	map->referenced = false;
 	map->erange_warned = false;
+	atomic_set(&map->refcnt, 1);
 }
 
 struct map *map__new(struct machine *machine, u64 start, u64 len,
@@ -229,6 +230,12 @@ void map__delete(struct map *map)
 	free(map);
 }
 
+void map__put(struct map *map)
+{
+	if (map && atomic_dec_and_test(&map->refcnt))
+		map__delete(map);
+}
+
 void map__fixup_start(struct map *map)
 {
 	struct rb_root *symbols = &map->dso->symbols[map->type];
@@ -448,7 +455,7 @@ static void __maps__purge(struct maps *maps)
 
 		next = rb_next(&pos->rb_node);
 		rb_erase_init(&pos->rb_node, root);
-		map__delete(pos);
+		map__put(pos);
 	}
 }
 
@@ -458,7 +465,7 @@ static void __maps__purge_removed_maps(struct maps *maps)
 
 	list_for_each_entry_safe(pos, n, &maps->removed_maps, node) {
 		list_del_init(&pos->node);
-		map__delete(pos);
+		map__put(pos);
 	}
 }
 
@@ -682,7 +689,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 
 			if (before == NULL) {
 				err = -ENOMEM;
-				goto move_map;
+				goto put_map;
 			}
 
 			before->end = map->start;
@@ -696,7 +703,7 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 
 			if (after == NULL) {
 				err = -ENOMEM;
-				goto move_map;
+				goto put_map;
 			}
 
 			after->start = map->end;
@@ -704,14 +711,14 @@ static int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp
 			if (verbose >= 2)
 				map__fprintf(after, fp);
 		}
-move_map:
+put_map:
 		/*
 		 * If we have references, just move them to a separate list.
 		 */
 		if (pos->referenced)
 			list_add_tail(&pos->node, &maps->removed_maps);
 		else
-			map__delete(pos);
+			map__put(pos);
 
 		if (err)
 			goto out;
@@ -772,6 +779,7 @@ static void __maps__insert(struct maps *maps, struct map *map)
 
 	rb_link_node(&map->rb_node, parent, p);
 	rb_insert_color(&map->rb_node, &maps->entries);
+	map__get(map);
 }
 
 void maps__insert(struct maps *maps, struct map *map)
@@ -784,6 +792,7 @@ void maps__insert(struct maps *maps, struct map *map)
 static void __maps__remove(struct maps *maps, struct map *map)
 {
 	rb_erase_init(&map->rb_node, &maps->entries);
+	map__put(map);
 }
 
 void maps__remove(struct maps *maps, struct map *map)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 6796f2785649..b8df09d94aca 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -52,6 +52,7 @@ struct map {
 
 	struct dso		*dso;
 	struct map_groups	*groups;
+	atomic_t		refcnt;
 };
 
 struct kmap {
@@ -150,6 +151,16 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
 struct map *map__new2(u64 start, struct dso *dso, enum map_type type);
 void map__delete(struct map *map);
 struct map *map__clone(struct map *map);
+
+static inline struct map *map__get(struct map *map)
+{
+	if (map)
+		atomic_inc(&map->refcnt);
+	return map;
+}
+
+void map__put(struct map *map);
+
 int map__overlap(struct map *l, struct map *r);
 size_t map__fprintf(struct map *map, FILE *fp);
 size_t map__fprintf_dsoname(struct map *map, FILE *fp);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 32471d0839d1..b0b8a8080009 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -195,7 +195,7 @@ static void put_target_map(struct map *map, bool user)
 {
 	if (map && user) {
 		/* Only the user map needs to be released */
-		map__delete(map);
+		map__put(map);
 	}
 }
 
@@ -1791,7 +1791,7 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
 
 out:
 	if (map && !is_kprobe) {
-		map__delete(map);
+		map__put(map);
 	}
 
 	return ret;
@@ -2884,7 +2884,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter,
 	dso__fprintf_symbols_by_name(map->dso, map->type, stdout);
 end:
 	if (user) {
-		map__delete(map);
+		map__put(map);
 	}
 	exit_symbol_maps();
 
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 9d526a5312b1..fa10116a12ab 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -972,8 +972,10 @@ int dso__load_sym(struct dso *dso, struct map *map,
 					map->unmap_ip = map__unmap_ip;
 					/* Ensure maps are correctly ordered */
 					if (kmaps) {
+						map__get(map);
 						map_groups__remove(kmaps, map);
 						map_groups__insert(kmaps, map);
+						map__put(map);
 					}
 				}
 
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 743a9b360e3d..a3e80d6ad70a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1180,13 +1180,16 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 			map->pgoff	= new_map->pgoff;
 			map->map_ip	= new_map->map_ip;
 			map->unmap_ip	= new_map->unmap_ip;
-			map__delete(new_map);
 			/* Ensure maps are correctly ordered */
+			map__get(map);
 			map_groups__remove(kmaps, map);
 			map_groups__insert(kmaps, map);
+			map__put(map);
 		} else {
 			map_groups__insert(kmaps, new_map);
 		}
+
+		map__put(new_map);
 	}
 
 	/*
@@ -1212,7 +1215,7 @@ out_err:
 	while (!list_empty(&md.maps)) {
 		map = list_entry(md.maps.next, struct map, node);
 		list_del_init(&map->node);
-		map__delete(map);
+		map__put(map);
 	}
 	close(fd);
 	return -EINVAL;
-- 
2.1.0


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

* [PATCH 5/7] perf tools: Add hint for 'Too many events are opened.' error message
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2015-05-28  0:05 ` [PATCH 4/7] perf tools: Reference count struct map Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 6/7] perf annotation: Add symbol__get_annotation Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, David Ahern, Michael Petlan,
	Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Jiri Olsa <jolsa@kernel.org>

Enhancing the 'Too many events are opened.' error message with hint to
use use 'ulimit -n <limit>' command.

Before:

  $ perf record -e 'sched:*,syscalls:*' ls
  Error:
  Too many events are opened.
  Try again after reducing the number of events.

Now:

  $ perf record -e 'sched:*,syscalls:*' ls
  Error:
  Too many events are opened.
  Probably the maximum number of open file descriptors has been reached.
  Hint: Try again after reducing the number of events.
  Hint: Try increasing the limit with 'ulimit -n <limit>'

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1432587114-14924-1-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c886b9f7a48d..a3e36fc634dc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2149,7 +2149,9 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	case EMFILE:
 		return scnprintf(msg, size, "%s",
 			 "Too many events are opened.\n"
-			 "Try again after reducing the number of events.");
+			 "Probably the maximum number of open file descriptors has been reached.\n"
+			 "Hint: Try again after reducing the number of events.\n"
+			 "Hint: Try increasing the limit with 'ulimit -n <limit>'");
 	case ENODEV:
 		if (target->cpu_list)
 			return scnprintf(msg, size, "%s",
-- 
2.1.0


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

* [PATCH 6/7] perf annotation: Add symbol__get_annotation
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2015-05-28  0:05 ` [PATCH 5/7] perf tools: Add hint for 'Too many events are opened.' error message Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:05 ` [PATCH 7/7] perf tools: Move branch option parsing to own file Arnaldo Carvalho de Melo
  2015-05-28  0:27 ` [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Andi Kleen, Jiri Olsa, Namhyung Kim,
	Stephane Eranian, Arnaldo Carvalho de Melo

From: Andi Kleen <ak@linux.intel.com>

Add a new utility function to get an function annotation out of existing
code.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1432749114-904-4-git-send-email-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7f5bdfc9bc87..bf8043009909 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -506,6 +506,17 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 	return 0;
 }
 
+static struct annotation *symbol__get_annotation(struct symbol *sym)
+{
+	struct annotation *notes = symbol__annotation(sym);
+
+	if (notes->src == NULL) {
+		if (symbol__alloc_hist(sym) < 0)
+			return NULL;
+	}
+	return notes;
+}
+
 static int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 				    int evidx, u64 addr)
 {
@@ -513,13 +524,9 @@ static int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 
 	if (sym == NULL)
 		return 0;
-
-	notes = symbol__annotation(sym);
-	if (notes->src == NULL) {
-		if (symbol__alloc_hist(sym) < 0)
-			return -ENOMEM;
-	}
-
+	notes = symbol__get_annotation(sym);
+	if (notes == NULL)
+		return -ENOMEM;
 	return __symbol__inc_addr_samples(sym, map, notes, evidx, addr);
 }
 
-- 
2.1.0


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

* [PATCH 7/7] perf tools: Move branch option parsing to own file
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2015-05-28  0:05 ` [PATCH 6/7] perf annotation: Add symbol__get_annotation Arnaldo Carvalho de Melo
@ 2015-05-28  0:05 ` Arnaldo Carvalho de Melo
  2015-05-28  0:27 ` [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
  7 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:05 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Andi Kleen, Jiri Olsa, Namhyung Kim,
	Stephane Eranian, Arnaldo Carvalho de Melo

From: Andi Kleen <ak@linux.intel.com>

.. to allow sharing between builtin-record and builtin-top later.  No
code changes, just moved code.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1432749114-904-9-git-send-email-andi@firstfloor.org
[ Rename too generic branch.[ch] name to parse-branch-options.[ch] ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c            | 89 +-------------------------------
 tools/perf/util/Build                  |  1 +
 tools/perf/util/parse-branch-options.c | 93 ++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-branch-options.h |  5 ++
 4 files changed, 100 insertions(+), 88 deletions(-)
 create mode 100644 tools/perf/util/parse-branch-options.c
 create mode 100644 tools/perf/util/parse-branch-options.h

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5dfe91395617..91aa2a3dcf19 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -28,6 +28,7 @@
 #include "util/thread_map.h"
 #include "util/data.h"
 #include "util/auxtrace.h"
+#include "util/parse-branch-options.h"
 
 #include <unistd.h>
 #include <sched.h>
@@ -751,94 +752,6 @@ out_delete_session:
 	return status;
 }
 
-#define BRANCH_OPT(n, m) \
-	{ .name = n, .mode = (m) }
-
-#define BRANCH_END { .name = NULL }
-
-struct branch_mode {
-	const char *name;
-	int mode;
-};
-
-static const struct branch_mode branch_modes[] = {
-	BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER),
-	BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL),
-	BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV),
-	BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY),
-	BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
-	BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
-	BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
-	BRANCH_OPT("abort_tx", PERF_SAMPLE_BRANCH_ABORT_TX),
-	BRANCH_OPT("in_tx", PERF_SAMPLE_BRANCH_IN_TX),
-	BRANCH_OPT("no_tx", PERF_SAMPLE_BRANCH_NO_TX),
-	BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_COND),
-	BRANCH_END
-};
-
-static int
-parse_branch_stack(const struct option *opt, const char *str, int unset)
-{
-#define ONLY_PLM \
-	(PERF_SAMPLE_BRANCH_USER	|\
-	 PERF_SAMPLE_BRANCH_KERNEL	|\
-	 PERF_SAMPLE_BRANCH_HV)
-
-	uint64_t *mode = (uint64_t *)opt->value;
-	const struct branch_mode *br;
-	char *s, *os = NULL, *p;
-	int ret = -1;
-
-	if (unset)
-		return 0;
-
-	/*
-	 * cannot set it twice, -b + --branch-filter for instance
-	 */
-	if (*mode)
-		return -1;
-
-	/* str may be NULL in case no arg is passed to -b */
-	if (str) {
-		/* because str is read-only */
-		s = os = strdup(str);
-		if (!s)
-			return -1;
-
-		for (;;) {
-			p = strchr(s, ',');
-			if (p)
-				*p = '\0';
-
-			for (br = branch_modes; br->name; br++) {
-				if (!strcasecmp(s, br->name))
-					break;
-			}
-			if (!br->name) {
-				ui__warning("unknown branch filter %s,"
-					    " check man page\n", s);
-				goto error;
-			}
-
-			*mode |= br->mode;
-
-			if (!p)
-				break;
-
-			s = p + 1;
-		}
-	}
-	ret = 0;
-
-	/* default to any branch */
-	if ((*mode & ~ONLY_PLM) == 0) {
-		*mode = PERF_SAMPLE_BRANCH_ANY;
-	}
-error:
-	free(os);
-	return ret;
-}
-
 static void callchain_debug(void)
 {
 	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF", "LBR" };
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 6966d0743bf7..e4b676de2f64 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -75,6 +75,7 @@ libperf-$(CONFIG_X86) += tsc.o
 libperf-y += cloexec.o
 libperf-y += thread-stack.o
 libperf-$(CONFIG_AUXTRACE) += auxtrace.o
+libperf-y += parse-branch-options.o
 
 libperf-$(CONFIG_LIBELF) += symbol-elf.o
 libperf-$(CONFIG_LIBELF) += probe-event.o
diff --git a/tools/perf/util/parse-branch-options.c b/tools/perf/util/parse-branch-options.c
new file mode 100644
index 000000000000..9d999436658f
--- /dev/null
+++ b/tools/perf/util/parse-branch-options.c
@@ -0,0 +1,93 @@
+#include "perf.h"
+#include "util/util.h"
+#include "util/debug.h"
+#include "util/parse-options.h"
+#include "util/parse-branch-options.h"
+
+#define BRANCH_OPT(n, m) \
+	{ .name = n, .mode = (m) }
+
+#define BRANCH_END { .name = NULL }
+
+struct branch_mode {
+	const char *name;
+	int mode;
+};
+
+static const struct branch_mode branch_modes[] = {
+	BRANCH_OPT("u", PERF_SAMPLE_BRANCH_USER),
+	BRANCH_OPT("k", PERF_SAMPLE_BRANCH_KERNEL),
+	BRANCH_OPT("hv", PERF_SAMPLE_BRANCH_HV),
+	BRANCH_OPT("any", PERF_SAMPLE_BRANCH_ANY),
+	BRANCH_OPT("any_call", PERF_SAMPLE_BRANCH_ANY_CALL),
+	BRANCH_OPT("any_ret", PERF_SAMPLE_BRANCH_ANY_RETURN),
+	BRANCH_OPT("ind_call", PERF_SAMPLE_BRANCH_IND_CALL),
+	BRANCH_OPT("abort_tx", PERF_SAMPLE_BRANCH_ABORT_TX),
+	BRANCH_OPT("in_tx", PERF_SAMPLE_BRANCH_IN_TX),
+	BRANCH_OPT("no_tx", PERF_SAMPLE_BRANCH_NO_TX),
+	BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_COND),
+	BRANCH_END
+};
+
+int
+parse_branch_stack(const struct option *opt, const char *str, int unset)
+{
+#define ONLY_PLM \
+	(PERF_SAMPLE_BRANCH_USER	|\
+	 PERF_SAMPLE_BRANCH_KERNEL	|\
+	 PERF_SAMPLE_BRANCH_HV)
+
+	uint64_t *mode = (uint64_t *)opt->value;
+	const struct branch_mode *br;
+	char *s, *os = NULL, *p;
+	int ret = -1;
+
+	if (unset)
+		return 0;
+
+	/*
+	 * cannot set it twice, -b + --branch-filter for instance
+	 */
+	if (*mode)
+		return -1;
+
+	/* str may be NULL in case no arg is passed to -b */
+	if (str) {
+		/* because str is read-only */
+		s = os = strdup(str);
+		if (!s)
+			return -1;
+
+		for (;;) {
+			p = strchr(s, ',');
+			if (p)
+				*p = '\0';
+
+			for (br = branch_modes; br->name; br++) {
+				if (!strcasecmp(s, br->name))
+					break;
+			}
+			if (!br->name) {
+				ui__warning("unknown branch filter %s,"
+					    " check man page\n", s);
+				goto error;
+			}
+
+			*mode |= br->mode;
+
+			if (!p)
+				break;
+
+			s = p + 1;
+		}
+	}
+	ret = 0;
+
+	/* default to any branch */
+	if ((*mode & ~ONLY_PLM) == 0) {
+		*mode = PERF_SAMPLE_BRANCH_ANY;
+	}
+error:
+	free(os);
+	return ret;
+}
diff --git a/tools/perf/util/parse-branch-options.h b/tools/perf/util/parse-branch-options.h
new file mode 100644
index 000000000000..b9d9470c2e82
--- /dev/null
+++ b/tools/perf/util/parse-branch-options.h
@@ -0,0 +1,5 @@
+#ifndef _PERF_PARSE_BRANCH_OPTIONS_H
+#define _PERF_PARSE_BRANCH_OPTIONS_H 1
+struct option;
+int parse_branch_stack(const struct option *opt, const char *str, int unset);
+#endif /* _PERF_PARSE_BRANCH_OPTIONS_H */
-- 
2.1.0


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

* Re: [GIT PULL 0/7] perf/core refactorings and improvements
  2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2015-05-28  0:05 ` [PATCH 7/7] perf tools: Move branch option parsing to own file Arnaldo Carvalho de Melo
@ 2015-05-28  0:27 ` Arnaldo Carvalho de Melo
  2015-05-28  9:27   ` Ingo Molnar
  7 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-28  0:27 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Andi Kleen, Borislav Petkov,
	David Ahern, Don Zickus, Frederic Weisbecker, Jiri Olsa,
	Michael Petlan, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian

Em Wed, May 27, 2015 at 09:05:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> Hi Ingo,
> 
> 	Please consider applying,

Sorry, missed the git-request-pull part, ouch, here it is:

The following changes since commit 6632c4b4e96c668e19173fa17f2c58c60490bac3:

  Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-05-27 18:42:36 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to f00898f4e20b286877b8d6d96d6e404661fd7985:

  perf tools: Move branch option parsing to own file (2015-05-27 21:02:17 -0300)

----------------------------------------------------------------
perf/core refactorings and improvements:

User visible:

- Add hint for 'Too many events are opened.' error message (Jiri Olsa)

Infrastructure:

- Protect accesses to map rbtrees with a lock and refcount struct map,
  reducing memory usage as maps not used get freed. The 'dso' struct is
  next in line. (Arnaldo Carvalho de Melo)

- Annotation and branch related option parsing refactorings to
  share code with upcoming patches (Andi Kleen)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Andi Kleen (2):
      perf annotation: Add symbol__get_annotation
      perf tools: Move branch option parsing to own file

Arnaldo Carvalho de Melo (4):
      perf tools: Introduce struct maps
      perf tools: Protect accesses the map rbtrees with a rw lock
      perf tools: Check if a map is still in use when deleting it
      perf tools: Reference count struct map

Jiri Olsa (1):
      perf tools: Add hint for 'Too many events are opened.' error message

 tools/perf/builtin-annotate.c          |   4 +
 tools/perf/builtin-record.c            |  89 +--------------
 tools/perf/tests/vmlinux-kallsyms.c    |   2 +-
 tools/perf/util/Build                  |   1 +
 tools/perf/util/annotate.c             |  21 ++--
 tools/perf/util/event.c                |   2 +-
 tools/perf/util/evsel.c                |   4 +-
 tools/perf/util/machine.c              |   3 +-
 tools/perf/util/map.c                  | 190 +++++++++++++++++++++++----------
 tools/perf/util/map.h                  |  29 +++--
 tools/perf/util/parse-branch-options.c |  93 ++++++++++++++++
 tools/perf/util/parse-branch-options.h |   5 +
 tools/perf/util/probe-event.c          |   8 +-
 tools/perf/util/symbol-elf.c           |   2 +
 tools/perf/util/symbol.c               |  36 +++++--
 15 files changed, 315 insertions(+), 174 deletions(-)
 create mode 100644 tools/perf/util/parse-branch-options.c
 create mode 100644 tools/perf/util/parse-branch-options.h

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

* Re: [GIT PULL 0/7] perf/core refactorings and improvements
  2015-05-28  0:27 ` [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
@ 2015-05-28  9:27   ` Ingo Molnar
  0 siblings, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2015-05-28  9:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Adrian Hunter, Andi Kleen, Borislav Petkov,
	David Ahern, Don Zickus, Frederic Weisbecker, Jiri Olsa,
	Michael Petlan, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Stephane Eranian


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Wed, May 27, 2015 at 09:05:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Hi Ingo,
> > 
> > 	Please consider applying,
> 
> Sorry, missed the git-request-pull part, ouch, here it is:
> 
> The following changes since commit 6632c4b4e96c668e19173fa17f2c58c60490bac3:
> 
>   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-05-27 18:42:36 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
> 
> for you to fetch changes up to f00898f4e20b286877b8d6d96d6e404661fd7985:
> 
>   perf tools: Move branch option parsing to own file (2015-05-27 21:02:17 -0300)
> 
> ----------------------------------------------------------------
> perf/core refactorings and improvements:
> 
> User visible:
> 
> - Add hint for 'Too many events are opened.' error message (Jiri Olsa)
> 
> Infrastructure:
> 
> - Protect accesses to map rbtrees with a lock and refcount struct map,
>   reducing memory usage as maps not used get freed. The 'dso' struct is
>   next in line. (Arnaldo Carvalho de Melo)
> 
> - Annotation and branch related option parsing refactorings to
>   share code with upcoming patches (Andi Kleen)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Andi Kleen (2):
>       perf annotation: Add symbol__get_annotation
>       perf tools: Move branch option parsing to own file
> 
> Arnaldo Carvalho de Melo (4):
>       perf tools: Introduce struct maps
>       perf tools: Protect accesses the map rbtrees with a rw lock
>       perf tools: Check if a map is still in use when deleting it
>       perf tools: Reference count struct map
> 
> Jiri Olsa (1):
>       perf tools: Add hint for 'Too many events are opened.' error message
> 
>  tools/perf/builtin-annotate.c          |   4 +
>  tools/perf/builtin-record.c            |  89 +--------------
>  tools/perf/tests/vmlinux-kallsyms.c    |   2 +-
>  tools/perf/util/Build                  |   1 +
>  tools/perf/util/annotate.c             |  21 ++--
>  tools/perf/util/event.c                |   2 +-
>  tools/perf/util/evsel.c                |   4 +-
>  tools/perf/util/machine.c              |   3 +-
>  tools/perf/util/map.c                  | 190 +++++++++++++++++++++++----------
>  tools/perf/util/map.h                  |  29 +++--
>  tools/perf/util/parse-branch-options.c |  93 ++++++++++++++++
>  tools/perf/util/parse-branch-options.h |   5 +
>  tools/perf/util/probe-event.c          |   8 +-
>  tools/perf/util/symbol-elf.c           |   2 +
>  tools/perf/util/symbol.c               |  36 +++++--
>  15 files changed, 315 insertions(+), 174 deletions(-)
>  create mode 100644 tools/perf/util/parse-branch-options.c
>  create mode 100644 tools/perf/util/parse-branch-options.h

Pulled, thanks a lot Arnaldo!

	Ingo

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

end of thread, other threads:[~2015-05-28  9:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28  0:05 [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 1/7] perf tools: Introduce struct maps Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 2/7] perf tools: Protect accesses the map rbtrees with a rw lock Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 3/7] perf tools: Check if a map is still in use when deleting it Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 4/7] perf tools: Reference count struct map Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 5/7] perf tools: Add hint for 'Too many events are opened.' error message Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 6/7] perf annotation: Add symbol__get_annotation Arnaldo Carvalho de Melo
2015-05-28  0:05 ` [PATCH 7/7] perf tools: Move branch option parsing to own file Arnaldo Carvalho de Melo
2015-05-28  0:27 ` [GIT PULL 0/7] perf/core refactorings and improvements Arnaldo Carvalho de Melo
2015-05-28  9:27   ` Ingo Molnar

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).