Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Arnaldo Carvalho de Melo 
Em Mon, Jun 22, 2015 at 07:37:45PM +0200, Jiri Olsa escreveu:
> On Mon, Jun 22, 2015 at 11:40:16AM -0300, Arnaldo Carvalho de Melo wrote:
> > > get/set functions seem like overkill..  we could use upper case to state
> > > that it's a macro:

> > >   THREAD_MAP__PID(evlist->threads, 0) = getpid();

> > > anyway I can keep the original way if you insist ;-)

> > I really think that 

> > foo(a, b, c) = bla;

> > is ugly :-\

> > *foo(a, b, c) = bla;

> > Is uglier tho, bored right now to go beyond aesthetics tho :-\

> discussed on irc and decided to go with:

>   thread_map__pid(threads, i) // to get pid
>   thread_map__set_pid(threads, i, pid)// to set pid

>   thread_map__comm(threads, i)// to get comm
>   thread_map__set_comm(threads, i, comm)  // to set comm

Ack.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Jiri Olsa 
On Mon, Jun 22, 2015 at 11:40:16AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > > > +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> > > > @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
> > > >  
> > > > perf_evsel__config(evsel, );
> > > >  
> > > > -   evlist->threads->map[0] = getpid();
> > > > +   thread_map__pid(evlist->threads, 0) = getpid();
> > > 
> > > So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
> > > we keep using:
> > > 
> > >   evlist->thread->map[0].pid = getpid();
> > 
> > hum, I like it more than above line.. what's ugly about that assignment?
> > 
> > I'm adding thread_map__comm to access new 'comm' member,
> > so I wanted to introduce easy accessors for both members
> > 
> > get/set functions seem like overkill..  we could use upper case to state
> > that it's a macro:
> > 
> >   THREAD_MAP__PID(evlist->threads, 0) = getpid();
> > 
> > anyway I can keep the original way if you insist ;-)
> 
> I really think that 
> 
>   foo(a, b, c) = bla;
> 
> is ugly :-\
> 
>   *foo(a, b, c) = bla;
> 
> Is uglier tho, bored right now to go beyond aesthetics tho :-\

discussed on irc and decided to go with:

  thread_map__pid(threads, i)   // to get pid
  thread_map__set_pid(threads, i, pid)  // to set pid

  thread_map__comm(threads, i)  // to get comm
  thread_map__set_comm(threads, i, comm)// to set comm

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Arnaldo Carvalho de Melo 
Em Sat, Jun 20, 2015 at 12:05:31AM +0200, Jiri Olsa escreveu:
> On Fri, Jun 19, 2015 at 06:07:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
> > > We need to store command names with the pid. Changing
> > > map to be struct holding pid. Process name is coming
> > > in shortly.
> > > 
> > > Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
> > > Signed-off-by: Jiri Olsa 
> > > ---
> > >  tools/perf/builtin-trace.c  |  4 ++--
> > >  tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
> > >  tools/perf/util/auxtrace.c  |  4 ++--
> > >  tools/perf/util/event.c |  6 +++---
> > >  tools/perf/util/evlist.c|  4 ++--
> > >  tools/perf/util/evsel.c |  2 +-
> > >  tools/perf/util/thread_map.c| 22 +++---
> > >  tools/perf/util/thread_map.h|  8 +++-
> > >  8 files changed, 29 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > > index 4bf805b2fbf6..b75a68c385ea 100644
> > > --- a/tools/perf/builtin-trace.c
> > > +++ b/tools/perf/builtin-trace.c
> > > @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int 
> > > argc, const char **argv)
> > >*/
> > >   if (trace->filter_pids.nr > 0)
> > >   err = perf_evlist__set_filter_pids(evlist, 
> > > trace->filter_pids.nr, trace->filter_pids.entries);
> > > - else if (evlist->threads->map[0] == -1)
> > > + else if (thread_map__pid(evlist->threads, 0) == -1)
> > >   err = perf_evlist__set_filter_pid(evlist, getpid());
> > >  
> > >   if (err < 0) {
> > > @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int 
> > > argc, const char **argv)
> > >   if (forks)
> > >   perf_evlist__start_workload(evlist);
> > >  
> > > - trace->multiple_threads = evlist->threads->map[0] == -1 ||
> > > + trace->multiple_threads = thread_map__pid(evlist->threads, 0) == -1 ||
> > > evlist->threads->nr > 1 ||
> > > perf_evlist__first(evlist)->attr.inherit;
> > >  again:
> > > diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
> > > b/tools/perf/tests/openat-syscall-tp-fields.c
> > > index 6245221479d7..ebc6e7938c9a 100644
> > > --- a/tools/perf/tests/openat-syscall-tp-fields.c
> > > +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> > > @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
> > >  
> > >   perf_evsel__config(evsel, );
> > >  
> > > - evlist->threads->map[0] = getpid();
> > > + thread_map__pid(evlist->threads, 0) = getpid();
> > 
> > So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
> > we keep using:
> > 
> > evlist->thread->map[0].pid = getpid();
> 
> hum, I like it more than above line.. what's ugly about that assignment?
> 
> I'm adding thread_map__comm to access new 'comm' member,
> so I wanted to introduce easy accessors for both members
> 
> get/set functions seem like overkill..  we could use upper case to state
> that it's a macro:
> 
>   THREAD_MAP__PID(evlist->threads, 0) = getpid();
> 
> anyway I can keep the original way if you insist ;-)

