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

["Jin, Yao" <yao.jin@linux.intel.com>]

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@linaro.org>
>> ---
>>   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