[RFC 0/5] perf tools: Share map groups within process

[RFC 0/5] perf tools: Share map groups within process

[Jiri Olsa]

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 implementes one of the solution ideas for issue
described by Don in following thread:
http://marc.info/?l=linux-kernel&m=139403876017159&w=2

This patches still has some loose ends, just wanted to hear
opinions for this concept.

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>
---
Jiri Olsa (5):
      perf tests: Add tip/pid mmap automated tests
      perf tools: Factor machine__find_thread to take tid argument
      perf tools: Allocate thread map_groups dynamicaly
      perf tools: Add machine pointer into thread struct
      perf tools: Share process map groups within process threads

 tools/perf/Makefile.perf                 |   1 +
 tools/perf/arch/x86/tests/dwarf-unwind.c |   2 +-
 tools/perf/perf.h                        |   6 ++
 tools/perf/tests/builtin-test.c          |   4 ++
 tools/perf/tests/dwarf-unwind.c          |   2 +-
 tools/perf/tests/mmap-events.c           | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                 |   1 +
 tools/perf/ui/stdio/hist.c               |   8 ++-
 tools/perf/util/event.c                  |   4 +-
 tools/perf/util/machine.c                |  21 ++++---
 tools/perf/util/machine.h                |   3 +-
 tools/perf/util/map.h                    |   3 +-
 tools/perf/util/thread.c                 |  87 ++++++++++++++++++++++++++---
 tools/perf/util/thread.h                 |  15 +++--
 14 files changed, 317 insertions(+), 28 deletions(-)
 create mode 100644 tools/perf/tests/mmap-events.c

[PATCH 1/5] perf tests: Add tip/pid mmap automated tests

[Jiri Olsa]

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.

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>
---
 tools/perf/Makefile.perf        |   1 +
 tools/perf/perf.h               |   6 ++
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/mmap-events.c  | 188 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 5 files changed, 200 insertions(+)
 create mode 100644 tools/perf/tests/mmap-events.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 50d875d..9324a40 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-events.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..d7a9b3a 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 events",