I really think that 

foo(a, b, c) = bla;

is ugly :-\

*foo(a, b, c) = bla;

Is uglier tho, bored right now to go beyond aesthetics tho :-\

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Jiri Olsa 
On Mon, Jun 22, 2015 at 11:40:16AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

+++ b/tools/perf/tests/openat-syscall-tp-fields.c
@@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
 
perf_evsel__config(evsel, opts);
 
-   evlist-threads-map[0] = getpid();
+   thread_map__pid(evlist-threads, 0) = getpid();
   
   So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
   we keep using:
   
 evlist-thread-map[0].pid = getpid();
  
  hum, I like it more than above line.. what's ugly about that assignment?
  
  I'm adding thread_map__comm to access new 'comm' member,
  so I wanted to introduce easy accessors for both members
  
  get/set functions seem like overkill..  we could use upper case to state
  that it's a macro:
  
THREAD_MAP__PID(evlist-threads, 0) = getpid();
  
  anyway I can keep the original way if you insist ;-)
 
 I really think that 
 
   foo(a, b, c) = bla;
 
 is ugly :-\
 
   *foo(a, b, c) = bla;
 
 Is uglier tho, bored right now to go beyond aesthetics tho :-\

discussed on irc and decided to go with:

  thread_map__pid(threads, i)   // to get pid
  thread_map__set_pid(threads, i, pid)  // to set pid

  thread_map__comm(threads, i)  // to get comm
  thread_map__set_comm(threads, i, comm)// to set comm

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Arnaldo Carvalho de Melo 
Em Mon, Jun 22, 2015 at 07:37:45PM +0200, Jiri Olsa escreveu:
 On Mon, Jun 22, 2015 at 11:40:16AM -0300, Arnaldo Carvalho de Melo wrote:
   get/set functions seem like overkill..  we could use upper case to state
   that it's a macro:

 THREAD_MAP__PID(evlist-threads, 0) = getpid();

   anyway I can keep the original way if you insist ;-)

  I really think that 

  foo(a, b, c) = bla;

  is ugly :-\

  *foo(a, b, c) = bla;

  Is uglier tho, bored right now to go beyond aesthetics tho :-\

 discussed on irc and decided to go with:

   thread_map__pid(threads, i) // to get pid
   thread_map__set_pid(threads, i, pid)// to set pid

   thread_map__comm(threads, i)// to get comm
   thread_map__set_comm(threads, i, comm)  // to set comm

Ack.

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-22 Thread Arnaldo Carvalho de Melo 
Em Sat, Jun 20, 2015 at 12:05:31AM +0200, Jiri Olsa escreveu:
 On Fri, Jun 19, 2015 at 06:07:13PM -0300, Arnaldo Carvalho de Melo wrote:
  Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
   We need to store command names with the pid. Changing
   map to be struct holding pid. Process name is coming
   in shortly.
   
   Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
   Signed-off-by: Jiri Olsa jo...@kernel.org
   ---
