Re: [PATCH] perf util: Fix evlist->threads when working with 'perf stat --per-thread'

[Jin, Yao]

On 1/13/2018 3:53 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jan 12, 2018 at 09:44:53AM -0700, Mathieu Poirier escreveu:
> > After commit ("73c0ca1eee3d perf thread_map: Enumerate all threads
> > from /proc") any condition where target->per_thread is set will see all the
> > threads in a system added to evlist->threads.
> >
> > Since the 'record' utility also uses function thread_map__new_str(), any
> > attempt to use the --perf-thread option will also end up accounting for all
>
> --perf-thread? Do you mean --per-thread, ok, so 'record' has this, uses
> the opts->per_thread that then Jin reused for his recent patches... Jin,
> can you try to untangle this, probably you can't reuse that
> opts->per-thread thing...
>
> Without looking further, I think that could be a better avenue to fix
> this issue, without having to add a 'stat' specific method to the core
> classes.
>
> - Arnaldo

Hi,

Very sorry, I just see this mail today.

Please let me look at this issue first.

Thanks
Jin Yao

> > threads in a system, resulting in new kernel events being created for all
> > threads rather than just the thread of interest.
> >
> > This patch keeps the newly introduced functionality but make sure it
> > doesn't affect other utilities outside of 'stat'.
> >
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> > ---
> >   tools/perf/builtin-stat.c |  2 +-
> >   tools/perf/util/evlist.c  | 29 ++++++++++++++++++++++++-----
> >   tools/perf/util/evlist.h  |  2 ++
> >   3 files changed, 27 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 98bf9d32f222..82d16426b984 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2833,7 +2833,7 @@ int cmd_stat(int argc, const char **argv)
> >   	if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
> >   		target.per_thread = true;
> >   
> > -	if (perf_evlist__create_maps(evsel_list, &target) < 0) {
> > +	if (perf_evlist__create_stat_maps(evsel_list, &target) < 0) {
> >   		if (target__has_task(&target)) {
> >   			pr_err("Problems finding threads of monitor\n");
> >   			parse_options_usage(stat_usage, stat_options, "p", 1);
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index f0a5e09c4071..a4ae684f28de 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -1100,13 +1100,11 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
> >   	return perf_evlist__mmap_ex(evlist, pages, 0, false);
> >   }
> >   
> > -int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> > +static int _perf_evlist__create_maps(struct perf_evlist *evlist,
> > +				     struct target *target,
> > +				     struct thread_map *threads)
> >   {
> >   	struct cpu_map *cpus;
> > -	struct thread_map *threads;
> > -
> > -	threads = thread_map__new_str(target->pid, target->tid, target->uid,
> > -				      target->per_thread);
> >   
> >   	if (!threads)
> >   		return -1;
> > @@ -1130,6 +1128,27 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> >   	return -1;
> >   }
> >   
> > +int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
> > +{
> > +	struct thread_map *threads;
> > +
> > +	threads = thread_map__new_str(target->pid, target->tid, target->uid,
> > +				      false);
> > +
> > +	return _perf_evlist__create_maps(evlist, target, threads);
> > +}
> > +
> > +int perf_evlist__create_stat_maps(struct perf_evlist *evlist,
> > +				  struct target *target)
> > +{
> > +	struct thread_map *threads;
> > +
> > +	threads = thread_map__new_str(target->pid, target->tid, target->uid,
> > +				      target->per_thread);
> > +
> > +	return _perf_evlist__create_maps(evlist, target, threads);
> > +}
> > +
> >   void perf_evlist__set_maps(struct perf_evlist *evlist, struct cpu_map *cpus,
> >   			   struct thread_map *threads)
> >   {
> > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> > index 75160666d305..80c654990e19 100644
> > --- a/tools/perf/util/evlist.h
> > +++ b/tools/perf/util/evlist.h
> > @@ -188,6 +188,8 @@ void perf_evlist__set_selected(struct perf_evlist *evlist,
> >   void perf_evlist__set_maps(struct perf_evlist *evlist, struct cpu_map *cpus,
> >   			   struct thread_map *threads);
> >   int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target);
> > +int perf_evlist__create_stat_maps(struct perf_evlist *evlist,
> > +				  struct target *target);
> >   int perf_evlist__apply_filters(struct perf_evlist *evlist, struct perf_evsel **err_evsel);
> >   
> >   void __perf_evlist__set_leader(struct list_head *list);
> > --
> > 2.7.4