* [PATCH 1/5] perf tests: Add tip/pid mmap automated tests
2014-03-14 14:00 [RFC 0/5] perf tools: Share map groups within process Jiri Olsa
@ 2014-03-14 14:00 ` Jiri Olsa
2014-03-14 20:24 ` Arnaldo Carvalho de Melo
2014-03-17 4:50 ` Namhyung Kim
2014-03-14 14:00 ` [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument Jiri Olsa
` (3 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-14 14:00 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
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests
2014-03-14 14:00 ` [PATCH 1/5] perf tests: Add tip/pid mmap automated tests Jiri Olsa
@ 2014-03-14 20:24 ` Arnaldo Carvalho de Melo
2014-03-17 6:57 ` Jiri Olsa
2014-03-17 4:50 ` Namhyung Kim
1 sibling, 1 reply; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-14 20:24 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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]$
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests
2014-03-14 20:24 ` Arnaldo Carvalho de Melo
@ 2014-03-17 6:57 ` Jiri Olsa
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-17 6:57 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests
2014-03-14 14:00 ` [PATCH 1/5] perf tests: Add tip/pid mmap automated tests Jiri Olsa
2014-03-14 20:24 ` Arnaldo Carvalho de Melo
@ 2014-03-17 4:50 ` Namhyung Kim
2014-03-17 10:39 ` Jiri Olsa
1 sibling, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2014-03-17 4:50 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
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 1/5] perf tests: Add tip/pid mmap automated tests
2014-03-17 4:50 ` Namhyung Kim
@ 2014-03-17 10:39 ` Jiri Olsa
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-17 10:39 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument
2014-03-14 14:00 [RFC 0/5] perf tools: Share map groups within process Jiri Olsa
2014-03-14 14:00 ` [PATCH 1/5] perf tests: Add tip/pid mmap automated tests Jiri Olsa
@ 2014-03-14 14:00 ` Jiri Olsa
2014-03-14 14:15 ` Arnaldo Carvalho de Melo
2014-03-18 8:31 ` [tip:perf/core] perf machine: " tip-bot for Jiri Olsa
2014-03-14 14:00 ` [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly Jiri Olsa
` (2 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-14 14:00 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
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument
2014-03-14 14:00 ` [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument Jiri Olsa
@ 2014-03-14 14:15 ` Arnaldo Carvalho de Melo
2014-03-14 15:40 ` Adrian Hunter
2014-03-18 8:31 ` [tip:perf/core] perf machine: " tip-bot for Jiri Olsa
1 sibling, 1 reply; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-14 14:15 UTC (permalink / raw)
To: Adrian Hunter
Cc: Jiri Olsa, linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument
2014-03-14 14:15 ` Arnaldo Carvalho de Melo
@ 2014-03-14 15:40 ` Adrian Hunter
0 siblings, 0 replies; 23+ messages in thread
From: Adrian Hunter @ 2014-03-14 15:40 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [tip:perf/core] perf machine: Factor machine__find_thread to take tid argument
2014-03-14 14:00 ` [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument Jiri Olsa
2014-03-14 14:15 ` Arnaldo Carvalho de Melo
@ 2014-03-18 8:31 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 23+ messages in thread
From: tip-bot for Jiri Olsa @ 2014-03-18 8:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, namhyung,
jolsa, fweisbec, adrian.hunter, dsahern, tglx, cjashfor, dzickus
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);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly
2014-03-14 14:00 [RFC 0/5] perf tools: Share map groups within process Jiri Olsa
2014-03-14 14:00 ` [PATCH 1/5] perf tests: Add tip/pid mmap automated tests Jiri Olsa
2014-03-14 14:00 ` [PATCH 2/5] perf tools: Factor machine__find_thread to take tid argument Jiri Olsa
@ 2014-03-14 14:00 ` Jiri Olsa
2014-03-14 14:13 ` Arnaldo Carvalho de Melo
2014-03-17 7:13 ` Namhyung Kim
2014-03-14 14:00 ` [PATCH 4/5] perf tools: Add machine pointer into thread struct Jiri Olsa
2014-03-14 14:00 ` [PATCH 5/5] perf tools: Share process map groups within process threads Jiri Olsa
4 siblings, 2 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-14 14:00 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
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly
2014-03-14 14:00 ` [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly Jiri Olsa
@ 2014-03-14 14:13 ` Arnaldo Carvalho de Melo
2014-03-17 7:13 ` Namhyung Kim
1 sibling, 0 replies; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-14 14:13 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly
2014-03-14 14:00 ` [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly Jiri Olsa
2014-03-14 14:13 ` Arnaldo Carvalho de Melo
@ 2014-03-17 7:13 ` Namhyung Kim
2014-03-17 14:52 ` Jiri Olsa
1 sibling, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2014-03-17 7:13 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
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly
2014-03-17 7:13 ` Namhyung Kim
@ 2014-03-17 14:52 ` Jiri Olsa
[not found] ` <87lhw8rnsi.fsf@sejong.aot.lge.com>
0 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2014-03-17 14:52 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 4/5] perf tools: Add machine pointer into thread struct
2014-03-14 14:00 [RFC 0/5] perf tools: Share map groups within process Jiri Olsa
` (2 preceding siblings ...)
2014-03-14 14:00 ` [PATCH 3/5] perf tools: Allocate thread map_groups dynamicaly Jiri Olsa
@ 2014-03-14 14:00 ` Jiri Olsa
2014-03-14 14:16 ` Arnaldo Carvalho de Melo
2014-03-14 14:00 ` [PATCH 5/5] perf tools: Share process map groups within process threads Jiri Olsa
4 siblings, 1 reply; 23+ messages in thread
From: Jiri Olsa @ 2014-03-14 14:00 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
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct
2014-03-14 14:00 ` [PATCH 4/5] perf tools: Add machine pointer into thread struct Jiri Olsa
@ 2014-03-14 14:16 ` Arnaldo Carvalho de Melo
2014-03-17 7:17 ` Namhyung Kim
0 siblings, 1 reply; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-14 14:16 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Don Zickus, Corey Ashford, Adrian Hunter,
David Ahern, Frederic Weisbecker, Ingo Molnar, Namhyung Kim,
Paul Mackerras, Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct
2014-03-14 14:16 ` Arnaldo Carvalho de Melo
@ 2014-03-17 7:17 ` Namhyung Kim
2014-03-17 14:56 ` Jiri Olsa
0 siblings, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2014-03-17 7:17 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, linux-kernel, Don Zickus, Corey Ashford, Adrian Hunter,
David Ahern, Frederic Weisbecker, Ingo Molnar, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/5] perf tools: Add machine pointer into thread struct
2014-03-17 7:17 ` Namhyung Kim
@ 2014-03-17 14:56 ` Jiri Olsa
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-17 14:56 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, linux-kernel, Don Zickus, Corey Ashford,
Adrian Hunter, David Ahern, Frederic Weisbecker, Ingo Molnar,
Paul Mackerras, Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 5/5] perf tools: Share process map groups within process threads
2014-03-14 14:00 [RFC 0/5] perf tools: Share map groups within process Jiri Olsa
` (3 preceding siblings ...)
2014-03-14 14:00 ` [PATCH 4/5] perf tools: Add machine pointer into thread struct Jiri Olsa
@ 2014-03-14 14:00 ` Jiri Olsa
2014-03-14 14:18 ` Arnaldo Carvalho de Melo
2014-03-17 7:25 ` Namhyung Kim
4 siblings, 2 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-14 14:00 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
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 5/5] perf tools: Share process map groups within process threads
2014-03-14 14:00 ` [PATCH 5/5] perf tools: Share process map groups within process threads Jiri Olsa
@ 2014-03-14 14:18 ` Arnaldo Carvalho de Melo
2014-03-17 7:25 ` Namhyung Kim
1 sibling, 0 replies; 23+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-03-14 14:18 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Namhyung Kim, Paul Mackerras,
Peter Zijlstra
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/5] perf tools: Share process map groups within process threads
2014-03-14 14:00 ` [PATCH 5/5] perf tools: Share process map groups within process threads Jiri Olsa
2014-03-14 14:18 ` Arnaldo Carvalho de Melo
@ 2014-03-17 7:25 ` Namhyung Kim
2014-03-17 10:45 ` Jiri Olsa
1 sibling, 1 reply; 23+ messages in thread
From: Namhyung Kim @ 2014-03-17 7:25 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
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 5/5] perf tools: Share process map groups within process threads
2014-03-17 7:25 ` Namhyung Kim
@ 2014-03-17 10:45 ` Jiri Olsa
0 siblings, 0 replies; 23+ messages in thread
From: Jiri Olsa @ 2014-03-17 10:45 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, Don Zickus, Corey Ashford, David Ahern,
Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
Arnaldo Carvalho de Melo
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
^ permalink raw reply [flat|nested] 23+ messages in thread