Skip to contentSkip to conversation

perf tools: Share map groups within process

PATCH In review Jiri Olsa 4 participants 17 messages Apr 14, 2014 – Apr 29, 2014 perf events

lore
Share & tools
lore

Jiri Olsa

author
#0 lore raw

v2 cover perf tools: Share map groups within process

hi,
this patchset moves thread's map_groups to be dynamically allocated and shared within process threads.

The main benefit would be to be able to look up memory map from any thread that belongs to the process.

This implements one of the solution ideas for issue described by Don in following thread:
http://marc.info/?l=linux-kernel&m=139403876017159&w=2

v2 changes:
- changing mmap groups allocation strategy - have it allocated always when thread is created, which seems less confusing and less error prone. The previous strategy was to allocate it when it's actually used.
- fix test compilation for i386

RFC changes:
- added automated test for thread map groups get/put methods
- fix reference count for case described by Namhyung
- rename the mmap-events.c test to mmap-thread-lookup.c
- added PROT_EXEC to mmap call in tests/mmap-thread-lookup.c as it's not implied by default on all archs (Namhyung)
- lazy mg allocation in thread__find_addr_map (Namhyung)
- fix compilation failures (Arnaldo)

also available in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_mmaps

thanks,
jirka

Cc Don Zickus <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>

---
Arnaldo Carvalho de Melo (2):
perf tools: Allocate thread map_groups's dynamically perf tools: Reference count map_groups objects

Jiri Olsa (3):
perf tests: Add thread maps lookup automated tests perf tools: Share map_groups among threads of the same group perf tests: Add map groups sharing with thread object test

 tools/perf/Makefile.perf                 |   2 +
 tools/perf/arch/x86/tests/dwarf-unwind.c |   2 +-
 tools/perf/perf.h                        |   6 +++
 tools/perf/tests/builtin-test.c          |   8 ++++
 tools/perf/tests/mmap-thread-lookup.c    | 233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                 |   2 +
 tools/perf/tests/thread-mg-share.c       |  90 +++++++++++++++++++++++++++++++++++
 tools/perf/ui/stdio/hist.c               |   2 +-
 tools/perf/util/event.c                  |   2 +-
 tools/perf/util/machine.c                |  11 +++++
 tools/perf/util/map.c                    |  23 +++++++++
 tools/perf/util/map.h                    |  12 +++++
 tools/perf/util/thread.c                 |  52 +++++++++++++++-----
 tools/perf/util/thread.h                 |   3 +-
 14 files changed, 432 insertions(+), 16 deletions(-)
 create mode 100644 tools/perf/tests/mmap-thread-lookup.c
 create mode 100644 tools/perf/tests/thread-mg-share.c
author
#1 lore raw

PATCH v2 1/5 perf tests: Add thread maps lookup automated tests

Adding automated test for memory maps lookup within multiple machines threads.

The test creates 4 threads and separated memory maps.

It checks that we could use thread__find_addr_map function with thread object based on TID to find memory maps.

Cc Don Zickus <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/Makefile.perf              |   1 +
 tools/perf/perf.h                     |   6 +
 tools/perf/tests/builtin-test.c       |   4 +
 tools/perf/tests/mmap-thread-lookup.c | 233 ++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h              |   1 +
 5 files changed, 245 insertions(+)
 create mode 100644 tools/perf/tests/mmap-thread-lookup.c
tools/perf/Makefile.perf +1 -0
index e969233..7bad323 100644
@@ -414,6 +414,7 @@ ifeq ($(ARCH),x86)
 LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o
 endif
 endif
+LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/perf.h +6 -0
index e18a8b5..1138c41 100644
@@ -15,6 +15,9 @@
 #ifndef __NR_futex
 # define __NR_futex 240
 #endif
+#ifndef __NR_gettid
+# define __NR_gettid 224
+#endif
 #endif
 
 #if defined(__x86_64__)
@@ -29,6 +32,9 @@
 #ifndef __NR_futex
 # define __NR_futex 202
 #endif
+#ifndef __NR_gettid
+# define __NR_gettid 186
+#endif
 #endif
 
 #ifdef __powerpc__
