From: "Liang, Kan" <kan.liang@linux.intel.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Jiri Olsa <jolsa@redhat.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
Stephane Eranian <eranian@google.com>,
Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH V2 3/4] perf stat: Support new per thread TopDown metrics
Date: Fri, 11 Sep 2020 10:12:07 -0400 [thread overview]
Message-ID: <b7d8dbb6-be8f-9c15-256b-fc57936aaa1e@linux.intel.com> (raw)
In-Reply-To: <CAM9d7ci1p1Ej-9=RuJLHJWQ76GR6gjHS2Y=rsQQ0LhNW5YKUBg@mail.gmail.com>
On 9/10/2020 11:37 PM, Namhyung Kim wrote:
> Hello,
>
> On Thu, Sep 10, 2020 at 10:48 PM <kan.liang@linux.intel.com> wrote:
>>
>> From: Andi Kleen <ak@linux.intel.com>
>>
>> Icelake has support for reporting per thread TopDown metrics.
>> These are reported differently than the previous TopDown support,
>> each metric is standalone, but scaled to pipeline "slots".
>> We don't need to do anything special for HyperThreading anymore.
>> Teach perf stat --topdown to handle these new metrics and
>> print them in the same way as the previous TopDown metrics.
>> The restrictions of only being able to report information per core is
>> gone.
>>
>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>> Co-developed-by: Kan Liang <kan.liang@linux.intel.com>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>> ---
>> tools/perf/Documentation/perf-stat.txt | 7 +-
>> tools/perf/builtin-stat.c | 30 ++++++++-
>> tools/perf/util/stat-shadow.c | 89 ++++++++++++++++++++++++++
>> tools/perf/util/stat.c | 4 ++
>> tools/perf/util/stat.h | 8 +++
>> 5 files changed, 134 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
>> index c9bfefc051fb..e803dbdc88a8 100644
>> --- a/tools/perf/Documentation/perf-stat.txt
>> +++ b/tools/perf/Documentation/perf-stat.txt
>> @@ -357,6 +357,11 @@ if the workload is actually bound by the CPU and not by something else.
>> For best results it is usually a good idea to use it with interval
>> mode like -I 1000, as the bottleneck of workloads can change often.
>>
>> +This enables --metric-only, unless overridden with --no-metric-only.
>> +
>> +The following restrictions only apply to older Intel CPUs and Atom,
>> +on newer CPUs (IceLake and later) TopDown can be collected for any thread:
>> +
>> The top down metrics are collected per core instead of per
>> CPU thread. Per core mode is automatically enabled
>> and -a (global monitoring) is needed, requiring root rights or
>> @@ -368,8 +373,6 @@ echo 0 > /proc/sys/kernel/nmi_watchdog
>> for best results. Otherwise the bottlenecks may be inconsistent
>> on workload with changing phases.
>>
>> -This enables --metric-only, unless overridden with --no-metric-only.
>> -
>> To interpret the results it is usually needed to know on which
>> CPUs the workload runs on. If needed the CPUs can be forced using
>> taskset.
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 5583e22ca808..6290da5bd142 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -128,6 +128,15 @@ static const char * topdown_attrs[] = {
>> NULL,
>> };
>>
>> +static const char *topdown_metric_attrs[] = {
>> + "slots",
>> + "topdown-retiring",
>> + "topdown-bad-spec",
>> + "topdown-fe-bound",
>> + "topdown-be-bound",
>> + NULL,
>> +};
>> +
>> static const char *smi_cost_attrs = {
>> "{"
>> "msr/aperf/,"
>> @@ -1691,6 +1700,24 @@ static int add_default_attributes(void)
>> char *str = NULL;
>> bool warn = false;
>>
>> + if (!force_metric_only)
>> + stat_config.metric_only = true;
>> +
>> + if (topdown_filter_events(topdown_metric_attrs, &str, 1) < 0) {
>> + pr_err("Out of memory\n");
>> + return -1;
>> + }
>> + if (topdown_metric_attrs[0] && str) {
>> + if (!stat_config.interval && !stat_config.metric_only) {
>> + fprintf(stat_config.output,
>> + "Topdown accuracy may decrease when measuring long periods.\n"
>> + "Please print the result regularly, e.g. -I1000\n");
>> + }
>> + goto setup_metrics;
>> + }
>> +
>> + str = NULL;
>
> zfree(&str) ?
Yes, even the topdown events don't exist, str is still allocated.
The str should be free.
I will send V3 to fix it shortly.
Thanks,
Kan
>
> Thanks
> Namhyung
>
>
>> +
>> if (stat_config.aggr_mode != AGGR_GLOBAL &&
>> stat_config.aggr_mode != AGGR_CORE) {
>> pr_err("top down event configuration requires --per-core mode\n");
>> @@ -1702,8 +1729,6 @@ static int add_default_attributes(void)
>> return -1;
>> }
>>
>> - if (!force_metric_only)
>> - stat_config.metric_only = true;
>> if (topdown_filter_events(topdown_attrs, &str,
>> arch_topdown_check_group(&warn)) < 0) {
>> pr_err("Out of memory\n");
>> @@ -1712,6 +1737,7 @@ static int add_default_attributes(void)
>> if (topdown_attrs[0] && str) {
>> if (warn)
>> arch_topdown_group_warn();
>> +setup_metrics:
>> err = parse_events(evsel_list, str, &errinfo);
>> if (err) {
>> fprintf(stderr,
prev parent reply other threads:[~2020-09-11 15:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200910134501.11352-1-kan.liang@linux.intel.com>
2020-09-10 13:44 ` [PATCH V2 1/4] perf tools: Rename group to topdown kan.liang
2020-09-10 13:44 ` [PATCH V2 2/4] perf record: Support sample-read topdown metric group kan.liang
2020-09-10 13:45 ` [PATCH V2 3/4] perf stat: Support new per thread TopDown metrics kan.liang
2020-09-11 3:37 ` Namhyung Kim
2020-09-11 14:12 ` Liang, Kan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b7d8dbb6-be8f-9c15-256b-fc57936aaa1e@linux.intel.com \
--to=kan.liang@linux.intel.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=eranian@google.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).