+		.func = test__mmap_events,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/mmap-events.c b/tools/perf/tests/mmap-events.c
new file mode 100644
index 0000000..5c7fd7e
--- /dev/null
+++ b/tools/perf/tests/mmap-events.c
@@ -0,0 +1,188 @@
+#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,
+		   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(void *arg)
+{
+	struct thread_data *td = arg;
+	int go;
+
+	if (thread_init(td))
+		return NULL;
+
+	write(td->ready[1], &go, sizeof(int));
+
+	while (!go_away) {
+		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, td);
+	if (!err) {
+		ssize_t ret = read(td->ready[0], &go, sizeof(int));
+		err = ret != sizeof(int);
+	}
+
+	return err;
+}
+
+static int threads_create(void)
+{
+	struct thread_data *td0 = &threads[0];
+	int i, err = 0;
+
+	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;
+
+	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_tid(syscall(SYS_gettid));
+
+	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;
+
+	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);
+
+	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,
+				      (u64) (td->map + 1), &al);
+
+		if (!al.map) {
+			pr_debug("failed, couldn't find map\n");
+			err = -1;
+			break;
+		}
+
+		pr_debug("map %p, addr %lx\n", al.map, al.map->start);
+	}
+
+	machine__delete_threads(machine);
+	machines__exit(&machines);
+	return err;
+}
+
+int test__mmap_events(void)
+{
+	TEST_ASSERT_VAL("failed with sythesizing all",
+			!mmap_events(synth_all));
+	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..3b2ba1e 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_events(void);
 
 #if defined(__x86_64__) || defined(__i386__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
-- 
1.8.3.1

[PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument

[Jiri Olsa]

Forcing the code to always search thread by pid/tid pair.

The PID value will be needed in future to determine
the process thread leader for map groups sharing.

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>
---
 tools/perf/tests/dwarf-unwind.c |  2 +-
 tools/perf/util/machine.c       | 13 +++++++++----
 tools/perf/util/machine.h       |  3 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index f16ea28..c059ee8 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -128,7 +128,7 @@ int test__dwarf_unwind(void)
 	if (verbose > 1)
 		machine__fprintf(machine, stderr);
 
-	thread = machine__find_thread(machine, getpid());
+	thread = machine__find_thread(machine, getpid(), getpid());
 	if (!thread) {
 		pr_err("Could not get thread\n");
 		goto out;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index eb26544..a189faf 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -327,9 +327,10 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
 	return __machine__findnew_thread(machine, pid, tid, true);
 }
 
-struct thread *machine__find_thread(struct machine *machine, pid_t tid)
+struct thread *machine__find_thread(struct machine *machine, pid_t pid,
+				    pid_t tid)
 {
-	return __machine__findnew_thread(machine, 0, tid, false);
+	return __machine__findnew_thread(machine, pid, tid, false);
 }
 
 int machine__process_comm_event(struct machine *machine, union perf_event *event,
@@ -1114,7 +1115,9 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
 int machine__process_fork_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample)
 {
-	struct thread *thread = machine__find_thread(machine, event->fork.tid);
+	struct thread *thread = machine__find_thread(machine,
+						     event->fork.pid,
+						     event->fork.tid);
 	struct thread *parent = machine__findnew_thread(machine,
 							event->fork.ppid,
 							event->fork.ptid);
@@ -1140,7 +1143,9 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 int machine__process_exit_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample __maybe_unused)
 {
-	struct thread *thread = machine__find_thread(machine, event->fork.tid);
+	struct thread *thread = machine__find_thread(machine,
+						     event->fork.pid,
+						     event->fork.tid);
 
 	if (dump_trace)
 		perf_event__fprintf_task(event, stdout);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 2e6c248..c8c74a1 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -41,7 +41,8 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
 	return machine->vmlinux_maps[type];
 }
 
-struct thread *machine__find_thread(struct machine *machine, pid_t tid);
+struct thread *machine__find_thread(struct machine *machine, pid_t pid,
+				    pid_t tid);
 
 int machine__process_comm_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample);
-- 
1.8.3.1

[PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly

[Jiri Olsa]

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.

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>
---
 tools/perf/arch/x86/tests/dwarf-unwind.c |  2 +-
 tools/perf/ui/stdio/hist.c               |  8 ++++-
 tools/perf/util/event.c                  |  4 +--
 tools/perf/util/machine.c                |  6 ++--
 tools/perf/util/map.h                    |  1 -
 tools/perf/util/thread.c                 | 52 +++++++++++++++++++++++++++-----
 tools/perf/util/thread.h                 | 10 ++++--
 7 files changed, 64 insertions(+), 19 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 9bad892..880238e 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -496,7 +496,13 @@ print_entries:
 			break;
 
 		if (h->ms.map == NULL && verbose > 1) {
-			__map_groups__fprintf_maps(&h->thread->mg,
+			struct map_groups *mg = thread__map_groups_get(h->thread);
+
+			if (!mg) {
+				ret = -ENOMEM;
+				break;
+			}
+			__map_groups__fprintf_maps(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 55eebe9..197b594 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -655,7 +655,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__map_groups_get(thread);
 	bool load_map = false;
 
 	al->machine = machine;
@@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
 	al->cpumode = cpumode;
 	al->filtered = false;
 
-	if (machine == NULL) {
+	if ((machine == NULL) || (mg == NULL)) {
 		al->map = NULL;
 		return;
 	}
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index a189faf..009dfb4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1046,8 +1046,7 @@ int machine__process_mmap2_event(struct machine *machine,
 	if (map == NULL)
 		goto out_problem;
 
-	thread__insert_map(thread, map);
-	return 0;
+	return thread__insert_map(thread, map);
 
 out_problem:
 	dump_printf("problem processing PERF_RECORD_MMAP2, skipping event.\n");
@@ -1093,8 +1092,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
 	if (map == NULL)
 		goto out_problem;
 
-	thread__insert_map(thread, map);
-	return 0;
+	return thread__insert_map(thread, map);
 
 out_problem:
 	dump_printf("problem processing PERF_RECORD_MMAP, skipping event.\n");
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index f00f058..99a6488 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -202,5 +202,4 @@ struct map *map_groups__find_by_name(struct map_groups *mg,
 				     enum map_type type, const char *name);
 
 void map_groups__flush(struct map_groups *mg);
-
 #endif /* __PERF_MAP_H */
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 0358882..ac77b6c 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -15,7 +15,6 @@ 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->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
@@ -45,12 +44,15 @@ void thread__delete(struct thread *thread)
 {
 	struct comm *comm, *tmp;
 
-	map_groups__exit(&thread->mg);
+	if (thread->mg)
+		map_groups__exit(thread->mg);
+
 	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
 		list_del(&comm->list);
 		comm__free(comm);
 	}
 
+	thread__map_groups_put(thread);
 	free(thread);
 }
 
@@ -111,13 +113,22 @@ 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)
+int thread__insert_map(struct thread *thread, struct map *map)
 {
-	map_groups__fixup_overlappings(&thread->mg, map, verbose, stderr);
-	map_groups__insert(&thread->mg, map);
+	struct map_groups *mg = thread->mg;
+
+	if (!mg) {
+		mg = thread__map_groups_get(thread);
+		if (!mg)
+			return -ENOMEM;
+	}
+
+	map_groups__fixup_overlappings(mg, map, verbose, stderr);
+	map_groups__insert(mg, map);
+	return 0;
 }
 
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
@@ -135,10 +146,37 @@ 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;
 
 	return 0;
 }
+
+static struct map_groups* thread__map_groups_alloc(struct thread *thread)
+{
+	struct map_groups* mg = zalloc(sizeof(*mg));
+
+	if (mg) {
+		map_groups__init(mg);
+		thread->mg = mg;
+	}
+
+	return mg;
+}
+
+struct map_groups* thread__map_groups_get(struct thread *thread)
+{
+	struct map_groups* mg = thread->mg;
+
+	if (!mg)
+		mg = thread__map_groups_alloc(thread);
+
+	return mg;
+}
+
+void thread__map_groups_put(struct thread *thread)
+{
+	zfree(&thread->mg);
+}
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 5b856bf..77d0be2 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;
@@ -40,14 +40,14 @@ int thread__set_comm(struct thread *thread, const char *comm, u64 timestamp);
 int thread__comm_len(struct thread *thread);
 struct comm *thread__comm(const struct thread *thread);
 const char *thread__comm_str(const struct thread *thread);
-void thread__insert_map(struct thread *thread, struct map *map);
+int thread__insert_map(struct thread *thread, struct map *map);
 int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
 size_t thread__fprintf(struct thread *thread, FILE *fp);
 
 static inline struct map *thread__find_map(struct thread *thread,
 					   enum map_type type, u64 addr)
 {
-	return thread ? map_groups__find(&thread->mg, type, addr) : NULL;
+	return thread ? map_groups__find(thread->mg, type, addr) : NULL;
 }
 
 void thread__find_addr_map(struct thread *thread, struct machine *machine,
@@ -78,4 +78,8 @@ static inline bool thread__is_filtered(struct thread *thread)
 	return false;
 }
 
+struct map_groups* thread__map_groups_get(struct thread *thread);
+
+void thread__map_groups_put(struct thread *thread);
+
 #endif	/* __PERF_THREAD_H */
-- 
1.8.3.1

[PATCH 4/5] perf tools: Add machine pointer into thread struct

[Jiri Olsa]

Need machine pointer in thread object, so we could
lookup the process thread in following patch.

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>
---
 tools/perf/util/machine.c | 2 +-
 tools/perf/util/thread.c  | 4 +++-
 tools/perf/util/thread.h  | 5 ++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 009dfb4..6196bb9 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -311,7 +311,7 @@ static struct thread *__machine__findnew_thread(struct machine *machine,
 	if (!create)
 		return NULL;
 
-	th = thread__new(pid, tid);
+	th = thread__new(pid, tid, machine);
 	if (th != NULL) {
 		rb_link_node(&th->rb_node, parent, p);
 		rb_insert_color(&th->rb_node, &machine->threads);
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index ac77b6c..7c1aad0 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -8,13 +8,15 @@
 #include "debug.h"
 #include "comm.h"
 
-struct thread *thread__new(pid_t pid, pid_t tid)
+struct thread *thread__new(pid_t pid, pid_t tid,
+			   struct machine *machine)
 {
 	char *comm_str;
 	struct comm *comm;
 	struct thread *thread = zalloc(sizeof(*thread));
 
 	if (thread != NULL) {
+		thread->machine = machine;
 		thread->pid_ = pid;
 		thread->tid = tid;
 		thread->ppid = -1;
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 77d0be2..2200557 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -7,12 +7,14 @@
 #include <sys/types.h>
 #include "symbol.h"
 #include <strlist.h>
+#include "machine.h"
 
 struct thread {
 	union {
 		struct rb_node	 rb_node;
 		struct list_head node;
 	};
+	struct machine		*machine;
 	struct map_groups	*mg;
 	pid_t			pid_; /* Not all tools update this */
 	pid_t			tid;
@@ -29,7 +31,8 @@ struct thread {
 struct machine;
 struct comm;
 
-struct thread *thread__new(pid_t pid, pid_t tid);
+struct thread *thread__new(pid_t pid, pid_t tid,
+			   struct machine *machine);
 void thread__delete(struct thread *thread);
 static inline void thread__exited(struct thread *thread)
 {
-- 
1.8.3.1

[PATCH 5/5] perf tools: Share process map groups within process threads

[Jiri Olsa]

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

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>
---
 tools/perf/util/map.h    |  2 ++
 tools/perf/util/thread.c | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 99a6488..d4c8df2 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -59,6 +59,8 @@ struct map_groups {
 	struct rb_root	 maps[MAP__NR_TYPES];
 	struct list_head removed_maps[MAP__NR_TYPES];
 	struct machine	 *machine;
+	/* Used for thread sharing */
+	int		  refcnt;
 };
 
 static inline struct kmap *map__kmap(struct map *map)
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 7c1aad0..2599754 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -156,6 +156,16 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 	return 0;
 }
 
+static struct thread* thread__get_leader(struct thread *thread)
+{
+	pid_t pid = thread->pid_;
+
+	if (pid == thread->tid)
+		return thread;
+
+	return machine__findnew_thread(thread->machine, pid, pid);
+}
+
 static struct map_groups* thread__map_groups_alloc(struct thread *thread)
 {
 	struct map_groups* mg = zalloc(sizeof(*mg));
@@ -172,13 +182,34 @@ struct map_groups* thread__map_groups_get(struct thread *thread)
 {
 	struct map_groups* mg = thread->mg;
 
-	if (!mg)
-		mg = thread__map_groups_alloc(thread);
+	if (!mg) {
+		struct thread *leader = thread__get_leader(thread);
+
+		if (!leader)
+			return NULL;
+
+		if (leader->mg)
+			mg = leader->mg;
+		else
+			mg = thread__map_groups_alloc(leader);
+
+		if (leader != thread)
+			thread->mg = mg;
+
+		mg->refcnt++;
+	}
 
 	return mg;
 }
 
 void thread__map_groups_put(struct thread *thread)
 {
-	zfree(&thread->mg);
+	struct map_groups* mg = thread->mg;
+
+	if (mg) {
+		BUG_ON(!mg->refcnt);
+
+		if (!--mg->refcnt)
+			zfree(&thread->mg);
+	}
 }
-- 
1.8.3.1

Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly

[Arnaldo Carvalho de Melo]

Em Fri, Mar 14, 2014 at 03:00:04PM +0100, Jiri Olsa escreveu:
> 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.

When I saw the get() idiom I thought: cool, he is doing refcounting, but
as I see it you're using it just to make sure that it gets allocated.

I'm continuing to read the patches, but I thought it would be better to
make sure it gets allocated at some suitable moment, that at first sight
doesn't seem to be "the first time it is accessed anywhere", for
instance, I wouldn't expect at all that it would be allocated just
before calling __map_groups__fprintf_maps() :-)

- Arnaldo
 
> 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>
> ---
>  tools/perf/arch/x86/tests/dwarf-unwind.c |  2 +-
>  tools/perf/ui/stdio/hist.c               |  8 ++++-
>  tools/perf/util/event.c                  |  4 +--
>  tools/perf/util/machine.c                |  6 ++--
>  tools/perf/util/map.h                    |  1 -
>  tools/perf/util/thread.c                 | 52 +++++++++++++++++++++++++++-----
>  tools/perf/util/thread.h                 | 10 ++++--
>  7 files changed, 64 insertions(+), 19 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 9bad892..880238e 100644
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -496,7 +496,13 @@ print_entries:
>  			break;
>  
>  		if (h->ms.map == NULL && verbose > 1) {
> -			__map_groups__fprintf_maps(&h->thread->mg,
> +			struct map_groups *mg = thread__map_groups_get(h->thread);
> +
> +			if (!mg) {
> +				ret = -ENOMEM;
> +				break;
> +			}
> +			__map_groups__fprintf_maps(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 55eebe9..197b594 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -655,7 +655,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__map_groups_get(thread);
>  	bool load_map = false;
>  
>  	al->machine = machine;
> @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
>  	al->cpumode = cpumode;
>  	al->filtered = false;
>  
> -	if (machine == NULL) {
> +	if ((machine == NULL) || (mg == NULL)) {
>  		al->map = NULL;
>  		return;
>  	}
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index a189faf..009dfb4 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1046,8 +1046,7 @@ int machine__process_mmap2_event(struct machine *machine,
>  	if (map == NULL)
>  		goto out_problem;
>  
> -	thread__insert_map(thread, map);
> -	return 0;
> +	return thread__insert_map(thread, map);
>  
>  out_problem:
>  	dump_printf("problem processing PERF_RECORD_MMAP2, skipping event.\n");
> @@ -1093,8 +1092,7 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
>  	if (map == NULL)
>  		goto out_problem;
>  
> -	thread__insert_map(thread, map);
> -	return 0;
> +	return thread__insert_map(thread, map);
>  
>  out_problem:
>  	dump_printf("problem processing PERF_RECORD_MMAP, skipping event.\n");
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index f00f058..99a6488 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -202,5 +202,4 @@ struct map *map_groups__find_by_name(struct map_groups *mg,
>  				     enum map_type type, const char *name);
>  
>  void map_groups__flush(struct map_groups *mg);
> -
>  #endif /* __PERF_MAP_H */
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 0358882..ac77b6c 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -15,7 +15,6 @@ 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->pid_ = pid;
>  		thread->tid = tid;
>  		thread->ppid = -1;
> @@ -45,12 +44,15 @@ void thread__delete(struct thread *thread)
>  {
>  	struct comm *comm, *tmp;
>  
> -	map_groups__exit(&thread->mg);
> +	if (thread->mg)
> +		map_groups__exit(thread->mg);
> +
>  	list_for_each_entry_safe(comm, tmp, &thread->comm_list, list) {
>  		list_del(&comm->list);
>  		comm__free(comm);
>  	}
>  
> +	thread__map_groups_put(thread);
>  	free(thread);
>  }
>  
> @@ -111,13 +113,22 @@ 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)
> +int thread__insert_map(struct thread *thread, struct map *map)
>  {
> -	map_groups__fixup_overlappings(&thread->mg, map, verbose, stderr);
> -	map_groups__insert(&thread->mg, map);
> +	struct map_groups *mg = thread->mg;
> +
> +	if (!mg) {
> +		mg = thread__map_groups_get(thread);
> +		if (!mg)
> +			return -ENOMEM;
> +	}
> +
> +	map_groups__fixup_overlappings(mg, map, verbose, stderr);
> +	map_groups__insert(mg, map);
> +	return 0;
>  }
>  
>  int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
> @@ -135,10 +146,37 @@ 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;
>  
>  	return 0;
>  }
> +
> +static struct map_groups* thread__map_groups_alloc(struct thread *thread)
> +{
> +	struct map_groups* mg = zalloc(sizeof(*mg));
> +
> +	if (mg) {
> +		map_groups__init(mg);
> +		thread->mg = mg;
> +	}
> +
> +	return mg;
> +}
> +
> +struct map_groups* thread__map_groups_get(struct thread *thread)
> +{
> +	struct map_groups* mg = thread->mg;
> +
> +	if (!mg)
> +		mg = thread__map_groups_alloc(thread);
> +
> +	return mg;
> +}
> +
> +void thread__map_groups_put(struct thread *thread)
> +{
> +	zfree(&thread->mg);
> +}
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 5b856bf..77d0be2 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;
> @@ -40,14 +40,14 @@ int thread__set_comm(struct thread *thread, const char *comm, u64 timestamp);
>  int thread__comm_len(struct thread *thread);
>  struct comm *thread__comm(const struct thread *thread);
>  const char *thread__comm_str(const struct thread *thread);
> -void thread__insert_map(struct thread *thread, struct map *map);
> +int thread__insert_map(struct thread *thread, struct map *map);
>  int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp);
>  size_t thread__fprintf(struct thread *thread, FILE *fp);
>  
>  static inline struct map *thread__find_map(struct thread *thread,
>  					   enum map_type type, u64 addr)
>  {
> -	return thread ? map_groups__find(&thread->mg, type, addr) : NULL;
> +	return thread ? map_groups__find(thread->mg, type, addr) : NULL;
>  }
>  
>  void thread__find_addr_map(struct thread *thread, struct machine *machine,
> @@ -78,4 +78,8 @@ static inline bool thread__is_filtered(struct thread *thread)
>  	return false;
>  }
>  
> +struct map_groups* thread__map_groups_get(struct thread *thread);
> +
> +void thread__map_groups_put(struct thread *thread);
> +
>  #endif	/* __PERF_THREAD_H */
> -- 
> 1.8.3.1

Re: [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument

[Arnaldo Carvalho de Melo]

Em Fri, Mar 14, 2014 at 03:00:03PM +0100, Jiri Olsa escreveu:
> Forcing the code to always search thread by pid/tid pair.
> 
> The PID value will be needed in future to determine
> the process thread leader for map groups sharing.

This one looks OK, Adrian?

- Arnaldo
 
> 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>
> ---
>  tools/perf/tests/dwarf-unwind.c |  2 +-
>  tools/perf/util/machine.c       | 13 +++++++++----
>  tools/perf/util/machine.h       |  3 ++-
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index f16ea28..c059ee8 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -128,7 +128,7 @@ int test__dwarf_unwind(void)
>  	if (verbose > 1)
>  		machine__fprintf(machine, stderr);
>  
> -	thread = machine__find_thread(machine, getpid());
> +	thread = machine__find_thread(machine, getpid(), getpid());
>  	if (!thread) {
>  		pr_err("Could not get thread\n");
>  		goto out;
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index eb26544..a189faf 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -327,9 +327,10 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
>  	return __machine__findnew_thread(machine, pid, tid, true);
>  }
>  
> -struct thread *machine__find_thread(struct machine *machine, pid_t tid)
> +struct thread *machine__find_thread(struct machine *machine, pid_t pid,
> +				    pid_t tid)
>  {
> -	return __machine__findnew_thread(machine, 0, tid, false);
> +	return __machine__findnew_thread(machine, pid, tid, false);
>  }
>  
>  int machine__process_comm_event(struct machine *machine, union perf_event *event,
> @@ -1114,7 +1115,9 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
>  int machine__process_fork_event(struct machine *machine, union perf_event *event,
>  				struct perf_sample *sample)
>  {
> -	struct thread *thread = machine__find_thread(machine, event->fork.tid);
> +	struct thread *thread = machine__find_thread(machine,
> +						     event->fork.pid,
> +						     event->fork.tid);
>  	struct thread *parent = machine__findnew_thread(machine,
>  							event->fork.ppid,
>  							event->fork.ptid);
> @@ -1140,7 +1143,9 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
>  int machine__process_exit_event(struct machine *machine, union perf_event *event,
>  				struct perf_sample *sample __maybe_unused)
>  {
> -	struct thread *thread = machine__find_thread(machine, event->fork.tid);
> +	struct thread *thread = machine__find_thread(machine,
> +						     event->fork.pid,
> +						     event->fork.tid);
>  
>  	if (dump_trace)
>  		perf_event__fprintf_task(event, stdout);
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 2e6c248..c8c74a1 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -41,7 +41,8 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
>  	return machine->vmlinux_maps[type];
>  }
>  
> -struct thread *machine__find_thread(struct machine *machine, pid_t tid);
> +struct thread *machine__find_thread(struct machine *machine, pid_t pid,
> +				    pid_t tid);
>  
>  int machine__process_comm_event(struct machine *machine, union perf_event *event,
>  				struct perf_sample *sample);
> -- 
> 1.8.3.1

Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct

[Arnaldo Carvalho de Melo]

Em Fri, Mar 14, 2014 at 03:00:05PM +0100, Jiri Olsa escreveu:
> Need machine pointer in thread object, so we could
> lookup the process thread in following patch.

Can't we use the already existing thread->mg.machine for that?

- Arnaldo
 
> 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>
> ---
>  tools/perf/util/machine.c | 2 +-
>  tools/perf/util/thread.c  | 4 +++-
>  tools/perf/util/thread.h  | 5 ++++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 009dfb4..6196bb9 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -311,7 +311,7 @@ static struct thread *__machine__findnew_thread(struct machine *machine,
>  	if (!create)
>  		return NULL;
>  
> -	th = thread__new(pid, tid);
> +	th = thread__new(pid, tid, machine);
>  	if (th != NULL) {
>  		rb_link_node(&th->rb_node, parent, p);
>  		rb_insert_color(&th->rb_node, &machine->threads);
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index ac77b6c..7c1aad0 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -8,13 +8,15 @@
>  #include "debug.h"
>  #include "comm.h"
>  
> -struct thread *thread__new(pid_t pid, pid_t tid)
> +struct thread *thread__new(pid_t pid, pid_t tid,
> +			   struct machine *machine)
>  {
>  	char *comm_str;
>  	struct comm *comm;
>  	struct thread *thread = zalloc(sizeof(*thread));
>  
>  	if (thread != NULL) {
> +		thread->machine = machine;
>  		thread->pid_ = pid;
>  		thread->tid = tid;
>  		thread->ppid = -1;
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 77d0be2..2200557 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -7,12 +7,14 @@
>  #include <sys/types.h>
>  #include "symbol.h"
>  #include <strlist.h>
> +#include "machine.h"
>  
>  struct thread {
>  	union {
>  		struct rb_node	 rb_node;
>  		struct list_head node;
>  	};
> +	struct machine		*machine;
>  	struct map_groups	*mg;
>  	pid_t			pid_; /* Not all tools update this */
>  	pid_t			tid;
> @@ -29,7 +31,8 @@ struct thread {
>  struct machine;
>  struct comm;
>  
> -struct thread *thread__new(pid_t pid, pid_t tid);
> +struct thread *thread__new(pid_t pid, pid_t tid,
> +			   struct machine *machine);
>  void thread__delete(struct thread *thread);
>  static inline void thread__exited(struct thread *thread)
>  {
> -- 
> 1.8.3.1

Re: [PATCH 5/5] perf tools: Share process map groups within process threads

[Arnaldo Carvalho de Melo]

Em Fri, Mar 14, 2014 at 03:00:06PM +0100, Jiri Olsa escreveu:
> Sharing map groups within all process threads. This way
> there's only one copy of mmap info and it's reachale
> from any thread within the process.
> 
> 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>
> ---
>  tools/perf/util/map.h    |  2 ++
>  tools/perf/util/thread.c | 37 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
> index 99a6488..d4c8df2 100644
> --- a/tools/perf/util/map.h
> +++ b/tools/perf/util/map.h
> @@ -59,6 +59,8 @@ struct map_groups {
>  	struct rb_root	 maps[MAP__NR_TYPES];
>  	struct list_head removed_maps[MAP__NR_TYPES];
>  	struct machine	 *machine;
> +	/* Used for thread sharing */
> +	int		  refcnt;

So in the end you use those get/put ops for ref counting, cool. We need
some more of those for other stuff like those pointers in hist_entries
to dead_threads, etc.

- Arnaldo

>  };
>  
>  static inline struct kmap *map__kmap(struct map *map)
> diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
> index 7c1aad0..2599754 100644
> --- a/tools/perf/util/thread.c
> +++ b/tools/perf/util/thread.c
> @@ -156,6 +156,16 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
>  	return 0;
>  }
>  
> +static struct thread* thread__get_leader(struct thread *thread)
> +{
> +	pid_t pid = thread->pid_;
> +
> +	if (pid == thread->tid)
> +		return thread;
> +
> +	return machine__findnew_thread(thread->machine, pid, pid);
> +}
> +
>  static struct map_groups* thread__map_groups_alloc(struct thread *thread)
>  {
>  	struct map_groups* mg = zalloc(sizeof(*mg));
> @@ -172,13 +182,34 @@ struct map_groups* thread__map_groups_get(struct thread *thread)
>  {
>  	struct map_groups* mg = thread->mg;
>  
> -	if (!mg)
> -		mg = thread__map_groups_alloc(thread);
> +	if (!mg) {
> +		struct thread *leader = thread__get_leader(thread);
> +
> +		if (!leader)
> +			return NULL;
> +
> +		if (leader->mg)
> +			mg = leader->mg;
> +		else
> +			mg = thread__map_groups_alloc(leader);
> +
> +		if (leader != thread)
> +			thread->mg = mg;
> +
> +		mg->refcnt++;
> +	}
>  
>  	return mg;
>  }
>  
>  void thread__map_groups_put(struct thread *thread)
>  {
> -	zfree(&thread->mg);
> +	struct map_groups* mg = thread->mg;
> +
> +	if (mg) {
> +		BUG_ON(!mg->refcnt);
> +
> +		if (!--mg->refcnt)
> +			zfree(&thread->mg);
> +	}
>  }
> -- 
> 1.8.3.1

Re: [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument

[Adrian Hunter]

On 14/03/2014 4:15 p.m., Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 14, 2014 at 03:00:03PM +0100, Jiri Olsa escreveu:
>> Forcing the code to always search thread by pid/tid pair.
>>
>> The PID value will be needed in future to determine
>> the process thread leader for map groups sharing.
>
> This one looks OK, Adrian?

Yes it is OK

>
> - Arnaldo
>
>> 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>
>> ---
>>   tools/perf/tests/dwarf-unwind.c |  2 +-
>>   tools/perf/util/machine.c       | 13 +++++++++----
>>   tools/perf/util/machine.h       |  3 ++-
>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
>> index f16ea28..c059ee8 100644
>> --- a/tools/perf/tests/dwarf-unwind.c
>> +++ b/tools/perf/tests/dwarf-unwind.c
>> @@ -128,7 +128,7 @@ int test__dwarf_unwind(void)
>>   	if (verbose > 1)
>>   		machine__fprintf(machine, stderr);
>>
>> -	thread = machine__find_thread(machine, getpid());
>> +	thread = machine__find_thread(machine, getpid(), getpid());
>>   	if (!thread) {
>>   		pr_err("Could not get thread\n");
>>   		goto out;
>> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
>> index eb26544..a189faf 100644
>> --- a/tools/perf/util/machine.c
>> +++ b/tools/perf/util/machine.c
>> @@ -327,9 +327,10 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
>>   	return __machine__findnew_thread(machine, pid, tid, true);
>>   }
>>
>> -struct thread *machine__find_thread(struct machine *machine, pid_t tid)
>> +struct thread *machine__find_thread(struct machine *machine, pid_t pid,
>> +				    pid_t tid)
>>   {
>> -	return __machine__findnew_thread(machine, 0, tid, false);
>> +	return __machine__findnew_thread(machine, pid, tid, false);
>>   }
>>
>>   int machine__process_comm_event(struct machine *machine, union perf_event *event,
>> @@ -1114,7 +1115,9 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
>>   int machine__process_fork_event(struct machine *machine, union perf_event *event,
>>   				struct perf_sample *sample)
>>   {
>> -	struct thread *thread = machine__find_thread(machine, event->fork.tid);
>> +	struct thread *thread = machine__find_thread(machine,
>> +						     event->fork.pid,
>> +						     event->fork.tid);
>>   	struct thread *parent = machine__findnew_thread(machine,
>>   							event->fork.ppid,
>>   							event->fork.ptid);
>> @@ -1140,7 +1143,9 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
>>   int machine__process_exit_event(struct machine *machine, union perf_event *event,
>>   				struct perf_sample *sample __maybe_unused)
>>   {
>> -	struct thread *thread = machine__find_thread(machine, event->fork.tid);
>> +	struct thread *thread = machine__find_thread(machine,
>> +						     event->fork.pid,
>> +						     event->fork.tid);
>>
>>   	if (dump_trace)
>>   		perf_event__fprintf_task(event, stdout);
>> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
>> index 2e6c248..c8c74a1 100644
>> --- a/tools/perf/util/machine.h
>> +++ b/tools/perf/util/machine.h
>> @@ -41,7 +41,8 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
>>   	return machine->vmlinux_maps[type];
>>   }
>>
>> -struct thread *machine__find_thread(struct machine *machine, pid_t tid);
>> +struct thread *machine__find_thread(struct machine *machine, pid_t pid,
>> +				    pid_t tid);
>>
>>   int machine__process_comm_event(struct machine *machine, union perf_event *event,
>>   				struct perf_sample *sample);
>> --
>> 1.8.3.1

Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests

[Arnaldo Carvalho de Melo]

Em Fri, Mar 14, 2014 at 03:00:02PM +0100, Jiri Olsa escreveu:
> Adding automated test for memory maps lookup within
> multiple machines threads.

  CC       /tmp/build/perf/arch/x86/util/tsc.o
tests/mmap-events.c: In function ‘mmap_events’:
tests/mmap-events.c:157:18: error: declaration of ‘thread’ shadows a global declaration [-Werror=shadow]
tests/mmap-events.c:46:14: error: shadowed declaration is here [-Werror=shadow]
tests/mmap-events.c: In function ‘thread’:
tests/mmap-events.c:54:7: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
cc1: all warnings being treated as errors
make[1]: *** [/tmp/build/perf/tests/mmap-events.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [install-bin] Error 2
make: Leaving directory `/home/acme/git/linux/tools/perf'

real	0m10.024s
user	0m45.900s
sys	0m4.932s
[acme@ssdandy linux]$ 

[acme@ssdandy linux]$ cat /etc/fedora-release 
Fedora release 18 (Spherical Cow)
[acme@ssdandy linux]$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/4.7.2/lto-wrapper
Target: x86_64-redhat-linux
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man
--infodir=/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap
--enable-shared --enable-threads=posix --enable-checking=release
--disable-build-with-cxx --disable-build-poststage1-with-cxx
--with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions
--enable-gnu-unique-object --enable-linker-build-id
--with-linker-hash-style=gnu
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
--enable-plugin --enable-initfini-array --enable-java-awt=gtk
--disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre
--enable-libgcj-multifile --enable-java-maintainer-mode
--with-ecj-jar=/usr/share/java/eclipse-ecj.jar
--disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic
--with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC) 
[acme@ssdandy linux]$

Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests

[Namhyung Kim]

Hi Jiri,

On Fri, 14 Mar 2014 15:00:02 +0100, Jiri Olsa wrote:
> +static int thread_init(struct thread_data *td)
> +{
> +	void *map;
> +
> +	map = mmap(NULL, page_size, PROT_READ|PROT_WRITE,
> +		   MAP_SHARED|MAP_ANONYMOUS, -1, 0);

Shouldn't it be an executable mapping to be found by MAP__FUNCTION?

Thanks,
Namhyung

Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests

[Jiri Olsa]

On Fri, Mar 14, 2014 at 05:24:21PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 14, 2014 at 03:00:02PM +0100, Jiri Olsa escreveu:
> > Adding automated test for memory maps lookup within
> > multiple machines threads.
> 
>   CC       /tmp/build/perf/arch/x86/util/tsc.o
> tests/mmap-events.c: In function ‘mmap_events’:
> tests/mmap-events.c:157:18: error: declaration of ‘thread’ shadows a global declaration [-Werror=shadow]
> tests/mmap-events.c:46:14: error: shadowed declaration is here [-Werror=shadow]

hum, I thought this one was fixed already.. will check

> tests/mmap-events.c: In function ‘thread’:
> tests/mmap-events.c:54:7: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]

got this one fixed in new version

thanks,
jirka

Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly

[Namhyung Kim]

On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
> 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.
> @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
>  	al->cpumode = cpumode;
>  	al->filtered = false;
>  
> -	if (machine == NULL) {
> +	if ((machine == NULL) || (mg == NULL)) {
>  		al->map = NULL;
>  		return;
>  	}

What about the kernel threads?  I guess they're not using thread->mg but
machine->kmaps instead?

Thanks,
Namhyung

Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct

[Namhyung Kim]

Hi Arnaldo,

On Fri, 14 Mar 2014 11:16:12 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Mar 14, 2014 at 03:00:05PM +0100, Jiri Olsa escreveu:
>> Need machine pointer in thread object, so we could
>> lookup the process thread in following patch.
>
> Can't we use the already existing thread->mg.machine for that?

It needs to know the machine when ->mg is not allocated yet.

That means we can now get rid of the mg->machine?

Thanks,
Namhyung

Re: [PATCH 5/5] perf tools: Share process map groups within process threads

[Namhyung Kim]

On Fri, 14 Mar 2014 15:00:06 +0100, Jiri Olsa wrote:
> +	if (!mg) {
> +		struct thread *leader = thread__get_leader(thread);
> +
> +		if (!leader)
> +			return NULL;
> +
> +		if (leader->mg)
> +			mg = leader->mg;
> +		else
> +			mg = thread__map_groups_alloc(leader);
> +
> +		if (leader != thread)
> +			thread->mg = mg;
> +
> +		mg->refcnt++;

What's the value of mg->refcnt here in case of leader != thread and
leader->mg was not allocated originally?  I think it's 1 - but shouldn't
it be 2 since it's referenced from both of leader and the thread now?

Thanks,
Namhyung

Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests

[Jiri Olsa]

On Mon, Mar 17, 2014 at 01:50:12PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Fri, 14 Mar 2014 15:00:02 +0100, Jiri Olsa wrote:
> > +static int thread_init(struct thread_data *td)
> > +{
> > +	void *map;
> > +
> > +	map = mmap(NULL, page_size, PROT_READ|PROT_WRITE,
> > +		   MAP_SHARED|MAP_ANONYMOUS, -1, 0);
> 
> Shouldn't it be an executable mapping to be found by MAP__FUNCTION?

yea.. looks like my arch implies PROT_EXEC via PROT_READ, man mmap:

  On some hardware architectures (e.g., i386), PROT_WRITE implies PROT_READ.   It  is  architecture  dependent  whether  PROT_READ
  implies PROT_EXEC or not.  Portable programs should always set PROT_EXEC if they intend to execute code in the new mapping.


mm/mmap.c:

        /*
         * Does the application expect PROT_READ to imply PROT_EXEC?
         *
         * (the exception is when the underlying filesystem is noexec
         *  mounted, in which case we dont add PROT_EXEC.)
         */
        if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
                if (!(file && (file->f_path.mnt->mnt_flags & MNT_NOEXEC)))
                        prot |= PROT_EXEC;


I'll set the PROT_EXEC as the man page says.


thanks,
jirka

Re: [PATCH 5/5] perf tools: Share process map groups within process threads

[Jiri Olsa]

On Mon, Mar 17, 2014 at 04:25:15PM +0900, Namhyung Kim wrote:
> On Fri, 14 Mar 2014 15:00:06 +0100, Jiri Olsa wrote:
> > +	if (!mg) {
> > +		struct thread *leader = thread__get_leader(thread);
> > +
> > +		if (!leader)
> > +			return NULL;
> > +
> > +		if (leader->mg)
> > +			mg = leader->mg;
> > +		else
> > +			mg = thread__map_groups_alloc(leader);
> > +
> > +		if (leader != thread)
> > +			thread->mg = mg;
> > +
> > +		mg->refcnt++;
> 
> What's the value of mg->refcnt here in case of leader != thread and
> leader->mg was not allocated originally?  I think it's 1 - but shouldn't
> it be 2 since it's referenced from both of leader and the thread now?

right you are..

I need to initialize refcnt to 1 in thread__map_groups_alloc
and increase it only for the '(leader != thread)' case

I'll add some test for this

thanks,
jirka

Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly

[Jiri Olsa]

On Mon, Mar 17, 2014 at 04:13:53PM +0900, Namhyung Kim wrote:
> On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
> > 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.
> > @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
> >  	al->cpumode = cpumode;
> >  	al->filtered = false;
> >  
> > -	if (machine == NULL) {
> > +	if ((machine == NULL) || (mg == NULL)) {
> >  		al->map = NULL;
> >  		return;
> >  	}
> 
> What about the kernel threads?  I guess they're not using thread->mg but
> machine->kmaps instead?

The machine->kmaps is used to store kernel maps - core + modules.

All threads (including kernel ones) are using thread->mg,
kernel threads have empty /proc/x/maps file.

In case the sample address is detected within kernel space,
the machine->kmaps is used instead of thread->mg:

---
  void thread__find_addr_map(struct thread *thread, ...
  {
	struct map_groups *mg = thread__map_groups_get(thread);

  ...

        if (cpumode == PERF_RECORD_MISC_KERNEL && perf_host) {
                al->level = 'k';
                mg = &machine->kmaps;

  ...
	al->map = map_groups__find(mg, type, al->addr);
---

jirka

Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct

[Jiri Olsa]

On Mon, Mar 17, 2014 at 04:17:43PM +0900, Namhyung Kim wrote:
> Hi Arnaldo,
> 
> On Fri, 14 Mar 2014 11:16:12 -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Mar 14, 2014 at 03:00:05PM +0100, Jiri Olsa escreveu:
> >> Need machine pointer in thread object, so we could
> >> lookup the process thread in following patch.
> >
> > Can't we use the already existing thread->mg.machine for that?
> 
> It needs to know the machine when ->mg is not allocated yet.

right

> 
> That means we can now get rid of the mg->machine?

not really, it's heavilly used in kernel symbol handling,
to get machine details.. and there's no kernel thread
specific info, which we could use to get machine pointer

we would need to factor this part to be able to get rid
of mg->machine

jirka

[tip:perf/core] perf machine: Factor machine__find_thread to take tid argument

[tip-bot for Jiri Olsa]

Commit-ID:  d75e6097ef1f7669deb500fbbdf53cfe524f1b53
Gitweb:     http://git.kernel.org/tip/d75e6097ef1f7669deb500fbbdf53cfe524f1b53
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 14 Mar 2014 15:00:03 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 14 Mar 2014 18:08:42 -0300

perf machine: Factor machine__find_thread to take tid argument

Forcing the code to always search thread by pid/tid pair.

The PID value will be needed in future to determine the process thread
leader for map groups sharing.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: 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: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1394805606-25883-3-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/dwarf-unwind.c |  2 +-
 tools/perf/util/machine.c       | 13 +++++++++----
 tools/perf/util/machine.h       |  3 ++-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index f16ea28..c059ee8 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -128,7 +128,7 @@ int test__dwarf_unwind(void)
 	if (verbose > 1)
 		machine__fprintf(machine, stderr);
 
-	thread = machine__find_thread(machine, getpid());
+	thread = machine__find_thread(machine, getpid(), getpid());
 	if (!thread) {
 		pr_err("Could not get thread\n");
 		goto out;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index a679953..5cecd98 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -327,9 +327,10 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
 	return __machine__findnew_thread(machine, pid, tid, true);
 }
 
-struct thread *machine__find_thread(struct machine *machine, pid_t tid)
+struct thread *machine__find_thread(struct machine *machine, pid_t pid,
+				    pid_t tid)
 {
-	return __machine__findnew_thread(machine, 0, tid, false);
+	return __machine__findnew_thread(machine, pid, tid, false);
 }
 
 int machine__process_comm_event(struct machine *machine, union perf_event *event,
@@ -1114,7 +1115,9 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
 int machine__process_fork_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample)
 {
-	struct thread *thread = machine__find_thread(machine, event->fork.tid);
+	struct thread *thread = machine__find_thread(machine,
+						     event->fork.pid,
+						     event->fork.tid);
 	struct thread *parent = machine__findnew_thread(machine,
 							event->fork.ppid,
 							event->fork.ptid);
@@ -1140,7 +1143,9 @@ int machine__process_fork_event(struct machine *machine, union perf_event *event
 int machine__process_exit_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample __maybe_unused)
 {
-	struct thread *thread = machine__find_thread(machine, event->fork.tid);
+	struct thread *thread = machine__find_thread(machine,
+						     event->fork.pid,
+						     event->fork.tid);
 
 	if (dump_trace)
 		perf_event__fprintf_task(event, stdout);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 2e6c248..c8c74a1 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -41,7 +41,8 @@ struct map *machine__kernel_map(struct machine *machine, enum map_type type)
 	return machine->vmlinux_maps[type];
 }
 
-struct thread *machine__find_thread(struct machine *machine, pid_t tid);
+struct thread *machine__find_thread(struct machine *machine, pid_t pid,
+				    pid_t tid);
 
 int machine__process_comm_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample);

Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly

[Jiri Olsa]

On Tue, Mar 18, 2014 at 04:03:41PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon, 17 Mar 2014 15:52:59 +0100, Jiri Olsa wrote:
> > On Mon, Mar 17, 2014 at 04:13:53PM +0900, Namhyung Kim wrote:
> >> On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
> >> > 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.
> >> > @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
> >> >  	al->cpumode = cpumode;
> >> >  	al->filtered = false;
> >> >  
> >> > -	if (machine == NULL) {
> >> > +	if ((machine == NULL) || (mg == NULL)) {
> >> >  		al->map = NULL;
> >> >  		return;
> >> >  	}
> >> 
> >> What about the kernel threads?  I guess they're not using thread->mg but
> >> machine->kmaps instead?
> >
> > The machine->kmaps is used to store kernel maps - core + modules.
> >
> > All threads (including kernel ones) are using thread->mg,
> > kernel threads have empty /proc/x/maps file.
> >
> > In case the sample address is detected within kernel space,
> > the machine->kmaps is used instead of thread->mg:
> 
> That means kernel threads don't need to allocate ->mg, right?  So I'd
> suggest checking cpumode first and then allocate ->mg for userspace maps
> only.

good idea, will do

thanks,
jirka