tools/perf/tests/builtin-test.c +4 -0
index b11bf8a..d947d1a 100644
@@ -124,6 +124,10 @@ static struct test {
 #endif
 #endif
 	{
+		.desc = "Test mmap thread lookup",
+		.func = test__mmap_thread_lookup,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/mmap-thread-lookup.c new file +233 -0

233 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index a24795c..6db764c 100644
@@ -41,6 +41,7 @@ int test__sample_parsing(void);
 int test__keep_tracking(void);
 int test__parse_no_sample_id_all(void);
 int test__dwarf_unwind(void);
+int test__mmap_thread_lookup(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
Signature · 1 line
1.8.3.1
#2 lore raw

[tip:perf/core] perf tests: Add thread maps lookup automated tests

perf tests: Add thread maps lookup automated tests
Commit-ID:  4e85edfc3f5c0e016a960c1dcbe0217e86602525
Gitweb:     http://git.kernel.org/tip/4e85edfc3f5c0e016a960c1dcbe0217e86602525
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 5 Mar 2014 17:20:31 +0100
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 28 Apr 2014 13:42:52 +0200

perf tests: Add thread maps lookup automated tests

Adding automated test for memory maps lookup within multiple machines threads.

The test creates 4 threads and separated memory maps. It checks that we could use thread__find_addr_map function with thread object based on TID to find memory maps.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/Makefile.perf              |   1 +
 tools/perf/perf.h                     |   6 +
 tools/perf/tests/builtin-test.c       |   4 +
 tools/perf/tests/mmap-thread-lookup.c | 233 ++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h              |   1 +
 5 files changed, 245 insertions(+)
tools/perf/Makefile.perf +1 -0
index 0807e4c..16d4f4e 100644
@@ -416,6 +416,7 @@ ifeq ($(ARCH),x86)
 LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o
 endif
 endif
+LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/perf.h +6 -0
index 5c11eca..ebdad33 100644
@@ -15,6 +15,9 @@
 #ifndef __NR_futex
 # define __NR_futex 240
 #endif
+#ifndef __NR_gettid
+# define __NR_gettid 224
+#endif
 #endif
 
 #if defined(__x86_64__)
@@ -29,6 +32,9 @@
 #ifndef __NR_futex
 # define __NR_futex 202
 #endif
+#ifndef __NR_gettid
+# define __NR_gettid 186
+#endif
 #endif
 
 #ifdef __powerpc__
tools/perf/tests/builtin-test.c +4 -0
index ceb9dae..bb60792 100644
@@ -128,6 +128,10 @@ static struct test {
 		.func = test__hists_filter,
 	},
 	{
+		.desc = "Test mmap thread lookup",
+		.func = test__mmap_thread_lookup,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/mmap-thread-lookup.c new file +233 -0

233 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index fe39163..82e8061 100644
@@ -42,6 +42,7 @@ int test__keep_tracking(void);
 int test__parse_no_sample_id_all(void);
 int test__dwarf_unwind(void);
 int test__hists_filter(void);
+int test__mmap_thread_lookup(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
author
#3 lore raw

PATCH v2 2/5 perf tools: Allocate thread map_groups's dynamically

In reply to Jiri Olsa

From: Arnaldo Carvalho de Melo <[email protected]>

Moving towards sharing map groups within a process threads.

Because of this we need the map groups to be dynamically allocated. No other functional change is intended in here.

Based on a patch by Jiri Olsa, but this time _just_ making the conversion from statically allocating thread->mg to turning it into a pointer and instead of initializing it at thread's constructor, introduce a constructor/destructor for the map_groups class and call at thread creation time.

Later we will introduce the get/put methods when we move to sharing those map_groups, when the get/put refcounting semantics will be needed.

Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/arch/x86/tests/dwarf-unwind.c |  2 +-
 tools/perf/ui/stdio/hist.c               |  2 +-
 tools/perf/util/event.c                  |  2 +-
 tools/perf/util/map.c                    | 16 ++++++++++++++++
 tools/perf/util/map.h                    |  3 +++
 tools/perf/util/thread.c                 | 18 ++++++++++++------
 tools/perf/util/thread.h                 |  2 +-
 7 files changed, 35 insertions(+), 10 deletions(-)
tools/perf/arch/x86/tests/dwarf-unwind.c +1 -1
index b602ad9..d804511 100644
@@ -23,7 +23,7 @@ static int sample_ustack(struct perf_sample *sample,
 
 	sp = (unsigned long) regs[PERF_REG_X86_SP];
 
-	map = map_groups__find(&thread->mg, MAP__FUNCTION, (u64) sp);
+	map = map_groups__find(thread->mg, MAP__FUNCTION, (u64) sp);
 	if (!map) {
 		pr_debug("failed to get stack map\n");
 		return -1;
tools/perf/ui/stdio/hist.c +1 -1
index d59893e..9eccf7f 100644
@@ -495,7 +495,7 @@ print_entries:
 			break;
 
 		if (h->ms.map == NULL && verbose > 1) {
-			__map_groups__fprintf_maps(&h->thread->mg,
+			__map_groups__fprintf_maps(h->thread->mg,
 						   MAP__FUNCTION, verbose, fp);
 			fprintf(fp, "%.10s end\n", graph_dotted_line);
 		}
tools/perf/util/event.c +1 -1
index 9d12aa6..dbcaea1 100644
@@ -699,7 +699,7 @@ void thread__find_addr_map(struct thread *thread,
 			   enum map_type type, u64 addr,
 			   struct addr_location *al)
 {
-	struct map_groups *mg = &thread->mg;
+	struct map_groups *mg = thread->mg;
 	bool load_map = false;
 
 	al->machine = machine;
tools/perf/util/map.c +16 -0
index 39cd2d0..ae4c5e1 100644
@@ -358,6 +358,22 @@ void map_groups__exit(struct map_groups *mg)
 	}
 }
 
+struct map_groups *map_groups__new(void)
+{
+	struct map_groups *mg = malloc(sizeof(*mg));
+
+	if (mg != NULL)
+		map_groups__init(mg);
+
+	return mg;
+}
+
+void map_groups__delete(struct map_groups *mg)
+{
+	map_groups__exit(mg);
+	free(mg);
+}
+
 void map_groups__flush(struct map_groups *mg)
 {
 	int type;
tools/perf/util/map.h +3 -0
index f00f058..1073e2d 100644
@@ -61,6 +61,9 @@ struct map_groups {
 	struct machine	 *machine;
 };
 
+struct map_groups *map_groups__new(void);
+void map_groups__delete(struct map_groups *mg);
+
 static inline struct kmap *map__kmap(struct map *map)
 {
 	return (struct kmap *)(map + 1);
tools/perf/util/thread.c +12 -6
index 3ce0498..dc51d16 100644
@@ -15,7 +15,10 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	struct thread *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
-		map_groups__init(&thread->mg);
+		thread->mg = map_groups__new();
+		if (thread->mg == NULL)
+			goto out_free;
+
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
@@ -37,6 +40,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	return thread;
 
 err_thread:
+	map_groups__delete(thread->mg);
+out_free:
 	free(thread);
 	return NULL;
 }
@@ -45,7 +50,8 @@ void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
 
-	map_groups__exit(&thread->mg);
+	map_groups__delete(thread->mg);
+	thread->mg = NULL;
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
 		comm__free(comm);
@@ -111,13 +117,13 @@ int thread__comm_len(struct thread *thread)
 size_t thread__fprintf(struct thread *thread, FILE *fp)
 {
 	return fprintf(fp, "Thread %d %s\n", thread->tid, thread__comm_str(thread)) +
-	       map_groups__fprintf(&thread->mg, verbose, fp);
+	       map_groups__fprintf(thread->mg, verbose, fp);
 }
 
 void thread__insert_map(struct thread *thread, struct map *map)
 {
-	map_groups__fixup_overlappings(&thread->mg, map, verbose, stderr);
-	map_groups__insert(&thread->mg, map);
+	map_groups__fixup_overlappings(thread->mg, map, verbose, stderr);
+	map_groups__insert(thread->mg, map);
 }
 
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
@@ -135,7 +141,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 	}
 
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(&thread->mg, &parent->mg, i) < 0)
+		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
 			return -ENOMEM;
 
 	thread->ppid = parent->tid;
tools/perf/util/thread.h +1 -1
index 9b29f08..bee1eb0 100644
@@ -13,7 +13,7 @@ struct thread {
 		struct rb_node	 rb_node;
 		struct list_head node;
 	};
-	struct map_groups	mg;
+	struct map_groups	*mg;
 	pid_t			pid_; /* Not all tools update this */
 	pid_t			tid;
 	pid_t			ppid;
Signature · 1 line
1.8.3.1
#4 lore raw

[tip:perf/core] perf tools: Allocate thread map_groups' s dynamically

perf tools: Allocate thread map_groups's dynamically
Commit-ID:  93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45
Gitweb:     http://git.kernel.org/tip/93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45
Author:     Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Fri, 21 Mar 2014 17:57:01 -0300
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 28 Apr 2014 13:43:20 +0200

perf tools: Allocate thread map_groups's dynamically

Moving towards sharing map groups within a process threads.

Because of this we need the map groups to be dynamically allocated. No other functional change is intended in here.

Based on a patch by Jiri Olsa, but this time _just_ making the conversion from statically allocating thread->mg to turning it into a pointer and instead of initializing it at thread's constructor, introduce a constructor/destructor for the map_groups class and call at thread creation time.

Later we will introduce the get/put methods when we move to sharing those map_groups, when the get/put refcounting semantics will be needed.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/arch/x86/tests/dwarf-unwind.c |  2 +-
 tools/perf/ui/stdio/hist.c               |  2 +-
 tools/perf/util/event.c                  |  2 +-
 tools/perf/util/map.c                    | 16 ++++++++++++++++
 tools/perf/util/map.h                    |  3 +++
 tools/perf/util/thread.c                 | 18 ++++++++++++------
 tools/perf/util/thread.h                 |  2 +-
 7 files changed, 35 insertions(+), 10 deletions(-)
tools/perf/arch/x86/tests/dwarf-unwind.c +1 -1
index b8c0102..c916656 100644
@@ -23,7 +23,7 @@ static int sample_ustack(struct perf_sample *sample,
 
 	sp = (unsigned long) regs[PERF_REG_X86_SP];
 
-	map = map_groups__find(&thread->mg, MAP__FUNCTION, (u64) sp);
+	map = map_groups__find(thread->mg, MAP__FUNCTION, (u64) sp);
 	if (!map) {
 		pr_debug("failed to get stack map\n");
 		free(buf);
tools/perf/ui/stdio/hist.c +1 -1
index d59893e..9eccf7f 100644
@@ -495,7 +495,7 @@ print_entries:
 			break;
 
 		if (h->ms.map == NULL && verbose > 1) {
-			__map_groups__fprintf_maps(&h->thread->mg,
+			__map_groups__fprintf_maps(h->thread->mg,
 						   MAP__FUNCTION, verbose, fp);
 			fprintf(fp, "%.10s end\n", graph_dotted_line);
 		}
tools/perf/util/event.c +1 -1
index 9d12aa6..dbcaea1 100644
@@ -699,7 +699,7 @@ void thread__find_addr_map(struct thread *thread,
 			   enum map_type type, u64 addr,
 			   struct addr_location *al)
 {
-	struct map_groups *mg = &thread->mg;
+	struct map_groups *mg = thread->mg;
 	bool load_map = false;
 
 	al->machine = machine;
tools/perf/util/map.c +16 -0
index 39cd2d0..ae4c5e1 100644
@@ -358,6 +358,22 @@ void map_groups__exit(struct map_groups *mg)
 	}
 }
 
+struct map_groups *map_groups__new(void)
+{
+	struct map_groups *mg = malloc(sizeof(*mg));
+
+	if (mg != NULL)
+		map_groups__init(mg);
+
+	return mg;
+}
+
+void map_groups__delete(struct map_groups *mg)
+{
+	map_groups__exit(mg);
+	free(mg);
+}
+
 void map_groups__flush(struct map_groups *mg)
 {
 	int type;
tools/perf/util/map.h +3 -0
index f00f058..1073e2d 100644
@@ -61,6 +61,9 @@ struct map_groups {
 	struct machine	 *machine;
 };
 
+struct map_groups *map_groups__new(void);
+void map_groups__delete(struct map_groups *mg);
+
 static inline struct kmap *map__kmap(struct map *map)
 {
 	return (struct kmap *)(map + 1);
tools/perf/util/thread.c +12 -6
index 3ce0498..dc51d16 100644
@@ -15,7 +15,10 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	struct thread *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
-		map_groups__init(&thread->mg);
+		thread->mg = map_groups__new();
+		if (thread->mg == NULL)
+			goto out_free;
+
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
@@ -37,6 +40,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	return thread;
 
 err_thread:
+	map_groups__delete(thread->mg);
+out_free:
 	free(thread);
 	return NULL;
 }
@@ -45,7 +50,8 @@ void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
 
-	map_groups__exit(&thread->mg);
+	map_groups__delete(thread->mg);
+	thread->mg = NULL;
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
 		comm__free(comm);
@@ -111,13 +117,13 @@ int thread__comm_len(struct thread *thread)
 size_t thread__fprintf(struct thread *thread, FILE *fp)
 {
 	return fprintf(fp, "Thread %d %s\n", thread->tid, thread__comm_str(thread)) +
-	       map_groups__fprintf(&thread->mg, verbose, fp);
+	       map_groups__fprintf(thread->mg, verbose, fp);
 }
 
 void thread__insert_map(struct thread *thread, struct map *map)
 {
-	map_groups__fixup_overlappings(&thread->mg, map, verbose, stderr);
-	map_groups__insert(&thread->mg, map);
+	map_groups__fixup_overlappings(thread->mg, map, verbose, stderr);
+	map_groups__insert(thread->mg, map);
 }
 
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
@@ -135,7 +141,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 	}
 
 	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(&thread->mg, &parent->mg, i) < 0)
+		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
 			return -ENOMEM;
 
 	thread->ppid = parent->tid;
tools/perf/util/thread.h +1 -1
index 9b29f08..bee1eb0 100644
@@ -13,7 +13,7 @@ struct thread {
 		struct rb_node	 rb_node;
 		struct list_head node;
 	};
-	struct map_groups	mg;
+	struct map_groups	*mg;
 	pid_t			pid_; /* Not all tools update this */
 	pid_t			tid;
 	pid_t			ppid;
author
#5 lore raw

PATCH v2 3/5 perf tools: Reference count map_groups objects

In reply to Jiri Olsa

From: Arnaldo Carvalho de Melo <[email protected]>

We will share it among threads in the same process.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/map.c    | 7 +++++++
 tools/perf/util/map.h    | 9 +++++++++
 tools/perf/util/thread.c | 2 +-
 3 files changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/map.c +7 -0
index ae4c5e1..ba5f5c0c 100644
@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)
 		INIT_LIST_HEAD(&mg->removed_maps[i]);
 	}
 	mg->machine = NULL;
+	mg->refcnt = 1;
 }
 
 static void maps__delete(struct rb_root *maps)
@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)
 	free(mg);
 }
 
+void map_groups__put(struct map_groups *mg)
+{
+	if (--mg->refcnt == 0)
+		map_groups__delete(mg);
+}
+
 void map_groups__flush(struct map_groups *mg)
 {
 	int type;
tools/perf/util/map.h +9 -0
index 1073e2d..d6445b2 100644
@@ -59,11 +59,20 @@ struct map_groups {
 	struct rb_root	 maps[MAP__NR_TYPES];
 	struct list_head removed_maps[MAP__NR_TYPES];
 	struct machine	 *machine;
+	int		 refcnt;
 };
 
 struct map_groups *map_groups__new(void);
 void map_groups__delete(struct map_groups *mg);
 
+static inline struct map_groups *map_groups__get(struct map_groups *mg)
+{
+	++mg->refcnt;
+	return mg;
+}
+
+void map_groups__put(struct map_groups *mg);
+
 static inline struct kmap *map__kmap(struct map *map)
 {
 	return (struct kmap *)(map + 1);
tools/perf/util/thread.c +1 -1
index dc51d16..b501848 100644
@@ -50,7 +50,7 @@ void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
 
-	map_groups__delete(thread->mg);
+	map_groups__put(thread->mg);
 	thread->mg = NULL;
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
Signature · 1 line
1.8.3.1
#6 lore raw

Re: [PATCH 3/5] perf tools: Reference count map_groups objects

Quoted 51 lines · Jiri Olsa

From: Arnaldo Carvalho de Melo <[email protected]>

We will share it among threads in the same process.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
  tools/perf/util/map.c    | 7 +++++++
  tools/perf/util/map.h    | 9 +++++++++
  tools/perf/util/thread.c | 2 +-
  3 files changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/map.c +7 -0
index ae4c5e1..ba5f5c0c 100644
@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)
  		INIT_LIST_HEAD(&mg->removed_maps[i]);
  	}
  	mg->machine = NULL;
+	mg->refcnt = 1;
  }
 
  static void maps__delete(struct rb_root *maps)
@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)
  	free(mg);
  }
 
