All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2 0/5] perf tools: Share map groups within process
@ 2014-04-14 15:51 Jiri Olsa
  2014-04-14 15:51 ` [PATCH 1/5] perf tests: Add thread maps lookup automated tests Jiri Olsa
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Don Zickus, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra, Arnaldo Carvalho de Melo

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


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---
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

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

* [PATCH 1/5] perf tests: Add thread maps lookup automated tests
  2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
@ 2014-04-14 15:51 ` Jiri Olsa
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2014-04-14 15:52 ` [PATCH 2/5] perf tools: Allocate thread map_groups's dynamically Jiri Olsa
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Don Zickus, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra

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 <dzickus@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index e969233..7bad323 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e18a8b5..1138c41 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -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__
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index b11bf8a..d947d1a 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -124,6 +124,10 @@ static struct test {
 #endif
 #endif
 	{
+		.desc = "Test mmap thread lookup",
+		.func = test__mmap_thread_lookup,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
new file mode 100644
index 0000000..4a456fe
--- /dev/null
+++ b/tools/perf/tests/mmap-thread-lookup.c
@@ -0,0 +1,233 @@
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include "debug.h"
+#include "tests.h"
+#include "machine.h"
+#include "thread_map.h"
+#include "symbol.h"
+#include "thread.h"
+
+#define THREADS 4
+
+static int go_away;
+
+struct thread_data {
+	pthread_t	pt;
+	pid_t		tid;
+	void		*map;
+	int		ready[2];
+};
+
+static struct thread_data threads[THREADS];
+
+static int thread_init(struct thread_data *td)
+{
+	void *map;
+
+	map = mmap(NULL, page_size,
+		   PROT_READ|PROT_WRITE|PROT_EXEC,
+		   MAP_SHARED|MAP_ANONYMOUS, -1, 0);
+
+	if (map == MAP_FAILED) {
+		perror("mmap failed");
+		return -1;
+	}
+
+	td->map = map;
+	td->tid = syscall(SYS_gettid);
+
+	pr_debug("tid = %d, map = %p\n", td->tid, map);
+	return 0;
+}
+
+static void *thread_fn(void *arg)
+{
+	struct thread_data *td = arg;
+	ssize_t ret;
+	int go;
+
+	if (thread_init(td))
+		return NULL;
+
+	/* Signal thread_create thread is initialized. */
+	ret = write(td->ready[1], &go, sizeof(int));
+	if (ret != sizeof(int)) {
+		pr_err("failed to notify\n");
+		return NULL;
+	}
+
+	while (!go_away) {
+		/* Waiting for main thread to kill us. */
+		usleep(100);
+	}
+
+	munmap(td->map, page_size);
+	return NULL;
+}
+
+static int thread_create(int i)
+{
+	struct thread_data *td = &threads[i];
+	int err, go;
+
+	if (pipe(td->ready))
+		return -1;
+
+	err = pthread_create(&td->pt, NULL, thread_fn, td);
+	if (!err) {
+		/* Wait for thread initialization. */
+		ssize_t ret = read(td->ready[0], &go, sizeof(int));
+		err = ret != sizeof(int);
+	}
+
+	close(td->ready[0]);
+	close(td->ready[1]);
+	return err;
+}
+
+static int threads_create(void)
+{
+	struct thread_data *td0 = &threads[0];
+	int i, err = 0;
+
+	go_away = 0;
+
+	/* 0 is main thread */
+	if (thread_init(td0))
+		return -1;
+
+	for (i = 1; !err && i < THREADS; i++)
+		err = thread_create(i);
+
+	return err;
+}
+
+static int threads_destroy(void)
+{
+	struct thread_data *td0 = &threads[0];
+	int i, err = 0;
+
+	/* cleanup the main thread */
+	munmap(td0->map, page_size);
+
+	go_away = 1;
+
+	for (i = 1; !err && i < THREADS; i++)
+		err = pthread_join(threads[i].pt, NULL);
+
+	return err;
+}
+
+typedef int (*synth_cb)(struct machine *machine);
+
+static int synth_all(struct machine *machine)
+{
+	return perf_event__synthesize_threads(NULL,
+					      perf_event__process,
+					      machine, 0);
+}
+
+static int synth_process(struct machine *machine)
+{
+	struct thread_map *map;
+	int err;
+
+	map = thread_map__new_by_pid(getpid());
+
+	err = perf_event__synthesize_thread_map(NULL, map,
+						perf_event__process,
+						machine, 0);
+
+	thread_map__delete(map);
+	return err;
+}
+
+static int mmap_events(synth_cb synth)
+{
+	struct machines machines;
+	struct machine *machine;
+	int err, i;
+
+	/*
+	 * The threads_create will not return before all threads
+	 * are spawned and all created memory map.
+	 *
+	 * They will loop until threads_destroy is called, so we
+	 * can safely run synthesizing function.
+	 */
+	TEST_ASSERT_VAL("failed to create threads", !threads_create());
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	dump_trace = verbose > 1 ? 1 : 0;
+
+	err = synth(machine);
+
+	dump_trace = 0;
+
+	TEST_ASSERT_VAL("failed to destroy threads", !threads_destroy());
+	TEST_ASSERT_VAL("failed to synthesize maps", !err);
+
+	/*
+	 * All data is synthesized, try to find map for each
+	 * thread object.
+	 */
+	for (i = 0; i < THREADS; i++) {
+		struct thread_data *td = &threads[i];
+		struct addr_location al;
+		struct thread *thread;
+
+		thread = machine__findnew_thread(machine, getpid(), td->tid);
+
+		pr_debug("looking for map %p\n", td->map);
+
+		thread__find_addr_map(thread, machine,
+				      PERF_RECORD_MISC_USER, MAP__FUNCTION,
+				      (unsigned long) (td->map + 1), &al);
+
+		if (!al.map) {
+			pr_debug("failed, couldn't find map\n");
+			err = -1;
+			break;
+		}
+
+		pr_debug("map %p, addr %" PRIx64 "\n", al.map, al.map->start);
+	}
+
+	machine__delete_threads(machine);
+	machines__exit(&machines);
+	return err;
+}
+
+/*
+ * This test creates 'THREADS' number of threads (including
+ * main thread) and each thread creates memory map.
+ *
+ * When threads are created, we synthesize them with both
+ * (separate tests):
+ *   perf_event__synthesize_thread_map (process based)
+ *   perf_event__synthesize_threads    (global)
+ *
+ * We test we can find all memory maps via:
+ *   thread__find_addr_map
+ *
+ * by using all thread objects.
+ */
+int test__mmap_thread_lookup(void)
+{
+	/* perf_event__synthesize_threads synthesize */
+	TEST_ASSERT_VAL("failed with sythesizing all",
+			!mmap_events(synth_all));
+
+	/* perf_event__synthesize_thread_map synthesize */
+	TEST_ASSERT_VAL("failed with sythesizing process",
+			!mmap_events(synth_process));
+
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index a24795c..6db764c 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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
-- 
1.8.3.1


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

* [PATCH 2/5] perf tools: Allocate thread map_groups's dynamically
  2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
  2014-04-14 15:51 ` [PATCH 1/5] perf tests: Add thread maps lookup automated tests Jiri Olsa