tools/perf/builtin-trace.c  |  4 ++--
tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
tools/perf/util/auxtrace.c  |  4 ++--
tools/perf/util/event.c |  6 +++---
tools/perf/util/evlist.c|  4 ++--
tools/perf/util/evsel.c |  2 +-
tools/perf/util/thread_map.c| 22 +++---
tools/perf/util/thread_map.h|  8 +++-
8 files changed, 29 insertions(+), 23 deletions(-)
   
   diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
   index 4bf805b2fbf6..b75a68c385ea 100644
   --- a/tools/perf/builtin-trace.c
   +++ b/tools/perf/builtin-trace.c
   @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int 
   argc, const char **argv)
  */
 if (trace-filter_pids.nr  0)
 err = perf_evlist__set_filter_pids(evlist, 
   trace-filter_pids.nr, trace-filter_pids.entries);
   - else if (evlist-threads-map[0] == -1)
   + else if (thread_map__pid(evlist-threads, 0) == -1)
 err = perf_evlist__set_filter_pid(evlist, getpid());

 if (err  0) {
   @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int 
   argc, const char **argv)
 if (forks)
 perf_evlist__start_workload(evlist);

   - trace-multiple_threads = evlist-threads-map[0] == -1 ||
   + trace-multiple_threads = thread_map__pid(evlist-threads, 0) == -1 ||
   evlist-threads-nr  1 ||
   perf_evlist__first(evlist)-attr.inherit;
again:
   diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
   b/tools/perf/tests/openat-syscall-tp-fields.c
   index 6245221479d7..ebc6e7938c9a 100644
   --- a/tools/perf/tests/openat-syscall-tp-fields.c
   +++ b/tools/perf/tests/openat-syscall-tp-fields.c
   @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)

 perf_evsel__config(evsel, opts);

   - evlist-threads-map[0] = getpid();
   + thread_map__pid(evlist-threads, 0) = getpid();
  
  So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
  we keep using:
  
  evlist-thread-map[0].pid = getpid();
 
 hum, I like it more than above line.. what's ugly about that assignment?
 
 I'm adding thread_map__comm to access new 'comm' member,
 so I wanted to introduce easy accessors for both members
 
 get/set functions seem like overkill..  we could use upper case to state
 that it's a macro:
 
   THREAD_MAP__PID(evlist-threads, 0) = getpid();
 
 anyway I can keep the original way if you insist ;-)

I really think that 

foo(a, b, c) = bla;

is ugly :-\

*foo(a, b, c) = bla;

Is uglier tho, bored right now to go beyond aesthetics tho :-\

- Arnaldo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-19 Thread Jiri Olsa 
On Fri, Jun 19, 2015 at 06:07:13PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
> > We need to store command names with the pid. Changing
> > map to be struct holding pid. Process name is coming
> > in shortly.
> > 
> > Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
> > Signed-off-by: Jiri Olsa 
> > ---
> >  tools/perf/builtin-trace.c  |  4 ++--
> >  tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
> >  tools/perf/util/auxtrace.c  |  4 ++--
> >  tools/perf/util/event.c |  6 +++---
> >  tools/perf/util/evlist.c|  4 ++--
> >  tools/perf/util/evsel.c |  2 +-
> >  tools/perf/util/thread_map.c| 22 +++---
> >  tools/perf/util/thread_map.h|  8 +++-
> >  8 files changed, 29 insertions(+), 23 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 4bf805b2fbf6..b75a68c385ea 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int argc, 
> > const char **argv)
> >  */
> > if (trace->filter_pids.nr > 0)
> > err = perf_evlist__set_filter_pids(evlist, 
> > trace->filter_pids.nr, trace->filter_pids.entries);
> > -   else if (evlist->threads->map[0] == -1)
> > +   else if (thread_map__pid(evlist->threads, 0) == -1)
> > err = perf_evlist__set_filter_pid(evlist, getpid());
> >  
> > if (err < 0) {
> > @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int argc, 
> > const char **argv)
> > if (forks)
> > perf_evlist__start_workload(evlist);
> >  
> > -   trace->multiple_threads = evlist->threads->map[0] == -1 ||
> > +   trace->multiple_threads = thread_map__pid(evlist->threads, 0) == -1 ||
> >   evlist->threads->nr > 1 ||
> >   perf_evlist__first(evlist)->attr.inherit;
> >  again:
> > diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
> > b/tools/perf/tests/openat-syscall-tp-fields.c
> > index 6245221479d7..ebc6e7938c9a 100644
> > --- a/tools/perf/tests/openat-syscall-tp-fields.c
> > +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> > @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
> >  
> > perf_evsel__config(evsel, );
> >  
> > -   evlist->threads->map[0] = getpid();
> > +   thread_map__pid(evlist->threads, 0) = getpid();
> 
> So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
> we keep using:
> 
>   evlist->thread->map[0].pid = getpid();