+void map_groups__put(struct map_groups *mg)
+{
+	if (--mg->refcnt == 0)
+		map_groups__delete(mg);
+}
+
  void map_groups__flush(struct map_groups *mg)
  {
  	int type;
tools/perf/util/map.h +0 -0
index 1073e2d..d6445b2 100644

tools/perf/util/map.h struct map_groups { · line 63

Quoted 5 lines
tools/perf/util/map.h +1 -0
@@ -59,11 +59,20 @@ struct map_groups {
  	struct rb_root	 maps[MAP__NR_TYPES];
  	struct list_head removed_maps[MAP__NR_TYPES];
  	struct machine	 *machine;
+	int		 refcnt;
  };

atomic for refcnt? This is part of a libperf; would be good to support multithreaded users.

David

author
#7 lore raw

Re: [PATCH 3/5] perf tools: Reference count map_groups objects

Quoted 1 line · David Ahern

On 4/14/14, 9:52 AM, Jiri Olsa wrote:

SNIP

Quoted 16 lines
Quoted 16 lines
tools/perf/util/map.c +6 -0
+void map_groups__put(struct map_groups *mg)
+{
+	if (--mg->refcnt == 0)
+		map_groups__delete(mg);
+}
+
 void map_groups__flush(struct map_groups *mg)
 {
 	int type;
tools/perf/util/map.h +0 -0
index 1073e2d..d6445b2 100644
@@ -59,11 +59,20 @@ struct map_groups {
 	struct rb_root	 maps[MAP__NR_TYPES];
Quoted 8 lines

tools/perf/util/map.h struct map_groups { · line 63

Quoted 4 lines
tools/perf/util/map.h +1 -0
 	struct list_head removed_maps[MAP__NR_TYPES];
 	struct machine	 *machine;
+	int		 refcnt;
 };

atomic for refcnt? This is part of a libperf; would be good to support multithreaded users.

hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well

not sure what's the support in user space for that.. will check

also not to be negative, but libperf is not thread safe anyway, right? ;-)

thanks,
jirka

#8 lore raw

Re: [PATCH 3/5] perf tools: Reference count map_groups objects

Quoted 7 lines · Jiri Olsa

hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well

not sure what's the support in user space for that.. will check

also not to be negative, but libperf is not thread safe anyway, right? ;-)

should be. pretty certain there have been bug reports (e.g, perf-top) that stumble onto problems and they get fixed.

David

author
#9 lore raw

Re: [PATCH 3/5] perf tools: Reference count map_groups objects

Quoted 11 lines · David Ahern
Quoted 7 lines · Jiri Olsa

hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well

not sure what's the support in user space for that.. will check

also not to be negative, but libperf is not thread safe anyway, right? ;-)

should be. pretty certain there have been bug reports (e.g, perf-top) that stumble onto problems and they get fixed.

I checked and there's no problem with this in perf top, because all thread allocations and frees (included map_groups) are done within the main (one) thread

You could get in trouble if you'd call map_groups__get and map_groups__put over same map_groups object from different threads, but this never happens in perf

Thats the same sort of issue you would get if you called machine__findnew_thread over same machine from 2 threads

I think that potential libperf users should be aware of the proper usage or they should ask for change

thoughts? ;-) thanks,
jirka