@ 2014-04-14 15:52 ` Jiri Olsa
  2014-04-29  6:46   ` [tip:perf/core] perf tools: Allocate thread map_groups' s dynamically tip-bot for Arnaldo Carvalho de Melo
  2014-04-14 15:52 ` [PATCH 3/5] perf tools: Reference count map_groups objects Jiri Olsa
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Corey Ashford, David Ahern, Don Zickus,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra, Jiri Olsa

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

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.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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(-)

diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c
index b602ad9..d804511 100644
--- a/tools/perf/arch/x86/tests/dwarf-unwind.c
+++ b/tools/perf/arch/x86/tests/dwarf-unwind.c
@@ -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;
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index d59893e..9eccf7f 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -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);
 		}
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9d12aa6..dbcaea1 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -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;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 39cd2d0..ae4c5e1 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -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;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f00f058..1073e2d 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -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);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 3ce0498..dc51d16 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -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;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 9b29f08..bee1eb0 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -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;
-- 
1.8.3.1


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

* [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
  2014-04-14 15:51 ` [PATCH 1/5] perf tests: Add thread maps lookup automated tests Jiri Olsa
  2014-04-14 15:52 ` [PATCH 2/5] perf tools: Allocate thread map_groups's dynamically Jiri Olsa
@ 2014-04-14 15:52 ` Jiri Olsa
  2014-04-14 17:22   ` David Ahern
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Arnaldo Carvalho de Melo
  2014-04-14 15:52 ` [PATCH 4/5] perf tools: Share map_groups among threads of the same group Jiri Olsa
  2014-04-14 15:52 ` [PATCH 5/5] perf tests: Add map groups sharing with thread object test Jiri Olsa
  4 siblings, 2 replies; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, David Ahern, Don Zickus,
	Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian, Jiri Olsa

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

We will share it among threads in the same process.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index ae4c5e1..ba5f5c0c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -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;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 1073e2d..d6445b2 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -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);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index dc51d16..b501848 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -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);
-- 
1.8.3.1


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

* [PATCH 4/5] perf tools: Share map_groups among threads of the same group
  2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
                   ` (2 preceding siblings ...)
  2014-04-14 15:52 ` [PATCH 3/5] perf tools: Reference count map_groups objects Jiri Olsa
@ 2014-04-14 15:52 ` Jiri Olsa
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
  2014-04-14 15:52 ` [PATCH 5/5] perf tests: Add map groups sharing with thread object test Jiri Olsa
  4 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Adrian Hunter, David Ahern, Don Zickus,
	Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

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.

Original-patch-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 116842e..e86545a 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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;
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index b501848..2fde0d5 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -8,6 +8,22 @@
 #include "debug.h"
 #include "comm.h"
 
+int thread__init_map_groups(struct thread *thread, struct machine *machine)
+{
+	struct thread *leader;
+	pid_t pid = thread->pid_;
+
+	if (pid == thread->tid) {
+		thread->mg = map_groups__new();
+	} else {
+		leader = machine__findnew_thread(machine, pid, pid);
+		if (leader)
+			thread->mg = map_groups__get(leader->mg);
+	}
+
+	return thread->mg ? 0 : -1;
+}
+
 struct thread *thread__new(pid_t pid, pid_t tid)
 {
 	char *comm_str;
@@ -15,10 +31,6 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	struct thread *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
-		thread->mg = map_groups__new();
-		if (thread->mg == NULL)
-			goto out_free;
-
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
@@ -40,8 +52,6 @@ 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;
 }
@@ -126,9 +136,26 @@ void thread__insert_map(struct thread *thread, struct map *map)
 	map_groups__insert(thread->mg, map);
 }
 
+static int thread__clone_map_groups(struct thread *thread,
+				    struct thread *parent)
+{
+	int i;
+
+	/* This is new thread, we share map groups for process. */
+	if (thread->pid_ == parent->pid_)
+		return 0;
+
+	/* But this one is new process, copy maps. */
+	for (i = 0; i < MAP__NR_TYPES; ++i)
+		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
+			return -ENOMEM;
+
+	return 0;
+}
+
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 {
-	int i, err;
+	int err;
 
 	if (parent->comm_set) {
 		const char *comm = thread__comm_str(parent);
@@ -140,13 +167,8 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 		thread->comm_set = true;
 	}
 
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
-			return -ENOMEM;
-
 	thread->ppid = parent->tid;
-
-	return 0;
+	return thread__clone_map_groups(thread, parent);
 }
 
 void thread__find_cpumode_addr_location(struct thread *thread,
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index bee1eb0..3c0c272 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -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)
 {
-- 
1.8.3.1


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

* [PATCH 5/5] perf tests: Add map groups sharing with thread object test
  2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
                   ` (3 preceding siblings ...)
  2014-04-14 15:52 ` [PATCH 4/5] perf tools: Share map_groups among threads of the same group Jiri Olsa
@ 2014-04-14 15:52 ` Jiri Olsa
  2014-04-24 14:29   ` Namhyung Kim
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
  4 siblings, 2 replies; 17+ messages in thread
From: Jiri Olsa @ 2014-04-14 15:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jiri Olsa, Don Zickus, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
	Peter Zijlstra, Arnaldo Carvalho de Melo

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

Cc: Don Zickus <dzickus@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 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

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7bad323..eba6a61 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index d947d1a..a1145d1 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -128,6 +128,10 @@ static struct test {
 		.func = test__mmap_thread_lookup,
 	},
 	{
+		.desc = "Test thread mg sharing",
+		.func = test__thread_mg_share,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 6db764c..5ed3bd5 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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
diff --git a/tools/perf/tests/thread-mg-share.c b/tools/perf/tests/thread-mg-share.c
new file mode 100644
index 0000000..2697636
--- /dev/null
+++ b/tools/perf/tests/thread-mg-share.c
@@ -0,0 +1,90 @@
+#include "tests.h"
+#include "machine.h"
+#include "thread.h"
+#include "map.h"
+
+int test__thread_mg_share(void)
+{
+	struct machines machines;
+	struct machine *machine;
+
+	/* thread group */
+	struct thread *leader;
+	struct thread *t1, *t2, *t3;
+	struct map_groups *mg;
+
+	/* other process */
+	struct thread *other, *other_leader;
+	struct map_groups *other_mg;
+
+	/*
+	 * This test create 2 processes abstractions (struct thread)
+	 * with several threads and checks they properly share and
+	 * maintain map groups info (struct map_groups).
+	 *
+	 * thread group (pid: 0, tids: 0, 1, 2, 3)
+	 * other  group (pid: 4, tids: 4, 5)
+	*/
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	/* create process with 4 threads */
+	leader = machine__findnew_thread(machine, 0, 0);
+	t1     = machine__findnew_thread(machine, 0, 1);
+	t2     = machine__findnew_thread(machine, 0, 2);
+	t3     = machine__findnew_thread(machine, 0, 3);
+
+	/* and create 1 separated process, without thread leader */
+	other  = machine__findnew_thread(machine, 4, 5);
+
+	TEST_ASSERT_VAL("failed to create threads",
+			leader && t1 && t2 && t3 && other);
+
+	mg = leader->mg;
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 4);
+
+	/* test the map groups pointer is shared */
+	TEST_ASSERT_VAL("map groups don't match", mg == t1->mg);
+	TEST_ASSERT_VAL("map groups don't match", mg == t2->mg);
+	TEST_ASSERT_VAL("map groups don't match", mg == t3->mg);
+
+	/*
+	 * Verify the other leader was created by previous call.
+	 * It should have shared map groups with no change in
+	 * refcnt.
+	 */
+	other_leader = machine__find_thread(machine, 4, 4);
+	TEST_ASSERT_VAL("failed to find other leader", other_leader);
+
+	other_mg = other->mg;
+	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 2);
+
+	TEST_ASSERT_VAL("map groups don't match", other_mg == other_leader->mg);
+
+	/* release thread group */
+	thread__delete(leader);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 3);
+
+	thread__delete(t1);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 2);
+
+	thread__delete(t2);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 1);
+
+	thread__delete(t3);
+
+	/* release other group  */
+	thread__delete(other_leader);
+	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 1);
+
+	thread__delete(other);
+
+	/*
+	 * Cannot call machine__delete_threads(machine) now,
+	 * because we've already realased all the threads.
+	 */
+
+	machines__exit(&machines);
+	return 0;
+}
-- 
1.8.3.1


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

* Re: [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-14 15:52 ` [PATCH 3/5] perf tools: Reference count map_groups objects Jiri Olsa
@ 2014-04-14 17:22   ` David Ahern
  2014-04-15  9:56     ` Jiri Olsa
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 17+ messages in thread
From: David Ahern @ 2014-04-14 17:22 UTC (permalink / raw)
  To: Jiri Olsa, linux-kernel
  Cc: Arnaldo Carvalho de Melo, Adrian Hunter, Don Zickus,
	Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