hum, I like it more than above line.. what's ugly about that assignment?

I'm adding thread_map__comm to access new 'comm' member,
so I wanted to introduce easy accessors for both members

get/set functions seem like overkill..  we could use upper case to state
that it's a macro:

  THREAD_MAP__PID(evlist->threads, 0) = getpid();

anyway I can keep the original way if you insist ;-)

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-19 Thread Arnaldo Carvalho de Melo 
Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
> We need to store command names with the pid. Changing
> map to be struct holding pid. Process name is coming
> in shortly.
> 
> Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/builtin-trace.c  |  4 ++--
>  tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
>  tools/perf/util/auxtrace.c  |  4 ++--
>  tools/perf/util/event.c |  6 +++---
>  tools/perf/util/evlist.c|  4 ++--
>  tools/perf/util/evsel.c |  2 +-
>  tools/perf/util/thread_map.c| 22 +++---
>  tools/perf/util/thread_map.h|  8 +++-
>  8 files changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 4bf805b2fbf6..b75a68c385ea 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int argc, 
> const char **argv)
>*/
>   if (trace->filter_pids.nr > 0)
>   err = perf_evlist__set_filter_pids(evlist, 
> trace->filter_pids.nr, trace->filter_pids.entries);
> - else if (evlist->threads->map[0] == -1)
> + else if (thread_map__pid(evlist->threads, 0) == -1)
>   err = perf_evlist__set_filter_pid(evlist, getpid());
>  
>   if (err < 0) {
> @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int argc, 
> const char **argv)
>   if (forks)
>   perf_evlist__start_workload(evlist);
>  
> - trace->multiple_threads = evlist->threads->map[0] == -1 ||
> + trace->multiple_threads = thread_map__pid(evlist->threads, 0) == -1 ||
> evlist->threads->nr > 1 ||
> perf_evlist__first(evlist)->attr.inherit;
>  again:
> diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
> b/tools/perf/tests/openat-syscall-tp-fields.c
> index 6245221479d7..ebc6e7938c9a 100644
> --- a/tools/perf/tests/openat-syscall-tp-fields.c
> +++ b/tools/perf/tests/openat-syscall-tp-fields.c
> @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
>  
>   perf_evsel__config(evsel, );
>  
> - evlist->threads->map[0] = getpid();
> + thread_map__pid(evlist->threads, 0) = getpid();

So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
we keep using:

evlist->thread->map[0].pid = getpid();

I.e. just add the .pid to the existing usage.

- Arnaldo