#10 lore raw

Re: [PATCH 3/5] perf tools: Reference count map_groups objects

Quoted 1 line · Jiri Olsa

thoughts?;-) thanks,

Only that libperf is still a pipe dream. Don't hold up progress for this, but we should be thinking about making it thread safe.

David

#11 lore raw

[tip:perf/core] perf tools: Reference count map_groups objects

In reply to Jiri Olsa
perf tools: Reference count map_groups objects
Commit-ID:  a26ca6716a6c683f40bd676cea7e89704653b98d
Gitweb:     http://git.kernel.org/tip/a26ca6716a6c683f40bd676cea7e89704653b98d
Author:     Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Tue, 25 Mar 2014 15:26:44 -0300
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 28 Apr 2014 13:43:26 +0200

perf tools: Reference count map_groups objects

We will share it among threads in the same process. Adding map_groups__get/map_groups__put interface for that.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/map.c    | 7 +++++++
 tools/perf/util/map.h    | 9 +++++++++
 tools/perf/util/thread.c | 2 +-
 3 files changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/map.c +7 -0
index ae4c5e1..ba5f5c0c 100644
@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)
 		INIT_LIST_HEAD(&mg->removed_maps[i]);
 	}
 	mg->machine = NULL;
+	mg->refcnt = 1;
 }
 
 static void maps__delete(struct rb_root *maps)