On 4/14/14, 9:52 AM, Jiri Olsa wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> We will share it among threads in the same process.
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Don Zickus <dzickus@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
>   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(-)
>
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index ae4c5e1..ba5f5c0c 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -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;
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 1073e2d..d6445b2 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -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

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

* Re: [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-14 17:22   ` David Ahern
@ 2014-04-15  9:56     ` Jiri Olsa
  2014-04-15 18:17       ` David Ahern
  0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-04-15  9:56 UTC (permalink / raw)
  To: David Ahern
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Don Zickus, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

On Mon, Apr 14, 2014 at 11:22:02AM -0600, David Ahern wrote:
> On 4/14/14, 9:52 AM, Jiri Olsa wrote:

SNIP

> >
> >+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;
> >diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> >index 1073e2d..d6445b2 100644
> >--- a/tools/perf/util/map.h
> >+++ b/tools/perf/util/map.h
> >@@ -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.

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

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

* Re: [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-15  9:56     ` Jiri Olsa
@ 2014-04-15 18:17       ` David Ahern
  2014-04-23 16:31         ` Jiri Olsa
  0 siblings, 1 reply; 17+ messages in thread
From: David Ahern @ 2014-04-15 18:17 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Don Zickus, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

On 4/15/14, 3:56 AM, Jiri Olsa wrote:
>
> 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


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

* Re: [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-15 18:17       ` David Ahern
@ 2014-04-23 16:31         ` Jiri Olsa
  2014-04-23 16:45           ` David Ahern
  0 siblings, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-04-23 16:31 UTC (permalink / raw)
  To: David Ahern
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Don Zickus, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

On Tue, Apr 15, 2014 at 12:17:22PM -0600, David Ahern wrote:
> On 4/15/14, 3:56 AM, Jiri Olsa wrote:
> >
> >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

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

* Re: [PATCH 3/5] perf tools: Reference count map_groups objects
  2014-04-23 16:31         ` Jiri Olsa
@ 2014-04-23 16:45           ` David Ahern
  0 siblings, 0 replies; 17+ messages in thread
From: David Ahern @ 2014-04-23 16:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Don Zickus, Frederic Weisbecker, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

On 4/23/14, 10:31 AM, Jiri Olsa wrote:
> 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

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

* Re: [PATCH 5/5] perf tests: Add map groups sharing with thread object test
  2014-04-14 15:52 ` [PATCH 5/5] perf tests: Add map groups sharing with thread object test Jiri Olsa
@ 2014-04-24 14:29   ` Namhyung Kim
  2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
  1 sibling, 0 replies; 17+ messages in thread
From: Namhyung Kim @ 2014-04-24 14:29 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Arnaldo Carvalho de Melo

2014-04-14 (월), 17:52 +0200, Jiri Olsa:
> +	/*
> +	 * 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.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> +	 */
> +
> +	machines__exit(&machines);
> +	return 0;
> +}




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

* [tip:perf/core] perf tests: Add thread maps lookup automated tests
  2014-04-14 15:51 ` [PATCH 1/5] perf tests: Add thread maps lookup automated tests Jiri Olsa
@ 2014-04-29  6:46   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-04-29  6:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: eranian, mingo, jolsa, a.p.zijlstra, efault, acme, fweisbec,
	dsahern, tglx, cjashfor, dzickus, hpa, paulus, linux-kernel,
	namhyung, adrian.hunter

Commit-ID:  4e85edfc3f5c0e016a960c1dcbe0217e86602525
Gitweb:     http://git.kernel.org/tip/4e85edfc3f5c0e016a960c1dcbe0217e86602525
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 5 Mar 2014 17:20:31 +0100
Committer:  Jiri Olsa <jolsa@kernel.org>
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.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1397490723-1992-2-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(+)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 0807e4c..16d4f4e 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 5c11eca..ebdad33 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -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__
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index ceb9dae..bb60792 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -128,6 +128,10 @@ static struct test {
 		.func = test__hists_filter,
 	},
 	{
+		.desc = "Test mmap thread lookup",
+		.func = test__mmap_thread_lookup,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
new file mode 100644
index 0000000..4a456fe
--- /dev/null
+++ b/tools/perf/tests/mmap-thread-lookup.c
@@ -0,0 +1,233 @@
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include "debug.h"
+#include "tests.h"
+#include "machine.h"
+#include "thread_map.h"
+#include "symbol.h"
+#include "thread.h"
+
+#define THREADS 4
+
+static int go_away;
+
+struct thread_data {
+	pthread_t	pt;
+	pid_t		tid;
+	void		*map;
+	int		ready[2];
+};
+
+static struct thread_data threads[THREADS];
+
+static int thread_init(struct thread_data *td)
+{
+	void *map;
+
+	map = mmap(NULL, page_size,
+		   PROT_READ|PROT_WRITE|PROT_EXEC,
+		   MAP_SHARED|MAP_ANONYMOUS, -1, 0);
+
+	if (map == MAP_FAILED) {
+		perror("mmap failed");
+		return -1;
+	}
+
+	td->map = map;
+	td->tid = syscall(SYS_gettid);
+
+	pr_debug("tid = %d, map = %p\n", td->tid, map);
+	return 0;
+}
+
+static void *thread_fn(void *arg)
+{
+	struct thread_data *td = arg;
+	ssize_t ret;
+	int go;
+
+	if (thread_init(td))
+		return NULL;
+
+	/* Signal thread_create thread is initialized. */
+	ret = write(td->ready[1], &go, sizeof(int));
+	if (ret != sizeof(int)) {
+		pr_err("failed to notify\n");
+		return NULL;
+	}
+
+	while (!go_away) {
+		/* Waiting for main thread to kill us. */
+		usleep(100);
+	}
+
+	munmap(td->map, page_size);
+	return NULL;
+}
+
+static int thread_create(int i)
+{
+	struct thread_data *td = &threads[i];
+	int err, go;
+
+	if (pipe(td->ready))
+		return -1;
+
+	err = pthread_create(&td->pt, NULL, thread_fn, td);
+	if (!err) {
+		/* Wait for thread initialization. */
+		ssize_t ret = read(td->ready[0], &go, sizeof(int));
+		err = ret != sizeof(int);
+	}
+
+	close(td->ready[0]);
+	close(td->ready[1]);
+	return err;
+}
+
+static int threads_create(void)
+{
+	struct thread_data *td0 = &threads[0];
+	int i, err = 0;
+
+	go_away = 0;
+
+	/* 0 is main thread */
+	if (thread_init(td0))
+		return -1;
+
+	for (i = 1; !err && i < THREADS; i++)
+		err = thread_create(i);
+
+	return err;
+}
+
+static int threads_destroy(void)
+{
+	struct thread_data *td0 = &threads[0];
+	int i, err = 0;
+
+	/* cleanup the main thread */
+	munmap(td0->map, page_size);
+
+	go_away = 1;
+
+	for (i = 1; !err && i < THREADS; i++)
+		err = pthread_join(threads[i].pt, NULL);
+
+	return err;
+}
+
+typedef int (*synth_cb)(struct machine *machine);
+
+static int synth_all(struct machine *machine)
+{
+	return perf_event__synthesize_threads(NULL,
+					      perf_event__process,
+					      machine, 0);
+}
+
+static int synth_process(struct machine *machine)
+{
+	struct thread_map *map;
+	int err;
+
+	map = thread_map__new_by_pid(getpid());
+
+	err = perf_event__synthesize_thread_map(NULL, map,
+						perf_event__process,
+						machine, 0);
+
+	thread_map__delete(map);
+	return err;
+}
+
+static int mmap_events(synth_cb synth)
+{
+	struct machines machines;
+	struct machine *machine;
+	int err, i;
+
+	/*
+	 * The threads_create will not return before all threads
+	 * are spawned and all created memory map.
+	 *
+	 * They will loop until threads_destroy is called, so we
+	 * can safely run synthesizing function.
+	 */
+	TEST_ASSERT_VAL("failed to create threads", !threads_create());
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	dump_trace = verbose > 1 ? 1 : 0;
+
+	err = synth(machine);
+
+	dump_trace = 0;
+
+	TEST_ASSERT_VAL("failed to destroy threads", !threads_destroy());
+	TEST_ASSERT_VAL("failed to synthesize maps", !err);
+
+	/*
+	 * All data is synthesized, try to find map for each
+	 * thread object.
+	 */
+	for (i = 0; i < THREADS; i++) {
+		struct thread_data *td = &threads[i];
+		struct addr_location al;
+		struct thread *thread;
+
+		thread = machine__findnew_thread(machine, getpid(), td->tid);
+
+		pr_debug("looking for map %p\n", td->map);
+
+		thread__find_addr_map(thread, machine,
+				      PERF_RECORD_MISC_USER, MAP__FUNCTION,
+				      (unsigned long) (td->map + 1), &al);
+
+		if (!al.map) {
+			pr_debug("failed, couldn't find map\n");
+			err = -1;
+			break;
+		}
+
+		pr_debug("map %p, addr %" PRIx64 "\n", al.map, al.map->start);
+	}
+
+	machine__delete_threads(machine);
+	machines__exit(&machines);
+	return err;
+}
+
+/*
+ * This test creates 'THREADS' number of threads (including
+ * main thread) and each thread creates memory map.
+ *
+ * When threads are created, we synthesize them with both
+ * (separate tests):
+ *   perf_event__synthesize_thread_map (process based)
+ *   perf_event__synthesize_threads    (global)
+ *
+ * We test we can find all memory maps via:
+ *   thread__find_addr_map
+ *
+ * by using all thread objects.
+ */
+int test__mmap_thread_lookup(void)
+{
+	/* perf_event__synthesize_threads synthesize */
+	TEST_ASSERT_VAL("failed with sythesizing all",
+			!mmap_events(synth_all));
+
+	/* perf_event__synthesize_thread_map synthesize */
+	TEST_ASSERT_VAL("failed with sythesizing process",
+			!mmap_events(synth_process));
+
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index fe39163..82e8061 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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

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

* [tip:perf/core] perf tools: Allocate thread map_groups' s dynamically
  2014-04-14 15:52 ` [PATCH 2/5] perf tools: Allocate thread map_groups's dynamically Jiri Olsa
@ 2014-04-29  6:46   ` tip-bot for Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Arnaldo Carvalho de Melo @ 2014-04-29  6:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: eranian, acme, mingo, jolsa, a.p.zijlstra, efault, acme,
	fweisbec, dsahern, tglx, cjashfor, dzickus, hpa, paulus,
	linux-kernel, namhyung, adrian.hunter

Commit-ID:  93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45
Gitweb:     http://git.kernel.org/tip/93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Fri, 21 Mar 2014 17:57:01 -0300
Committer:  Jiri Olsa <jolsa@kernel.org>
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.

Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1397490723-1992-3-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c
index b8c0102..c916656 100644
--- a/tools/perf/arch/x86/tests/dwarf-unwind.c
+++ b/tools/perf/arch/x86/tests/dwarf-unwind.c
@@ -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);
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index d59893e..9eccf7f 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -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);
 		}
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 9d12aa6..dbcaea1 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -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;
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 39cd2d0..ae4c5e1 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -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;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f00f058..1073e2d 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -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);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 3ce0498..dc51d16 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -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;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 9b29f08..bee1eb0 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -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;

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