>  
>   err = perf_evlist__open(evlist);
>   if (err < 0) {
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index df66966cfde7..3dab006b4a03 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -119,12 +119,12 @@ void auxtrace_mmap_params__set_idx(struct 
> auxtrace_mmap_params *mp,
>   if (per_cpu) {
>   mp->cpu = evlist->cpus->map[idx];
>   if (evlist->threads)
> - mp->tid = evlist->threads->map[0];
> + mp->tid = thread_map__pid(evlist->threads, 0);
>   else
>   mp->tid = -1;
>   } else {
>   mp->cpu = -1;
> - mp->tid = evlist->threads->map[idx];
> + mp->tid = thread_map__pid(evlist->threads, idx);
>   }
>  }
>  
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 793b1503d437..51a1bedf90fb 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -479,7 +479,7 @@ int perf_event__synthesize_thread_map(struct perf_tool 
> *tool,
>   for (thread = 0; thread < threads->nr; ++thread) {
>   if (__event__synthesize_thread(comm_event, mmap_event,
>  fork_event,
> -threads->map[thread], 0,
> +thread_map__pid(threads, 
> thread), 0,
>  process, tool, machine,
>  mmap_data)) {
>   err = -1;
> @@ -490,12 +490,12 @@ int perf_event__synthesize_thread_map(struct perf_tool 
> *tool,
>* comm.pid is set to thread group id by
>* perf_event__synthesize_comm
>*/
> - if ((int) comm_event->comm.pid != threads->map[thread]) {
> + if ((int) comm_event->comm.pid != thread_map__pid(threads, 
> thread)) {
>   bool need_leader = true;
>  
>   /* is thread group leader in thread_map? */
>   for (j = 0; j < threads->nr; ++j) {
> - if ((int) comm_event->comm.pid == 
> threads->map[j]) {
> + if ((int) comm_event->comm.pid == 
> thread_map__pid(threads, j)) {
>

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-19 Thread Arnaldo Carvalho de Melo 
Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
 We need to store command names with the pid. Changing
 map to be struct holding pid. Process name is coming
 in shortly.
 
 Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
 Signed-off-by: Jiri Olsa jo...@kernel.org
 ---
  tools/perf/builtin-trace.c  |  4 ++--
  tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
  tools/perf/util/auxtrace.c  |  4 ++--
  tools/perf/util/event.c |  6 +++---
  tools/perf/util/evlist.c|  4 ++--
  tools/perf/util/evsel.c |  2 +-
  tools/perf/util/thread_map.c| 22 +++---
  tools/perf/util/thread_map.h|  8 +++-
  8 files changed, 29 insertions(+), 23 deletions(-)
 
 diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
 index 4bf805b2fbf6..b75a68c385ea 100644
 --- a/tools/perf/builtin-trace.c
 +++ b/tools/perf/builtin-trace.c
 @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int argc, 
 const char **argv)
*/
   if (trace-filter_pids.nr  0)
   err = perf_evlist__set_filter_pids(evlist, 
 trace-filter_pids.nr, trace-filter_pids.entries);
 - else if (evlist-threads-map[0] == -1)
 + else if (thread_map__pid(evlist-threads, 0) == -1)
   err = perf_evlist__set_filter_pid(evlist, getpid());
  
   if (err  0) {
 @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int argc, 
 const char **argv)
   if (forks)
   perf_evlist__start_workload(evlist);
  
 - trace-multiple_threads = evlist-threads-map[0] == -1 ||
 + trace-multiple_threads = thread_map__pid(evlist-threads, 0) == -1 ||
 evlist-threads-nr  1 ||
 perf_evlist__first(evlist)-attr.inherit;
  again:
 diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
 b/tools/perf/tests/openat-syscall-tp-fields.c
 index 6245221479d7..ebc6e7938c9a 100644
 --- a/tools/perf/tests/openat-syscall-tp-fields.c
 +++ b/tools/perf/tests/openat-syscall-tp-fields.c
 @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
  
   perf_evsel__config(evsel, opts);
  
 - evlist-threads-map[0] = getpid();
 + thread_map__pid(evlist-threads, 0) = getpid();

So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
we keep using:

evlist-thread-map[0].pid = getpid();

I.e. just add the .pid to the existing usage.

- Arnaldo

  
   err = perf_evlist__open(evlist);
   if (err  0) {
 diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
 index df66966cfde7..3dab006b4a03 100644
 --- a/tools/perf/util/auxtrace.c
 +++ b/tools/perf/util/auxtrace.c
 @@ -119,12 +119,12 @@ void auxtrace_mmap_params__set_idx(struct 
 auxtrace_mmap_params *mp,
   if (per_cpu) {
   mp-cpu = evlist-cpus-map[idx];
   if (evlist-threads)
 - mp-tid = evlist-threads-map[0];
 + mp-tid = thread_map__pid(evlist-threads, 0);
   else
   mp-tid = -1;
   } else {
   mp-cpu = -1;
 - mp-tid = evlist-threads-map[idx];
 + mp-tid = thread_map__pid(evlist-threads, idx);
   }
  }
  
 diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
 index 793b1503d437..51a1bedf90fb 100644
 --- a/tools/perf/util/event.c
 +++ b/tools/perf/util/event.c
 @@ -479,7 +479,7 @@ int perf_event__synthesize_thread_map(struct perf_tool 
 *tool,
   for (thread = 0; thread  threads-nr; ++thread) {
   if (__event__synthesize_thread(comm_event, mmap_event,
  fork_event,
 -threads-map[thread], 0,
 +thread_map__pid(threads, 
 thread), 0,
  process, tool, machine,
  mmap_data)) {
   err = -1;
 @@ -490,12 +490,12 @@ int perf_event__synthesize_thread_map(struct perf_tool 
 *tool,
* comm.pid is set to thread group id by
* perf_event__synthesize_comm
*/
 - if ((int) comm_event-comm.pid != threads-map[thread]) {
 + if ((int) comm_event-comm.pid != thread_map__pid(threads, 
 thread)) {
   bool need_leader = true;
  
   /* is thread group leader in thread_map? */
   for (j = 0; j  threads-nr; ++j) {
 - if ((int) comm_event-comm.pid == 
 threads-map[j]) {
 + if ((int) comm_event-comm.pid == 
 thread_map__pid(threads, j)) {
   need_leader = false;
   break;
   }
 diff --git a/tools/perf/util/evlist.c

Re: [PATCH 01/26] perf tools: Change thread_map::map into struct

2015-06-19 Thread Jiri Olsa 
On Fri, Jun 19, 2015 at 06:07:13PM -0300, Arnaldo Carvalho de Melo wrote:
 Em Thu, Jun 18, 2015 at 11:48:40PM +0200, Jiri Olsa escreveu:
  We need to store command names with the pid. Changing
  map to be struct holding pid. Process name is coming
  in shortly.
  
  Link: http://lkml.kernel.org/n/tip-z4zuyvcxa6glzqm8qubk6...@git.kernel.org
  Signed-off-by: Jiri Olsa jo...@kernel.org
  ---
   tools/perf/builtin-trace.c  |  4 ++--
   tools/perf/tests/openat-syscall-tp-fields.c |  2 +-
   tools/perf/util/auxtrace.c  |  4 ++--
   tools/perf/util/event.c |  6 +++---
   tools/perf/util/evlist.c|  4 ++--
   tools/perf/util/evsel.c |  2 +-
   tools/perf/util/thread_map.c| 22 +++---
   tools/perf/util/thread_map.h|  8 +++-
   8 files changed, 29 insertions(+), 23 deletions(-)
  
  diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
  index 4bf805b2fbf6..b75a68c385ea 100644
  --- a/tools/perf/builtin-trace.c
  +++ b/tools/perf/builtin-trace.c
  @@ -2324,7 +2324,7 @@ static int trace__run(struct trace *trace, int argc, 
  const char **argv)
   */
  if (trace-filter_pids.nr  0)
  err = perf_evlist__set_filter_pids(evlist, 
  trace-filter_pids.nr, trace-filter_pids.entries);
  -   else if (evlist-threads-map[0] == -1)
  +   else if (thread_map__pid(evlist-threads, 0) == -1)
  err = perf_evlist__set_filter_pid(evlist, getpid());
   
  if (err  0) {
  @@ -2342,7 +2342,7 @@ static int trace__run(struct trace *trace, int argc, 
  const char **argv)
  if (forks)
  perf_evlist__start_workload(evlist);
   
  -   trace-multiple_threads = evlist-threads-map[0] == -1 ||
  +   trace-multiple_threads = thread_map__pid(evlist-threads, 0) == -1 ||
evlist-threads-nr  1 ||
perf_evlist__first(evlist)-attr.inherit;
   again:
  diff --git a/tools/perf/tests/openat-syscall-tp-fields.c 
  b/tools/perf/tests/openat-syscall-tp-fields.c
  index 6245221479d7..ebc6e7938c9a 100644
  --- a/tools/perf/tests/openat-syscall-tp-fields.c
  +++ b/tools/perf/tests/openat-syscall-tp-fields.c
  @@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)
   
  perf_evsel__config(evsel, opts);
   
  -   evlist-threads-map[0] = getpid();
  +   thread_map__pid(evlist-threads, 0) = getpid();
 
 So this 'function(parms) = something' idiom looks ugly/unfamiliar, can't
 we keep using:
 
   evlist-thread-map[0].pid = getpid();

hum, I like it more than above line.. what's ugly about that assignment?

I'm adding thread_map__comm to access new 'comm' member,
so I wanted to introduce easy accessors for both members

get/set functions seem like overkill..  we could use upper case to state
that it's a macro:

  THREAD_MAP__PID(evlist-threads, 0) = getpid();

anyway I can keep the original way if you insist ;-)

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
Please read the FAQ at  http://www.tux.org/lkml/