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

[Arnaldo Carvalho de Melo]

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

> 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