* [tip:perf/core] perf tools: Reference count map_groups objects
  2014-04-14 15:52 ` [PATCH 3/5] perf tools: Reference count map_groups objects Jiri Olsa
  2014-04-14 17:22   ` David Ahern
@ 2014-04-29  6:46   ` tip-bot for Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot for Arnaldo Carvalho de Melo @ 2014-04-29  6:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: eranian, acme, mingo, jolsa, a.p.zijlstra, efault, acme,
	fweisbec, dsahern, tglx, cjashfor, dzickus, hpa, paulus,
	linux-kernel, namhyung, adrian.hunter

Commit-ID:  a26ca6716a6c683f40bd676cea7e89704653b98d
Gitweb:     http://git.kernel.org/tip/a26ca6716a6c683f40bd676cea7e89704653b98d
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Tue, 25 Mar 2014 15:26:44 -0300
Committer:  Jiri Olsa <jolsa@kernel.org>
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.

Signed-off-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1397490723-1992-4-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index ae4c5e1..ba5f5c0c 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -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;
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 1073e2d..d6445b2 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -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);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index dc51d16..b501848 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -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);

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

* [tip:perf/core] perf tools: Share map_groups among threads of the same group
  2014-04-14 15:52 ` [PATCH 4/5] perf tools: Share map_groups among threads of the same group Jiri Olsa
