From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>
Cc: Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
Clark Williams <williams@redhat.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Kim Phillips <kim.phillips@amd.com>,
Andi Kleen <ak@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>,
Song Liu <songliubraving@fb.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
Davidlohr Bueso <dave@stgolabs.net>,
Jin Yao <yao.jin@linux.intel.com>,
Kan Liang <kan.liang@linux.intel.com>,
Mark Rutland <mark.rutland@arm.com>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 01/23] perf stat: Don't report a null stalled cycles per insn metric
Date: Fri, 14 Feb 2020 16:10:35 -0300 [thread overview]
Message-ID: <20200214191057.26266-2-acme@kernel.org> (raw)
In-Reply-To: <20200214191057.26266-1-acme@kernel.org>
From: Kim Phillips <kim.phillips@amd.com>
For data collected on machines with front end stalled cycles supported,
such as found on modern AMD CPU families, commit 146540fb545b ("perf
stat: Always separate stalled cycles per insn") introduces a new line in
CSV output with a leading comma that upsets some automated scripts.
Scripts have to use "-e ex_ret_instr" to work around this issue, after
upgrading to a version of perf with that commit.
We could add "if (have_frontend_stalled && !config->csv_sep)" to the not
(total && avg) else clause, to emphasize that CSV users are usually
scripts, and are written to do only what is needed, i.e., they wouldn't
typically invoke "perf stat" without specifying an explicit event list.
But - let alone CSV output - why should users now tolerate a constant
0-reporting extra line in regular terminal output?:
BEFORE:
$ sudo perf stat --all-cpus -einstructions,cycles -- sleep 1
Performance counter stats for 'system wide':
181,110,981 instructions # 0.58 insn per cycle
# 0.00 stalled cycles per insn
309,876,469 cycles
1.002202582 seconds time elapsed
The user would not like to see the now permanent:
"0.00 stalled cycles per insn"
line fixture, as it gives no useful information.
So this patch removes the printing of the zeroed stalled cycles line
altogether, almost reverting the very original commit fb4605ba47e7
("perf stat: Check for frontend stalled for metrics"), which seems like
it was written to normalize --metric-only column output of common Intel
machines at the time: modern Intel machines have ceased to support the
genericised frontend stalled metrics AFAICT.
AFTER:
$ sudo perf stat --all-cpus -einstructions,cycles -- sleep 1
Performance counter stats for 'system wide':
244,071,432 instructions # 0.69 insn per cycle
355,353,490 cycles
1.001862516 seconds time elapsed
Output behaviour when stalled cycles is indeed measured is not affected
(BEFORE == AFTER):
$ sudo perf stat --all-cpus -einstructions,cycles,stalled-cycles-frontend -- sleep 1
Performance counter stats for 'system wide':
247,227,799 instructions # 0.63 insn per cycle
# 0.26 stalled cycles per insn
394,745,636 cycles
63,194,485 stalled-cycles-frontend # 16.01% frontend cycles idle
1.002079770 seconds time elapsed
Fixes: 146540fb545b ("perf stat: Always separate stalled cycles per insn")
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Song Liu <songliubraving@fb.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20200207230613.26709-1-kim.phillips@amd.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/stat-shadow.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 2c41d47f6f83..90d23cc3c8d4 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -18,7 +18,6 @@
* AGGR_NONE: Use matching CPU
* AGGR_THREAD: Not supported?
*/
-static bool have_frontend_stalled;
struct runtime_stat rt_stat;
struct stats walltime_nsecs_stats;
@@ -144,7 +143,6 @@ void runtime_stat__exit(struct runtime_stat *st)
void perf_stat__init_shadow_stats(void)
{
- have_frontend_stalled = pmu_have_event("cpu", "stalled-cycles-frontend");
runtime_stat__init(&rt_stat);
}
@@ -853,10 +851,6 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config,
print_metric(config, ctxp, NULL, "%7.2f ",
"stalled cycles per insn",
ratio);
- } else if (have_frontend_stalled) {
- out->new_line(config, ctxp);
- print_metric(config, ctxp, NULL, "%7.2f ",
- "stalled cycles per insn", 0);
}
} else if (perf_evsel__match(evsel, HARDWARE, HW_BRANCH_MISSES)) {
if (runtime_stat_n(st, STAT_BRANCHES, ctx, cpu) != 0)
--
2.21.1
next prev parent reply other threads:[~2020-02-14 19:10 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-14 19:10 [GIT PULL] perf/urgent improvements and fixes Arnaldo Carvalho de Melo
2020-02-14 19:10 ` Arnaldo Carvalho de Melo [this message]
2020-02-14 19:10 ` [PATCH 02/23] perf symbols: Update the list of kernel idle symbols Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 03/23] perf symbols: Convert symbol__is_idle() to use strlist Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 04/23] tools include UAPI: Sync x86's syscalls_64.tbl, generic unistd.h and fcntl.h to pick up openat2 and pidfd_getfd Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 05/23] perf maps: Mark module DSOs with kernel type Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 06/23] perf maps: Mark ksymbol " Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 07/23] perf maps: Fix map__clone() for struct kmap Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 08/23] perf maps: Move kmap::kmaps setup to maps__insert() Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 09/23] tools headers UAPI: Sync copy of arm64's asm/unistd.h with the kernel sources Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 10/23] tools headers UAPI: Sync prctl.h " Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 11/23] perf beauty prctl: Export the 'options' strarray Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 12/23] perf trace: Resolve prctl's 'option' arg strings to numbers Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 13/23] tools headers UAPI: Sync sched.h with the kernel Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 14/23] tools headers uapi: Sync linux/fscrypt.h with the kernel sources Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 15/23] tools headers UAPI: Sync drm/i915_drm.h " Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 16/23] perf tools: Add arm64 version of get_cpuid() Arnaldo Carvalho de Melo
2020-02-14 19:10 ` Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 17/23] tools headers UAPI: Sync asm-generic/mman-common.h with the kernel Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 18/23] tools include UAPI: Sync sound/asound.h copy Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 19/23] tools headers x86: Sync disabled-features.h Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 20/23] tools arch x86: Sync asm/cpufeatures.h with the kernel sources Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 21/23] tools headers kvm: Sync kvm headers " Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 22/23] tools headers kvm: Sync linux/kvm.h " Arnaldo Carvalho de Melo
2020-02-14 19:10 ` [PATCH 23/23] perf llvm: Fix script used to obtain kernel make directives to work with new kbuild Arnaldo Carvalho de Melo
2020-02-15 8:36 ` [GIT PULL] perf/urgent improvements and fixes Ingo Molnar
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=20200214191057.26266-2-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=dave@stgolabs.net \
--cc=jolsa@kernel.org \
--cc=jolsa@redhat.com \
--cc=kan.liang@linux.intel.com \
--cc=kim.phillips@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=tglx@linutronix.de \
--cc=williams@redhat.com \
--cc=xiyou.wangcong@gmail.com \
--cc=yao.jin@linux.intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.