perf tools: Share map groups within process
v2 cover perf tools: Share map groups within process
hi,
this patchset moves thread's map_groups to be dynamically allocated and shared within process threads.
The main benefit would be to be able to look up memory map from any thread that belongs to the process.
This implements one of the solution ideas for issue described by Don in following thread:
http://marc.info/?l=linux-kernel&m=139403876017159&w=2
v2 changes:
- changing mmap groups allocation strategy - have it allocated always when thread is created, which seems less confusing and less error prone. The previous strategy was to allocate it when it's actually used.
- fix test compilation for i386
RFC changes:
- added automated test for thread map groups get/put methods
- fix reference count for case described by Namhyung
- rename the mmap-events.c test to mmap-thread-lookup.c
- added PROT_EXEC to mmap call in tests/mmap-thread-lookup.c as it's not implied by default on all archs (Namhyung)
- lazy mg allocation in thread__find_addr_map (Namhyung)
- fix compilation failures (Arnaldo)
also available in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_mmaps
thanks,
jirka
---
Arnaldo Carvalho de Melo (2):
perf tools: Allocate thread map_groups's dynamically perf tools: Reference count map_groups objects
Jiri Olsa (3):
perf tests: Add thread maps lookup automated tests perf tools: Share map_groups among threads of the same group perf tests: Add map groups sharing with thread object test
tools/perf/Makefile.perf | 2 + tools/perf/arch/x86/tests/dwarf-unwind.c | 2 +- tools/perf/perf.h | 6 +++ tools/perf/tests/builtin-test.c | 8 ++++ tools/perf/tests/mmap-thread-lookup.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 2 + tools/perf/tests/thread-mg-share.c | 90 +++++++++++++++++++++++++++++++++++ tools/perf/ui/stdio/hist.c | 2 +- tools/perf/util/event.c | 2 +- tools/perf/util/machine.c | 11 +++++ tools/perf/util/map.c | 23 +++++++++ tools/perf/util/map.h | 12 +++++ tools/perf/util/thread.c | 52 +++++++++++++++----- tools/perf/util/thread.h | 3 +- 14 files changed, 432 insertions(+), 16 deletions(-) create mode 100644 tools/perf/tests/mmap-thread-lookup.c create mode 100644 tools/perf/tests/thread-mg-share.c
PATCH v2 1/5 perf tests: Add thread maps lookup automated tests
Adding automated test for memory maps lookup within multiple machines threads.
The test creates 4 threads and separated memory maps.
It checks that we could use thread__find_addr_map function with thread object based on TID to find memory maps.
--- tools/perf/Makefile.perf | 1 + tools/perf/perf.h | 6 + tools/perf/tests/builtin-test.c | 4 + tools/perf/tests/mmap-thread-lookup.c | 233 ++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 5 files changed, 245 insertions(+) create mode 100644 tools/perf/tests/mmap-thread-lookup.c
tools/perf/Makefile.perf +1 -0
@@ -414,6 +414,7 @@ ifeq ($(ARCH),x86) LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o endif endif +LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/perf.h +6 -0
@@ -15,6 +15,9 @@#ifndef __NR_futex# define __NR_futex 240#endif +#ifndef __NR_gettid +# define __NR_gettid 224 +#endif #endif #if defined(__x86_64__)
@@ -29,6 +32,9 @@#ifndef __NR_futex# define __NR_futex 202#endif +#ifndef __NR_gettid +# define __NR_gettid 186 +#endif #endif #ifdef __powerpc__
tools/perf/tests/builtin-test.c +4 -0
@@ -124,6 +124,10 @@ static struct test {#endif #endif { + .desc = "Test mmap thread lookup", + .func = test__mmap_thread_lookup, + }, + { .func = NULL, }, };
tools/perf/tests/mmap-thread-lookup.c new file +233 -0
233 lines not on this page · raw message
tools/perf/tests/tests.h +1 -0
@@ -41,6 +41,7 @@ int test__sample_parsing(void);int test__keep_tracking (void );int test__parse_no_sample_id_all (void );int test__dwarf_unwind (void ); +int test__mmap_thread_lookup (void );#if defined(__x86_64__) || defined(__i386__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
Signature · 1 line
1.8.3.1
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tests: Add thread maps lookup automated tests
perf tests: Add thread maps lookup automated tests
Commit-ID: 4e85edfc3f5c0e016a960c1dcbe0217e86602525 Gitweb: http://git.kernel.org/tip/4e85edfc3f5c0e016a960c1dcbe0217e86602525 Author: Jiri Olsa <[email protected]> AuthorDate: Wed, 5 Mar 2014 17:20:31 +0100 Committer: Jiri Olsa <[email protected]> CommitDate: Mon, 28 Apr 2014 13:42:52 +0200
perf tests: Add thread maps lookup automated tests
Adding automated test for memory maps lookup within multiple machines threads.
The test creates 4 threads and separated memory maps. It checks that we could use thread__find_addr_map function with thread object based on TID to find memory maps.
--- tools/perf/Makefile.perf | 1 + tools/perf/perf.h | 6 + tools/perf/tests/builtin-test.c | 4 + tools/perf/tests/mmap-thread-lookup.c | 233 ++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 5 files changed, 245 insertions(+)
tools/perf/Makefile.perf +1 -0
@@ -416,6 +416,7 @@ ifeq ($(ARCH),x86) LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o endif endif +LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/perf.h +6 -0
@@ -15,6 +15,9 @@#ifndef __NR_futex# define __NR_futex 240#endif +#ifndef __NR_gettid +# define __NR_gettid 224 +#endif #endif #if defined(__x86_64__)
@@ -29,6 +32,9 @@#ifndef __NR_futex# define __NR_futex 202#endif +#ifndef __NR_gettid +# define __NR_gettid 186 +#endif #endif #ifdef __powerpc__
tools/perf/tests/builtin-test.c +4 -0
@@ -128,6 +128,10 @@ static struct test { .func = test__hists_filter, }, { + .desc = "Test mmap thread lookup", + .func = test__mmap_thread_lookup, + }, + { .func = NULL, }, };
tools/perf/tests/mmap-thread-lookup.c new file +233 -0
233 lines not on this page · raw message
tools/perf/tests/tests.h +1 -0
@@ -42,6 +42,7 @@ int test__keep_tracking(void);int test__parse_no_sample_id_all (void );int test__dwarf_unwind (void );int test__hists_filter (void ); +int test__mmap_thread_lookup (void );#if defined(__x86_64__) || defined(__i386__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
PATCH v2 2/5 perf tools: Allocate thread map_groups's dynamically
In reply to Jiri OlsaFrom: Arnaldo Carvalho de Melo <[email protected]>
Moving towards sharing map groups within a process threads.
Because of this we need the map groups to be dynamically allocated. No other functional change is intended in here.
Based on a patch by Jiri Olsa, but this time _just_ making the conversion from statically allocating thread->mg to turning it into a pointer and instead of initializing it at thread's constructor, introduce a constructor/destructor for the map_groups class and call at thread creation time.
Later we will introduce the get/put methods when we move to sharing those map_groups, when the get/put refcounting semantics will be needed.
--- tools/perf/arch/x86/tests/dwarf-unwind.c | 2 +- tools/perf/ui/stdio/hist.c | 2 +- tools/perf/util/event.c | 2 +- tools/perf/util/map.c | 16 ++++++++++++++++ tools/perf/util/map.h | 3 +++ tools/perf/util/thread.c | 18 ++++++++++++------ tools/perf/util/thread.h | 2 +- 7 files changed, 35 insertions(+), 10 deletions(-)
tools/perf/arch/x86/tests/dwarf-unwind.c +1 -1
@@ -23,7 +23,7 @@ static int sample_ustack(struct perf_sample *sample, sp = (unsigned long ) regs[PERF_REG_X86_SP ]; - map =map_groups__find (&thread->mg,MAP__FUNCTION , (u64 ) sp); + map =map_groups__find (thread->mg,MAP__FUNCTION , (u64 ) sp);if (!map) {pr_debug ("failed to get stack map\n" );return -1 ;
tools/perf/ui/stdio/hist.c +1 -1
@@ -495,7 +495,7 @@ print_entries:break ;if (h->ms.map ==NULL && verbose >1 ) { -__map_groups__fprintf_maps (&h->thread->mg, +__map_groups__fprintf_maps (h->thread->mg,MAP__FUNCTION , verbose, fp);fprintf (fp,"%.10s end\n" , graph_dotted_line); }
tools/perf/util/event.c +1 -1
@@ -699,7 +699,7 @@ void thread__find_addr_map(struct thread *thread, enum map_type type, u64 addr, struct addr_location *al) { - struct map_groups *mg = &thread->mg; + struct map_groups *mg = thread->mg;bool load_map = false; al->machine = machine;
tools/perf/util/map.c +16 -0
@@ -358,6 +358,22 @@ void map_groups__exit(struct map_groups *mg) } } +struct map_groups *map_groups__new (void ) +{ +struct map_groups *mg =malloc (sizeof (*mg)); + +if (mg !=NULL ) +map_groups__init (mg); + +return mg; +} + +void map_groups__delete (struct map_groups *mg) +{ +map_groups__exit (mg); +free (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;
tools/perf/util/map.h +3 -0
@@ -61,6 +61,9 @@ struct map_groups {struct machine *machine; }; +struct map_groups *map_groups__new (void ); +void map_groups__delete (struct map_groups *mg); +static inline struct kmap *map__kmap (struct map *map) {return (struct kmap *)(map +1 );
tools/perf/util/thread.c +12 -6
@@ -15,7 +15,10 @@ struct thread *thread__new(pid_t pid, pid_t tid)struct thread *thread =zalloc (sizeof (*thread));if (thread !=NULL ) { -map_groups__init (&thread->mg); + thread->mg =map_groups__new (); +if (thread->mg ==NULL ) + goto out_free; + thread->pid_ = pid; thread->tid = tid; thread->ppid =-1 ;
@@ -37,6 +40,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)return thread; err_thread: +map_groups__delete (thread->mg); +out_free:free (thread);return NULL ; }
@@ -45,7 +50,8 @@ void thread__delete(struct thread *thread) {struct comm *comm, *tmp; -map_groups__exit (&thread->mg); +map_groups__delete (thread->mg); + thread->mg =NULL ;list_for_each_entry_safe (comm, tmp, &thread->comm_list, list) {list_del (&comm->list);comm__free (comm);
@@ -111,13 +117,13 @@ int thread__comm_len(struct thread *thread)size_t thread__fprintf (struct thread *thread,FILE *fp) {return fprintf (fp,"Thread %d %s\n" , thread->tid,thread__comm_str (thread)) + -map_groups__fprintf (&thread->mg, verbose, fp); +map_groups__fprintf (thread->mg, verbose, fp); }void thread__insert_map (struct thread *thread,struct map *map) { -map_groups__fixup_overlappings (&thread->mg, map, verbose, stderr); -map_groups__insert (&thread->mg, map); +map_groups__fixup_overlappings (thread->mg, map, verbose, stderr); +map_groups__insert (thread->mg, map); }int thread__fork (struct thread *thread,struct thread *parent,u64 timestamp)
@@ -135,7 +141,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp) }for (i =0 ; i <MAP__NR_TYPES ; ++i) -if (map_groups__clone (&thread->mg, &parent->mg, i) <0 ) +if (map_groups__clone (thread->mg, parent->mg, i) <0 )return -ENOMEM ; thread->ppid = parent->tid;
tools/perf/util/thread.h +1 -1
@@ -13,7 +13,7 @@ struct thread {struct rb_node rb_node;struct list_head node; }; -struct map_groups mg; +struct map_groups *mg;pid_t pid_;/* Not all tools update this */ pid_t tid;pid_t ppid;
Signature · 1 line
1.8.3.1
tip-bot for Arnaldo Carvalho de Melo
merge bot[tip:perf/core] perf tools: Allocate thread map_groups' s dynamically
perf tools: Allocate thread map_groups's dynamically
Commit-ID: 93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45 Gitweb: http://git.kernel.org/tip/93d5731dcb5b8cb7fa56ee11a5891f10c96c2a45 Author: Arnaldo Carvalho de Melo <[email protected]> AuthorDate: Fri, 21 Mar 2014 17:57:01 -0300 Committer: Jiri Olsa <[email protected]> CommitDate: Mon, 28 Apr 2014 13:43:20 +0200
perf tools: Allocate thread map_groups's dynamically
Moving towards sharing map groups within a process threads.
Because of this we need the map groups to be dynamically allocated. No other functional change is intended in here.
Based on a patch by Jiri Olsa, but this time _just_ making the conversion from statically allocating thread->mg to turning it into a pointer and instead of initializing it at thread's constructor, introduce a constructor/destructor for the map_groups class and call at thread creation time.
Later we will introduce the get/put methods when we move to sharing those map_groups, when the get/put refcounting semantics will be needed.
--- tools/perf/arch/x86/tests/dwarf-unwind.c | 2 +- tools/perf/ui/stdio/hist.c | 2 +- tools/perf/util/event.c | 2 +- tools/perf/util/map.c | 16 ++++++++++++++++ tools/perf/util/map.h | 3 +++ tools/perf/util/thread.c | 18 ++++++++++++------ tools/perf/util/thread.h | 2 +- 7 files changed, 35 insertions(+), 10 deletions(-)
tools/perf/arch/x86/tests/dwarf-unwind.c +1 -1
@@ -23,7 +23,7 @@ static int sample_ustack(struct perf_sample *sample, sp = (unsigned long ) regs[PERF_REG_X86_SP ]; - map =map_groups__find (&thread->mg,MAP__FUNCTION , (u64 ) sp); + map =map_groups__find (thread->mg,MAP__FUNCTION , (u64 ) sp);if (!map) {pr_debug ("failed to get stack map\n" );free (buf);
tools/perf/ui/stdio/hist.c +1 -1
@@ -495,7 +495,7 @@ print_entries:break ;if (h->ms.map ==NULL && verbose >1 ) { -__map_groups__fprintf_maps (&h->thread->mg, +__map_groups__fprintf_maps (h->thread->mg,MAP__FUNCTION , verbose, fp);fprintf (fp,"%.10s end\n" , graph_dotted_line); }
tools/perf/util/event.c +1 -1
@@ -699,7 +699,7 @@ void thread__find_addr_map(struct thread *thread, enum map_type type, u64 addr, struct addr_location *al) { - struct map_groups *mg = &thread->mg; + struct map_groups *mg = thread->mg;bool load_map = false; al->machine = machine;
tools/perf/util/map.c +16 -0
@@ -358,6 +358,22 @@ void map_groups__exit(struct map_groups *mg) } } +struct map_groups *map_groups__new (void ) +{ +struct map_groups *mg =malloc (sizeof (*mg)); + +if (mg !=NULL ) +map_groups__init (mg); + +return mg; +} + +void map_groups__delete (struct map_groups *mg) +{ +map_groups__exit (mg); +free (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;
tools/perf/util/map.h +3 -0
@@ -61,6 +61,9 @@ struct map_groups {struct machine *machine; }; +struct map_groups *map_groups__new (void ); +void map_groups__delete (struct map_groups *mg); +static inline struct kmap *map__kmap (struct map *map) {return (struct kmap *)(map +1 );
tools/perf/util/thread.c +12 -6
@@ -15,7 +15,10 @@ struct thread *thread__new(pid_t pid, pid_t tid)struct thread *thread =zalloc (sizeof (*thread));if (thread !=NULL ) { -map_groups__init (&thread->mg); + thread->mg =map_groups__new (); +if (thread->mg ==NULL ) + goto out_free; + thread->pid_ = pid; thread->tid = tid; thread->ppid =-1 ;
@@ -37,6 +40,8 @@ struct thread *thread__new(pid_t pid, pid_t tid)return thread; err_thread: +map_groups__delete (thread->mg); +out_free:free (thread);return NULL ; }
@@ -45,7 +50,8 @@ void thread__delete(struct thread *thread) {struct comm *comm, *tmp; -map_groups__exit (&thread->mg); +map_groups__delete (thread->mg); + thread->mg =NULL ;list_for_each_entry_safe (comm, tmp, &thread->comm_list, list) {list_del (&comm->list);comm__free (comm);
@@ -111,13 +117,13 @@ int thread__comm_len(struct thread *thread)size_t thread__fprintf (struct thread *thread,FILE *fp) {return fprintf (fp,"Thread %d %s\n" , thread->tid,thread__comm_str (thread)) + -map_groups__fprintf (&thread->mg, verbose, fp); +map_groups__fprintf (thread->mg, verbose, fp); }void thread__insert_map (struct thread *thread,struct map *map) { -map_groups__fixup_overlappings (&thread->mg, map, verbose, stderr); -map_groups__insert (&thread->mg, map); +map_groups__fixup_overlappings (thread->mg, map, verbose, stderr); +map_groups__insert (thread->mg, map); }int thread__fork (struct thread *thread,struct thread *parent,u64 timestamp)
@@ -135,7 +141,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp) }for (i =0 ; i <MAP__NR_TYPES ; ++i) -if (map_groups__clone (&thread->mg, &parent->mg, i) <0 ) +if (map_groups__clone (thread->mg, parent->mg, i) <0 )return -ENOMEM ; thread->ppid = parent->tid;
tools/perf/util/thread.h +1 -1
@@ -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;
PATCH v2 3/5 perf tools: Reference count map_groups objects
In reply to Jiri OlsaFrom: Arnaldo Carvalho de Melo <[email protected]>
We will share it among threads in the same process.
--- tools/perf/util/map.c | 7 +++++++ tools/perf/util/map.h | 9 +++++++++ tools/perf/util/thread.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/map.c +7 -0
@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)INIT_LIST_HEAD (&mg->removed_maps[i]); } mg->machine =NULL ; + mg->refcnt =1 ; }static void maps__delete (struct rb_root *maps)
@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)free (mg); } +void map_groups__put (struct map_groups *mg) +{ +if (--mg->refcnt ==0 ) +map_groups__delete (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;
tools/perf/util/map.h +9 -0
@@ -59,11 +59,20 @@ struct map_groups {struct rb_root maps[MAP__NR_TYPES ];struct list_head removed_maps[MAP__NR_TYPES ];struct machine *machine; +int refcnt; };struct map_groups *map_groups__new (void );void map_groups__delete (struct map_groups *mg); +static inline struct map_groups *map_groups__get (struct map_groups *mg) +{ + ++mg->refcnt; +return mg; +} + +void map_groups__put (struct map_groups *mg); +static inline struct kmap *map__kmap (struct map *map) {return (struct kmap *)(map +1 );
tools/perf/util/thread.c +1 -1
@@ -50,7 +50,7 @@ void thread__delete(struct thread *thread) {struct comm *comm, *tmp; -map_groups__delete (thread->mg); +map_groups__put (thread->mg); thread->mg =NULL ;list_for_each_entry_safe (comm, tmp, &thread->comm_list, list) {list_del (&comm->list);
Signature · 1 line
1.8.3.1
Re: [PATCH 3/5] perf tools: Reference count map_groups objects
Quoted 51 lines · Jiri Olsa
From: Arnaldo Carvalho de Melo <[email protected]>
We will share it among threads in the same process.
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>Cc Adrian Hunter <[email protected]>Cc David Ahern <[email protected]>Cc Don Zickus <[email protected]>Cc Frederic Weisbecker <[email protected]>Cc Mike Galbraith <[email protected]>Cc Namhyung Kim <[email protected]>Cc Paul Mackerras <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/util/map.c | 7 +++++++ tools/perf/util/map.h | 9 +++++++++ tools/perf/util/thread.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)tools/perf/util/map.c +7
-0@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)INIT_LIST_HEAD (&mg->removed_maps[i]); } mg->machine =NULL ; + mg->refcnt =1 ; }static void maps__delete (struct rb_root *maps)@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)free (mg); } +void map_groups__put (struct map_groups *mg) +{ +if (--mg->refcnt ==0 ) +map_groups__delete (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;tools/perf/util/map.h +0
-0
tools/perf/util/map.h struct map_groups { · line 63
Quoted 5 lines
tools/perf/util/map.h +1
-0@@ -59,11 +59,20 @@ struct map_groups {struct rb_root maps[MAP__NR_TYPES ];struct list_head removed_maps[MAP__NR_TYPES ];struct machine *machine; +int refcnt; };
atomic for refcnt? This is part of a libperf; would be good to support multithreaded users.
David
Re: [PATCH 3/5] perf tools: Reference count map_groups objects
Quoted 1 line · David Ahern
On 4/14/14, 9:52 AM, Jiri Olsa wrote:
SNIP
Quoted 16 lines
Quoted 16 lines
tools/perf/util/map.c +6
-0+void map_groups__put (struct map_groups *mg) +{ +if (--mg->refcnt ==0 ) +map_groups__delete (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;tools/perf/util/map.h +0
-0@@ -59,11 +59,20 @@ struct map_groups {struct rb_root maps[MAP__NR_TYPES ];
Quoted 8 lines
tools/perf/util/map.h struct map_groups { · line 63
Quoted 4 lines
tools/perf/util/map.h +1
-0struct list_head removed_maps[MAP__NR_TYPES ];struct machine *machine; +int refcnt; };atomic for refcnt? This is part of a libperf; would be good to support multithreaded users.
hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well
not sure what's the support in user space for that.. will check
also not to be negative, but libperf is not thread safe anyway, right? ;-)
thanks,
jirka
Re: [PATCH 3/5] perf tools: Reference count map_groups objects
Quoted 7 lines · Jiri Olsa
hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well
not sure what's the support in user space for that.. will check
also not to be negative, but libperf is not thread safe anyway, right? ;-)
should be. pretty certain there have been bug reports (e.g, perf-top) that stumble onto problems and they get fixed.
David
Re: [PATCH 3/5] perf tools: Reference count map_groups objects
Quoted 11 lines · David Ahern
Quoted 7 lines · Jiri Olsa
hum.. I think using atomic type is not enough, we'd need to make map_groups__put/get atomic as well
not sure what's the support in user space for that.. will check
also not to be negative, but libperf is not thread safe anyway, right? ;-)
should be. pretty certain there have been bug reports (e.g, perf-top) that stumble onto problems and they get fixed.
I checked and there's no problem with this in perf top, because all thread allocations and frees (included map_groups) are done within the main (one) thread
You could get in trouble if you'd call map_groups__get and map_groups__put over same map_groups object from different threads, but this never happens in perf
Thats the same sort of issue you would get if you called machine__findnew_thread over same machine from 2 threads
I think that potential libperf users should be aware of the proper usage or they should ask for change
thoughts? ;-) thanks,
jirka
Re: [PATCH 3/5] perf tools: Reference count map_groups objects
Quoted 1 line · Jiri Olsa
thoughts?;-) thanks,
Only that libperf is still a pipe dream. Don't hold up progress for this, but we should be thinking about making it thread safe.
David
tip-bot for Arnaldo Carvalho de Melo
merge bot[tip:perf/core] perf tools: Reference count map_groups objects
In reply to Jiri Olsaperf tools: Reference count map_groups objects
Commit-ID: a26ca6716a6c683f40bd676cea7e89704653b98d Gitweb: http://git.kernel.org/tip/a26ca6716a6c683f40bd676cea7e89704653b98d Author: Arnaldo Carvalho de Melo <[email protected]> AuthorDate: Tue, 25 Mar 2014 15:26:44 -0300 Committer: Jiri Olsa <[email protected]> CommitDate: Mon, 28 Apr 2014 13:43:26 +0200
perf tools: Reference count map_groups objects
We will share it among threads in the same process. Adding map_groups__get/map_groups__put interface for that.
--- tools/perf/util/map.c | 7 +++++++ tools/perf/util/map.h | 9 +++++++++ tools/perf/util/thread.c | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-)
tools/perf/util/map.c +7 -0
@@ -323,6 +323,7 @@ void map_groups__init(struct map_groups *mg)INIT_LIST_HEAD (&mg->removed_maps[i]); } mg->machine =NULL ; + mg->refcnt =1 ; }static void maps__delete (struct rb_root *maps)
@@ -374,6 +375,12 @@ void map_groups__delete(struct map_groups *mg)free (mg); } +void map_groups__put (struct map_groups *mg) +{ +if (--mg->refcnt ==0 ) +map_groups__delete (mg); +} +void map_groups__flush (struct map_groups *mg) {int type;
tools/perf/util/map.h +9 -0
@@ -59,11 +59,20 @@ struct map_groups {struct rb_root maps[MAP__NR_TYPES ];struct list_head removed_maps[MAP__NR_TYPES ];struct machine *machine; +int refcnt; };struct map_groups *map_groups__new (void );void map_groups__delete (struct map_groups *mg); +static inline struct map_groups *map_groups__get (struct map_groups *mg) +{ + ++mg->refcnt; +return mg; +} + +void map_groups__put (struct map_groups *mg); +static inline struct kmap *map__kmap (struct map *map) {return (struct kmap *)(map +1 );
tools/perf/util/thread.c +1 -1
@@ -50,7 +50,7 @@ void thread__delete(struct thread *thread) {struct comm *comm, *tmp; -map_groups__delete (thread->mg); +map_groups__put (thread->mg); thread->mg =NULL ;list_for_each_entry_safe (comm, tmp, &thread->comm_list, list) {list_del (&comm->list);
PATCH v2 4/5 perf tools: Share map_groups among threads of the same group
In reply to Jiri OlsaSharing map groups within all process threads. This way there's only one copy of mmap info and it's reachable from any thread within the process.
--- tools/perf/util/machine.c | 11 +++++++++++ tools/perf/util/thread.c | 48 ++++++++++++++++++++++++++++++++++------------- tools/perf/util/thread.h | 1 + 3 files changed, 47 insertions(+), 13 deletions(-)
tools/perf/util/machine.c +11 -0
@@ -316,6 +316,17 @@ static struct thread *__machine__findnew_thread(struct machine *machine,rb_link_node (&th->rb_node, parent, p);rb_insert_color (&th->rb_node, &machine->threads); machine->last_match = th; + +/* +* We have to initialize map_groups separately +* after rb tree is updated. +* +* The reason is that we call machine__findnew_thread +* within thread__init_map_groups to find the thread +* leader and that would screwed the rb tree. +*/ +if (thread__init_map_groups (th, machine)) +return NULL ; }return th;
tools/perf/util/thread.c +35 -13
81 lines not on this page · raw message
tools/perf/util/thread.h +1 -0
@@ -30,6 +30,7 @@ struct machine;struct comm ;struct thread *thread__new (pid_t pid,pid_t tid); +int thread__init_map_groups (struct thread *thread,struct machine *machine);void thread__delete (struct thread *thread);static inline void thread__exited (struct thread *thread) {
Signature · 1 line
1.8.3.1
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Share map_groups among threads of the same group
perf tools: Share map_groups among threads of the same group
Commit-ID: cddcef607782966f1601808c17fe9c4c5f79f9f4 Gitweb: http://git.kernel.org/tip/cddcef607782966f1601808c17fe9c4c5f79f9f4 Author: Jiri Olsa <[email protected]> AuthorDate: Wed, 9 Apr 2014 20:54:29 +0200 Committer: Jiri Olsa <[email protected]> CommitDate: Mon, 28 Apr 2014 13:43:33 +0200
perf tools: Share map_groups among threads of the same group
Sharing map groups within all process threads. This way there's only one copy of mmap info and it's reachable from any thread within the process.
--- tools/perf/util/machine.c | 11 +++++++++++ tools/perf/util/thread.c | 48 ++++++++++++++++++++++++++++++++++------------- tools/perf/util/thread.h | 1 + 3 files changed, 47 insertions(+), 13 deletions(-)
tools/perf/util/machine.c +11 -0
@@ -316,6 +316,17 @@ static struct thread *__machine__findnew_thread(struct machine *machine,rb_link_node (&th->rb_node, parent, p);rb_insert_color (&th->rb_node, &machine->threads); machine->last_match = th; + +/* +* We have to initialize map_groups separately +* after rb tree is updated. +* +* The reason is that we call machine__findnew_thread +* within thread__init_map_groups to find the thread +* leader and that would screwed the rb tree. +*/ +if (thread__init_map_groups (th, machine)) +return NULL ; }return th;
tools/perf/util/thread.c +35 -13
81 lines not on this page · raw message
tools/perf/util/thread.h +1 -0
@@ -30,6 +30,7 @@ struct machine;struct comm ;struct thread *thread__new (pid_t pid,pid_t tid); +int thread__init_map_groups (struct thread *thread,struct machine *machine);void thread__delete (struct thread *thread);static inline void thread__exited (struct thread *thread) {
PATCH v2 5/5 perf tests: Add map groups sharing with thread object test
In reply to Jiri OlsaThis test create 2 processes abstractions, with several threads and checks they properly share and maintain map groups info.
--- tools/perf/Makefile.perf | 1 + tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/tests.h | 1 + tools/perf/tests/thread-mg-share.c | 90 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+) create mode 100644 tools/perf/tests/thread-mg-share.c
tools/perf/Makefile.perf +1 -0
@@ -415,6 +415,7 @@ LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o endif endif LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o +LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
@@ -128,6 +128,10 @@ static struct test { .func = test__mmap_thread_lookup, }, { + .desc = "Test thread mg sharing", + .func = test__thread_mg_share, + }, + { .func = NULL, }, };
tools/perf/tests/tests.h +1 -0
@@ -42,6 +42,7 @@ int test__keep_tracking(void);int test__parse_no_sample_id_all (void );int test__dwarf_unwind (void );int test__mmap_thread_lookup (void ); +int test__thread_mg_share (void );#if defined(__x86_64__) || defined(__i386__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/thread-mg-share.c new file +90 -0
90 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Re: [PATCH 5/5] perf tests: Add map groups sharing with thread object test
2014-04-14 (월), 17:52 +0200, Jiri Olsa:
tools/perf/tests/thread-mg-share.c line 85
Quoted 3 lines
tools/perf/tests/thread-mg-share.c new file +3
-0+/* +* Cannot call machine__delete_threads(machine) now, +* because we've already realased all the threads.
s/realased/released/
Other than that, this series looks good to me.
Thanks,
Namhyung
Quoted 5 lines
tools/perf/tests/thread-mg-share.c new file +5
-0+*/ + +machines__exit (&machines); +return 0 ; +}
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tests: Add map groups sharing with thread object test
In reply to Jiri Olsaperf tests: Add map groups sharing with thread object test
Commit-ID: fabf01238289e9ae009499594fc54642f5802a24 Gitweb: http://git.kernel.org/tip/fabf01238289e9ae009499594fc54642f5802a24 Author: Jiri Olsa <[email protected]> AuthorDate: Mon, 17 Mar 2014 14:39:00 +0100 Committer: Jiri Olsa <[email protected]> CommitDate: Mon, 28 Apr 2014 13:43:40 +0200
perf tests: Add map groups sharing with thread object test
This test create 2 processes abstractions, with several threads and checks they properly share and maintain map groups info.
--- tools/perf/Makefile.perf | 1 + tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/tests.h | 1 + tools/perf/tests/thread-mg-share.c | 90 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+)
tools/perf/Makefile.perf +1 -0
@@ -417,6 +417,7 @@ LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o endif endif LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o +LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
@@ -132,6 +132,10 @@ static struct test { .func = test__mmap_thread_lookup, }, { + .desc = "Test thread mg sharing", + .func = test__thread_mg_share, + }, + { .func = NULL, }, };
tools/perf/tests/tests.h +1 -0
@@ -43,6 +43,7 @@ int test__parse_no_sample_id_all(void);int test__dwarf_unwind (void );int test__hists_filter (void );int test__mmap_thread_lookup (void ); +int test__thread_mg_share (void );#if defined(__x86_64__) || defined(__i386__)#ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/thread-mg-share.c new file +90 -0
90 lines not on this page · raw message