@ 2014-04-29  6:46   ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-04-29  6:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: eranian, mingo, jolsa, a.p.zijlstra, efault, acme, fweisbec,
	dsahern, tglx, cjashfor, dzickus, hpa, paulus, linux-kernel,
	namhyung, adrian.hunter

Commit-ID:  cddcef607782966f1601808c17fe9c4c5f79f9f4
Gitweb:     http://git.kernel.org/tip/cddcef607782966f1601808c17fe9c4c5f79f9f4
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Wed, 9 Apr 2014 20:54:29 +0200
Committer:  Jiri Olsa <jolsa@kernel.org>
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.

Original-patch-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1397490723-1992-5-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index a53cd0b..98ec56d 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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;
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index b501848..2fde0d5 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -8,6 +8,22 @@
 #include "debug.h"
 #include "comm.h"
 
+int thread__init_map_groups(struct thread *thread, struct machine *machine)
+{
+	struct thread *leader;
+	pid_t pid = thread->pid_;
+
+	if (pid == thread->tid) {
+		thread->mg = map_groups__new();
+	} else {
+		leader = machine__findnew_thread(machine, pid, pid);
+		if (leader)
+			thread->mg = map_groups__get(leader->mg);
+	}
+
+	return thread->mg ? 0 : -1;
+}
+
 struct thread *thread__new(pid_t pid, pid_t tid)
 {
 	char *comm_str;
@@ -15,10 +31,6 @@ struct thread *thread__new(pid_t pid, pid_t tid)
 	struct thread *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
-		thread->mg = map_groups__new();
-		if (thread->mg == NULL)
-			goto out_free;
-
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
@@ -40,8 +52,6 @@ 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;
 }