@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)
 	free(mg);
 }
 
+void map_groups__put(struct map_groups *mg)
+{
+	if (--mg->refcnt == 0)
+		map_groups__delete(mg);
+}
+
 void map_groups__flush(struct map_groups *mg)
 {
 	int type;
tools/perf/util/map.h +9 -0
index 1073e2d..d6445b2 100644
@@ -59,11 +59,20 @@ struct map_groups {
 	struct rb_root	 maps[MAP__NR_TYPES];
 	struct list_head removed_maps[MAP__NR_TYPES];
 	struct machine	 *machine;
+	int		 refcnt;
 };
 
 struct map_groups *map_groups__new(void);
 void map_groups__delete(struct map_groups *mg);
 
+static inline struct map_groups *map_groups__get(struct map_groups *mg)
+{
+	++mg->refcnt;
+	return mg;
+}
+
+void map_groups__put(struct map_groups *mg);
+
 static inline struct kmap *map__kmap(struct map *map)
 {
 	return (struct kmap *)(map + 1);
tools/perf/util/thread.c +1 -1
index dc51d16..b501848 100644
@@ -50,7 +50,7 @@ void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
 
-	map_groups__delete(thread->mg);
+	map_groups__put(thread->mg);
 	thread->mg = NULL;
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
author
#12 lore raw

PATCH v2 4/5 perf tools: Share map_groups among threads of the same group

In reply to Jiri Olsa

Sharing map groups within all process threads. This way there's only one copy of mmap info and it's reachable from any thread within the process.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 11 +++++++++++
 tools/perf/util/thread.c  | 48 ++++++++++++++++++++++++++++++++++-------------
 tools/perf/util/thread.h  |  1 +
 3 files changed, 47 insertions(+), 13 deletions(-)
tools/perf/util/machine.c +11 -0
index 116842e..e86545a 100644
@@ -316,6 +316,17 @@ static struct thread *__machine__findnew_thread(struct machine *machine,
 		rb_link_node(&th->rb_node, parent, p);
 		rb_insert_color(&th->rb_node, &machine->threads);
 		machine->last_match = th;
+
+		/*
+		 * We have to initialize map_groups separately
+		 * after rb tree is updated.
+		 *
+		 * The reason is that we call machine__findnew_thread
+		 * within thread__init_map_groups to find the thread
+		 * leader and that would screwed the rb tree.
+		 */
+		if (thread__init_map_groups(th, machine))
+			return NULL;
 	}
 
 	return th;
tools/perf/util/thread.c +35 -13

81 lines not on this page · raw message

tools/perf/util/thread.h +1 -0
index bee1eb0..3c0c272 100644
@@ -30,6 +30,7 @@ struct machine;
 struct comm;
 
 struct thread *thread__new(pid_t pid, pid_t tid);
+int thread__init_map_groups(struct thread *thread, struct machine *machine);
 void thread__delete(struct thread *thread);
 static inline void thread__exited(struct thread *thread)
 {
Signature · 1 line
1.8.3.1
#13 lore raw

[tip:perf/core] perf tools: Share map_groups among threads of the same group

perf tools: Share map_groups among threads of the same group
Commit-ID:  cddcef607782966f1601808c17fe9c4c5f79f9f4
Gitweb:     http://git.kernel.org/tip/cddcef607782966f1601808c17fe9c4c5f79f9f4
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 9 Apr 2014 20:54:29 +0200
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 28 Apr 2014 13:43:33 +0200

perf tools: Share map_groups among threads of the same group

Sharing map groups within all process threads. This way there's only one copy of mmap info and it's reachable from any thread within the process.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 11 +++++++++++
 tools/perf/util/thread.c  | 48 ++++++++++++++++++++++++++++++++++-------------
 tools/perf/util/thread.h  |  1 +
 3 files changed, 47 insertions(+), 13 deletions(-)
tools/perf/util/machine.c +11 -0
index a53cd0b..98ec56d 100644
@@ -316,6 +316,17 @@ static struct thread *__machine__findnew_thread(struct machine *machine,
 		rb_link_node(&th->rb_node, parent, p);
 		rb_insert_color(&th->rb_node, &machine->threads);
 		machine->last_match = th;
+
+		/*
+		 * We have to initialize map_groups separately
+		 * after rb tree is updated.
+		 *
+		 * The reason is that we call machine__findnew_thread
+		 * within thread__init_map_groups to find the thread
+		 * leader and that would screwed the rb tree.
+		 */
+		if (thread__init_map_groups(th, machine))
+			return NULL;
 	}
 
 	return th;
tools/perf/util/thread.c +35 -13

81 lines not on this page · raw message

tools/perf/util/thread.h +1 -0
index bee1eb0..3c0c272 100644
@@ -30,6 +30,7 @@ struct machine;
 struct comm;
 
 struct thread *thread__new(pid_t pid, pid_t tid);
+int thread__init_map_groups(struct thread *thread, struct machine *machine);
 void thread__delete(struct thread *thread);
 static inline void thread__exited(struct thread *thread)
 {
author
#14 lore raw

PATCH v2 5/5 perf tests: Add map groups sharing with thread object test

In reply to Jiri Olsa

This test create 2 processes abstractions, with several threads and checks they properly share and maintain map groups info.

Cc Don Zickus <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/Makefile.perf           |  1 +
 tools/perf/tests/builtin-test.c    |  4 ++
 tools/perf/tests/tests.h           |  1 +
 tools/perf/tests/thread-mg-share.c | 90 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
 create mode 100644 tools/perf/tests/thread-mg-share.c
tools/perf/Makefile.perf +1 -0
index 7bad323..eba6a61 100644
@@ -415,6 +415,7 @@ LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o
 endif
 endif
 LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
+LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
index d947d1a..a1145d1 100644
@@ -128,6 +128,10 @@ static struct test {
 		.func = test__mmap_thread_lookup,
 	},
 	{
+		.desc = "Test thread mg sharing",
+		.func = test__thread_mg_share,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/tests.h +1 -0
index 6db764c..5ed3bd5 100644
@@ -42,6 +42,7 @@ int test__keep_tracking(void);
 int test__parse_no_sample_id_all(void);
 int test__dwarf_unwind(void);
 int test__mmap_thread_lookup(void);
+int test__thread_mg_share(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/thread-mg-share.c new file +90 -0

90 lines not on this page · raw message

Signature · 1 line
1.8.3.1
maintainer Acked-by on v2 5/5
#15 lore raw

Re: [PATCH 5/5] perf tests: Add map groups sharing with thread object test

2014-04-14 (월), 17:52 +0200, Jiri Olsa:

tools/perf/tests/thread-mg-share.c line 85

Quoted 3 lines
tools/perf/tests/thread-mg-share.c new file +3 -0
+	/*
+	 * Cannot call machine__delete_threads(machine) now,
+	 * because we've already realased all the threads.

s/realased/released/

Other than that, this series looks good to me.

Thanks,
Namhyung

Quoted 5 lines
tools/perf/tests/thread-mg-share.c new file +5 -0
+	 */
+
+	machines__exit(&machines);
+	return 0;
+}
#16 lore raw

[tip:perf/core] perf tests: Add map groups sharing with thread object test

In reply to Jiri Olsa
perf tests: Add map groups sharing with thread object test
Commit-ID:  fabf01238289e9ae009499594fc54642f5802a24
Gitweb:     http://git.kernel.org/tip/fabf01238289e9ae009499594fc54642f5802a24
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 17 Mar 2014 14:39:00 +0100
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 28 Apr 2014 13:43:40 +0200

perf tests: Add map groups sharing with thread object test

This test create 2 processes abstractions, with several threads and checks they properly share and maintain map groups info.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/Makefile.perf           |  1 +
 tools/perf/tests/builtin-test.c    |  4 ++
 tools/perf/tests/tests.h           |  1 +
 tools/perf/tests/thread-mg-share.c | 90 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 96 insertions(+)
tools/perf/Makefile.perf +1 -0
index 16d4f4e..46e0c32 100644
@@ -417,6 +417,7 @@ LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o
 endif
 endif
 LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
+LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
index bb60792..0d5afaf 100644
@@ -132,6 +132,10 @@ static struct test {
 		.func = test__mmap_thread_lookup,
 	},
 	{
+		.desc = "Test thread mg sharing",
+		.func = test__thread_mg_share,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/tests.h +1 -0
index 82e8061..a9d7cb0 100644
@@ -43,6 +43,7 @@ int test__parse_no_sample_id_all(void);
 int test__dwarf_unwind(void);
 int test__hists_filter(void);
 int test__mmap_thread_lookup(void);
+int test__thread_mg_share(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/thread-mg-share.c new file +90 -0

90 lines not on this page · raw message