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

From: Mathieu Poirier
Date: Fri Jan 12 2018 - 15:45:37 EST


On 12 January 2018 at 12:53, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> 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...

Right, I meant --per-thread.

>
> 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.

Ok, let me have another look at builtin-stat.c...

>
> - 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