@@ -126,9 +136,26 @@ void thread__insert_map(struct thread *thread, struct map *map)
 	map_groups__insert(thread->mg, map);
 }
 
+static int thread__clone_map_groups(struct thread *thread,
+				    struct thread *parent)
+{
+	int i;
+
+	/* This is new thread, we share map groups for process. */
+	if (thread->pid_ == parent->pid_)
+		return 0;
+
+	/* But this one is new process, copy maps. */
+	for (i = 0; i < MAP__NR_TYPES; ++i)
+		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
+			return -ENOMEM;
+
+	return 0;
+}
+
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 {
-	int i, err;
+	int err;
 
 	if (parent->comm_set) {
 		const char *comm = thread__comm_str(parent);
@@ -140,13 +167,8 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 		thread->comm_set = true;
 	}
 
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		if (map_groups__clone(thread->mg, parent->mg, i) < 0)
-			return -ENOMEM;
-
 	thread->ppid = parent->tid;
-
-	return 0;
+	return thread__clone_map_groups(thread, parent);
 }
 
 void thread__find_cpumode_addr_location(struct thread *thread,
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index bee1eb0..3c0c272 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -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)
 {

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

* [tip:perf/core] perf tests: Add map groups sharing with thread object test
  2014-04-14 15:52 ` [PATCH 5/5] perf tests: Add map groups sharing with thread object test Jiri Olsa
  2014-04-24 14:29   ` Namhyung Kim
@ 2014-04-29  6:46   ` tip-bot for Jiri Olsa
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-04-29  6:46 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: eranian, mingo, jolsa, a.p.zijlstra, efault, acme, fweisbec,
	dsahern, tglx, cjashfor, dzickus, hpa, paulus, linux-kernel,
	namhyung, adrian.hunter

Commit-ID:  fabf01238289e9ae009499594fc54642f5802a24
Gitweb:     http://git.kernel.org/tip/fabf01238289e9ae009499594fc54642f5802a24
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Mon, 17 Mar 2014 14:39:00 +0100
Committer:  Jiri Olsa <jolsa@kernel.org>
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.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1397490723-1992-6-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(+)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 16d4f4e..46e0c32 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -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
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index bb60792..0d5afaf 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -132,6 +132,10 @@ static struct test {
 		.func = test__mmap_thread_lookup,
 	},
 	{
+		.desc = "Test thread mg sharing",
+		.func = test__thread_mg_share,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 82e8061..a9d7cb0 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -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
diff --git a/tools/perf/tests/thread-mg-share.c b/tools/perf/tests/thread-mg-share.c
new file mode 100644
index 0000000..2b2e0db
--- /dev/null
+++ b/tools/perf/tests/thread-mg-share.c
@@ -0,0 +1,90 @@
+#include "tests.h"
+#include "machine.h"
+#include "thread.h"
+#include "map.h"
+
+int test__thread_mg_share(void)
+{
+	struct machines machines;
+	struct machine *machine;
+
+	/* thread group */
+	struct thread *leader;
+	struct thread *t1, *t2, *t3;
+	struct map_groups *mg;
+
+	/* other process */
+	struct thread *other, *other_leader;
+	struct map_groups *other_mg;
+
+	/*
+	 * This test create 2 processes abstractions (struct thread)
+	 * with several threads and checks they properly share and
+	 * maintain map groups info (struct map_groups).
+	 *
+	 * thread group (pid: 0, tids: 0, 1, 2, 3)
+	 * other  group (pid: 4, tids: 4, 5)
+	*/
+
+	machines__init(&machines);
+	machine = &machines.host;
+
+	/* create process with 4 threads */
+	leader = machine__findnew_thread(machine, 0, 0);
+	t1     = machine__findnew_thread(machine, 0, 1);
+	t2     = machine__findnew_thread(machine, 0, 2);
+	t3     = machine__findnew_thread(machine, 0, 3);
+
+	/* and create 1 separated process, without thread leader */
+	other  = machine__findnew_thread(machine, 4, 5);
+
+	TEST_ASSERT_VAL("failed to create threads",
+			leader && t1 && t2 && t3 && other);
+
+	mg = leader->mg;
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 4);
+
+	/* test the map groups pointer is shared */
+	TEST_ASSERT_VAL("map groups don't match", mg == t1->mg);
+	TEST_ASSERT_VAL("map groups don't match", mg == t2->mg);
+	TEST_ASSERT_VAL("map groups don't match", mg == t3->mg);
+
+	/*
+	 * Verify the other leader was created by previous call.
+	 * It should have shared map groups with no change in
+	 * refcnt.
+	 */
+	other_leader = machine__find_thread(machine, 4, 4);
+	TEST_ASSERT_VAL("failed to find other leader", other_leader);
+
+	other_mg = other->mg;
+	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 2);
+
+	TEST_ASSERT_VAL("map groups don't match", other_mg == other_leader->mg);
+
+	/* release thread group */
+	thread__delete(leader);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 3);
+
+	thread__delete(t1);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 2);
+
+	thread__delete(t2);
+	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 1);
+
+	thread__delete(t3);
+
+	/* release other group  */
+	thread__delete(other_leader);
+	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 1);
+
+	thread__delete(other);
+
+	/*
+	 * Cannot call machine__delete_threads(machine) now,
+	 * because we've already released all the threads.
+	 */
+
+	machines__exit(&machines);
+	return 0;
+}

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

end of thread, other threads:[~2014-04-29  6:47 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-14 15:51 [PATCHv2 0/5] perf tools: Share map groups within process Jiri Olsa
2014-04-14 15:51 ` [PATCH 1/5] perf tests: Add thread maps lookup automated tests Jiri Olsa
2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
2014-04-14 15:52 ` [PATCH 2/5] perf tools: Allocate thread map_groups's dynamically Jiri Olsa
2014-04-29  6:46   ` [tip:perf/core] perf tools: Allocate thread map_groups' s dynamically tip-bot for Arnaldo Carvalho de Melo
2014-04-14 15:52 ` [PATCH 3/5] perf tools: Reference count map_groups objects Jiri Olsa
2014-04-14 17:22   ` David Ahern
2014-04-15  9:56     ` Jiri Olsa
2014-04-15 18:17       ` David Ahern
2014-04-23 16:31         ` Jiri Olsa
2014-04-23 16:45           ` David Ahern
2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Arnaldo Carvalho de Melo
2014-04-14 15:52 ` [PATCH 4/5] perf tools: Share map_groups among threads of the same group Jiri Olsa
2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa
2014-04-14 15:52 ` [PATCH 5/5] perf tests: Add map groups sharing with thread object test Jiri Olsa
2014-04-24 14:29   ` Namhyung Kim
2014-04-29  6:46   ` [tip:perf/core] " tip-bot for Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.