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

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

good idea, will do

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-18 Thread Jiri Olsa 
On Tue, Mar 18, 2014 at 04:03:41PM +0900, Namhyung Kim wrote:
 Hi Jiri,
 
 On Mon, 17 Mar 2014 15:52:59 +0100, Jiri Olsa wrote:
  On Mon, Mar 17, 2014 at 04:13:53PM +0900, Namhyung Kim wrote:
  On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
   Moving towards sharing map groups within a process threads.
  
   Because of this we need the map groups to be dynamically
   allocated. No other functional change is intended in here.
   @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
al-cpumode = cpumode;
al-filtered = false;

   -if (machine == NULL) {
   +if ((machine == NULL) || (mg == NULL)) {
al-map = NULL;
return;
}
  
  What about the kernel threads?  I guess they're not using thread-mg but
  machine-kmaps instead?
 
  The machine-kmaps is used to store kernel maps - core + modules.
 
  All threads (including kernel ones) are using thread-mg,
  kernel threads have empty /proc/x/maps file.
 
  In case the sample address is detected within kernel space,
  the machine-kmaps is used instead of thread-mg:
 
 That means kernel threads don't need to allocate -mg, right?  So I'd
 suggest checking cpumode first and then allocate -mg for userspace maps
 only.

good idea, will do

thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-17 Thread Jiri Olsa 
On Mon, Mar 17, 2014 at 04:13:53PM +0900, Namhyung Kim wrote:
> On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
> > Moving towards sharing map groups within a process threads.
> >
> > Because of this we need the map groups to be dynamically
> > allocated. No other functional change is intended in here.
> > @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
> > al->cpumode = cpumode;
> > al->filtered = false;
> >  
> > -   if (machine == NULL) {
> > +   if ((machine == NULL) || (mg == NULL)) {
> > al->map = NULL;
> > return;
> > }
> 
> What about the kernel threads?  I guess they're not using thread->mg but
> machine->kmaps instead?

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

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

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

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

  ...

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

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

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-17 Thread Namhyung Kim 
On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
> Moving towards sharing map groups within a process threads.
>
> Because of this we need the map groups to be dynamically
> allocated. No other functional change is intended in here.
> @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
>   al->cpumode = cpumode;
>   al->filtered = false;
>  
> - if (machine == NULL) {
> + if ((machine == NULL) || (mg == NULL)) {
>   al->map = NULL;
>   return;
>   }

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

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-17 Thread Namhyung Kim 
On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
 Moving towards sharing map groups within a process threads.

 Because of this we need the map groups to be dynamically
 allocated. No other functional change is intended in here.
 @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
   al-cpumode = cpumode;
   al-filtered = false;
  
 - if (machine == NULL) {
 + if ((machine == NULL) || (mg == NULL)) {
   al-map = NULL;
   return;
   }

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

Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-17 Thread Jiri Olsa 
On Mon, Mar 17, 2014 at 04:13:53PM +0900, Namhyung Kim wrote:
 On Fri, 14 Mar 2014 15:00:04 +0100, Jiri Olsa wrote:
  Moving towards sharing map groups within a process threads.
 
  Because of this we need the map groups to be dynamically
  allocated. No other functional change is intended in here.
  @@ -664,7 +664,7 @@ void thread__find_addr_map(struct thread *thread,
  al-cpumode = cpumode;
  al-filtered = false;
   
  -   if (machine == NULL) {
  +   if ((machine == NULL) || (mg == NULL)) {
  al-map = NULL;
  return;
  }
 
 What about the kernel threads?  I guess they're not using thread-mg but
 machine-kmaps instead?

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

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

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

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

  ...

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

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

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2014-03-14 Thread Arnaldo Carvalho de Melo 
Em Fri, Mar 14, 2014 at 03:00:04PM +0100, Jiri Olsa escreveu:
> Moving towards sharing map groups within a process threads.
> 
> Because of this we need the map groups to be dynamically
> allocated. No other functional change is intended in here.

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

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

- Arnaldo
 
> Signed-off-by: Jiri Olsa 
> Cc: Don Zickus 
> Cc: Corey Ashford 
> Cc: David Ahern 
> Cc: Frederic Weisbecker 
> Cc: Ingo Molnar 
> Cc: Namhyung Kim 
> Cc: Paul Mackerras 
> Cc: Peter Zijlstra 
> Cc: Arnaldo Carvalho de Melo 
> ---
>  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(>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(>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 = >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

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

2014-03-14 Thread Jiri Olsa 
Moving towards sharing map groups within a process threads.

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

Signed-off-by: Jiri Olsa 
Cc: Don Zickus 
Cc: Corey Ashford 
Cc: David Ahern 
Cc: Frederic Weisbecker 
Cc: Ingo Molnar 
Cc: Namhyung Kim 
Cc: Paul Mackerras 
Cc: Peter Zijlstra 
Cc: Arnaldo Carvalho de Melo 
---
 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(>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(>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 = >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(>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(>mg);
+   if (thread->mg)
+   map_groups__exit(thread->mg);
+
list_for_each_entry_safe(comm, tmp, >comm_list, list) {

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

2014-03-14 Thread Jiri Olsa 
Moving towards sharing map groups within a process threads.

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

Signed-off-by: Jiri Olsa jo...@redhat.com
Cc: Don Zickus dzic...@redhat.com
Cc: Corey Ashford cjash...@linux.vnet.ibm.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@kernel.org
Cc: Namhyung Kim namhy...@kernel.org
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Arnaldo Carvalho de Melo a...@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,

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

2014-03-14 Thread Arnaldo Carvalho de Melo 
Em Fri, Mar 14, 2014 at 03:00:04PM +0100, Jiri Olsa escreveu:
 Moving towards sharing map groups within a process threads.
 
 Because of this we need the map groups to be dynamically
 allocated. No other functional change is intended in here.

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

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

- Arnaldo
 
 Signed-off-by: Jiri Olsa jo...@redhat.com
 Cc: Don Zickus dzic...@redhat.com
 Cc: Corey Ashford cjash...@linux.vnet.ibm.com
 Cc: David Ahern dsah...@gmail.com
 Cc: Frederic Weisbecker fweis...@gmail.com
 Cc: Ingo Molnar mi...@kernel.org
 Cc: Namhyung Kim namhy...@kernel.org
 Cc: Paul Mackerras pau...@samba.org
 Cc: Peter Zijlstra a.p.zijls...@chello.nl
 Cc: Arnaldo Carvalho de Melo a...@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