From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751058AbeAVLnz (ORCPT ); Mon, 22 Jan 2018 06:43:55 -0500 Received: from mga01.intel.com ([192.55.52.88]:20925 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750848AbeAVLny (ORCPT ); Mon, 22 Jan 2018 06:43:54 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,396,1511856000"; d="scan'208";a="21592256" Subject: Re: [PATCH] perf util: Fix evlist->threads when working with 'perf stat --per-thread' To: Arnaldo Carvalho de Melo , Mathieu Poirier Cc: peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, jolsa@redhat.com, namhyung@kernel.org, linux-kernel@vger.kernel.org References: <1515775493-14254-1-git-send-email-mathieu.poirier@linaro.org> <20180112195326.GH9655@kernel.org> From: "Jin, Yao" Message-ID: <2672d362-a729-fe60-514f-6e7e423a2600@linux.intel.com> Date: Mon, 22 Jan 2018 19:43:51 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180112195326.GH9655@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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