* [PATCH 0/2] Toying with the perf tool
@ 2013-09-30 11:13 Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 1/2] perf tool: simplify ARCH code in Makefile Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 2/2] perf tool: don't print bogus data on -e cycles Ramkumar Ramachandra
0 siblings, 2 replies; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-30 11:13 UTC (permalink / raw)
To: LKML; +Cc: Ingo Molnar, Arnaldo Carvalho de Melo
Hi,
I was curiously poking around the perf tool this morning. The hacking
session resulted in two minor patches:
[1/2] is a minor non-functional cleanup of the Makefile.
[2/2] is more important: it omits printing bogus data in an edge case.
Thanks for reading.
Ramkumar Ramachandra (2):
perf tool: simplify ARCH code in Makefile
perf tool: don't print bogus data on -e cycles
tools/perf/builtin-stat.c | 9 ++++-----
tools/perf/config/Makefile | 47 ++++++++++++++++++++++------------------------
2 files changed, 26 insertions(+), 30 deletions(-)
--
1.8.4.477.g5d89aa9
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] perf tool: simplify ARCH code in Makefile
2013-09-30 11:13 [PATCH 0/2] Toying with the perf tool Ramkumar Ramachandra
@ 2013-09-30 11:13 ` Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 2/2] perf tool: don't print bogus data on -e cycles Ramkumar Ramachandra
1 sibling, 0 replies; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-30 11:13 UTC (permalink / raw)
To: LKML
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Jiri Olsa, Michael Witten,
Ingo Molnar, Namhyung Kim
The ARCH is first determined from `uname -m`, and then overridden to x86
anyway. This is unnecessarily ugly; follow the example set by
ffee0de (x86: Default to ARCH=x86 to avoid overriding CONFIG_64BIT,
2012-12-20) to simplify the code.
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Michael Witten <mfwitten@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
tools/perf/config/Makefile | 47 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 25 deletions(-)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 5f6f9b3..45a8515 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -1,33 +1,30 @@
-uname_M := $(shell uname -m 2>/dev/null || echo not)
-
-ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \
- -e s/arm.*/arm/ -e s/sa110/arm/ \
- -e s/s390x/s390/ -e s/parisc64/parisc/ \
- -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
- -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ )
+ARCH ?= $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
+ -e s/sun4u/sparc64/ \
+ -e s/arm.*/arm/ -e s/sa110/arm/ \
+ -e s/s390x/s390/ -e s/parisc64/parisc/ \
+ -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
+ -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ )
NO_PERF_REGS := 1
CFLAGS := $(EXTRA_CFLAGS) $(EXTRA_WARNINGS)
# Additional ARCH settings for x86
-ifeq ($(ARCH),i386)
- override ARCH := x86
- NO_PERF_REGS := 0
- LIBUNWIND_LIBS = -lunwind -lunwind-x86
-endif
-
-ifeq ($(ARCH),x86_64)
- override ARCH := x86
- IS_X86_64 := 0
- ifeq (, $(findstring m32,$(CFLAGS)))
- IS_X86_64 := $(shell echo __x86_64__ | ${CC} -E -x c - | tail -n 1)
- endif
- ifeq (${IS_X86_64}, 1)
- RAW_ARCH := x86_64
- CFLAGS += -DARCH_X86_64
- ARCH_INCLUDE = ../../arch/x86/lib/memcpy_64.S ../../arch/x86/lib/memset_64.S
+ifeq ($(ARCH),x86)
+ ifeq ($(shell uname -m),x86_64)
+ IS_X86_64 := 0
+ ifeq (, $(findstring m32,$(CFLAGS)))
+ IS_X86_64 := $(shell echo __x86_64__ | ${CC} -E -x c - | tail -n 1)
+ endif
+ ifeq (${IS_X86_64}, 1)
+ RAW_ARCH := x86_64
+ CFLAGS += -DARCH_X86_64
+ ARCH_INCLUDE = ../../arch/x86/lib/memcpy_64.S ../../arch/x86/lib/memset_64.S
+ endif
+ NO_PERF_REGS := 0
+ LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
+ else
+ NO_PERF_REGS := 0
+ LIBUNWIND_LIBS = -lunwind -lunwind-x86
endif
- NO_PERF_REGS := 0
- LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
endif
ifeq ($(NO_PERF_REGS),0)
--
1.8.4.477.g5d89aa9
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] perf tool: don't print bogus data on -e cycles
2013-09-30 11:13 [PATCH 0/2] Toying with the perf tool Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 1/2] perf tool: simplify ARCH code in Makefile Ramkumar Ramachandra
@ 2013-09-30 11:13 ` Ramkumar Ramachandra
2013-10-01 8:48 ` Ingo Molnar
2013-10-15 5:29 ` [tip:perf/core] perf stat: Don't " tip-bot for Ramkumar Ramachandra
1 sibling, 2 replies; 5+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-30 11:13 UTC (permalink / raw)
To: LKML
Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras
When only the cycles event is requested:
$ perf stat -e cycles dd if=/dev/zero of=/dev/null count=1000000
1000000+0 records in
1000000+0 records out
512000000 bytes (512 MB) copied, 0.26123 s, 2.0 GB/s
Performance counter stats for 'dd if=/dev/zero of=/dev/null count=1000000':
911,626,453 cycles # 0.000 GHz
0.262113350 seconds time elapsed
The 0.000 GHz comment in the output is totally bogus and misleading. It
happens because update_shadow_stats() doesn't touch runtime_nsecs_stats;
it is only written when a requested counter matches a SW_TASK_CLOCK. In
our case, since we have only requested HW_CPU_CYCLES,
runtime_nsecs_stats is unavailable. So, omit printing the comment
altogether.
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
tools/perf/builtin-stat.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f686d5f..cc167ae 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -918,11 +918,10 @@ static void abs_printout(int cpu, int nr, struct perf_evsel *evsel, double avg)
print_stalled_cycles_backend(cpu, evsel, avg);
} else if (perf_evsel__match(evsel, HARDWARE, HW_CPU_CYCLES)) {
total = avg_stats(&runtime_nsecs_stats[cpu]);
-
- if (total)
- ratio = 1.0 * avg / total;
-
- fprintf(output, " # %8.3f GHz ", ratio);
+ if (total) {
+ ratio = avg / total;
+ fprintf(output, " # %8.3f GHz ", ratio);
+ }
} else if (runtime_nsecs_stats[cpu].n != 0) {
char unit = 'M';
--
1.8.4.477.g5d89aa9
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] perf tool: don't print bogus data on -e cycles
2013-09-30 11:13 ` [PATCH 2/2] perf tool: don't print bogus data on -e cycles Ramkumar Ramachandra
@ 2013-10-01 8:48 ` Ingo Molnar
2013-10-15 5:29 ` [tip:perf/core] perf stat: Don't " tip-bot for Ramkumar Ramachandra
1 sibling, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2013-10-01 8:48 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: LKML, Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras
* Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> When only the cycles event is requested:
>
> $ perf stat -e cycles dd if=/dev/zero of=/dev/null count=1000000
> 1000000+0 records in
> 1000000+0 records out
> 512000000 bytes (512 MB) copied, 0.26123 s, 2.0 GB/s
>
> Performance counter stats for 'dd if=/dev/zero of=/dev/null count=1000000':
>
> 911,626,453 cycles # 0.000 GHz
>
> 0.262113350 seconds time elapsed
>
> The 0.000 GHz comment in the output is totally bogus and misleading. It
> happens because update_shadow_stats() doesn't touch runtime_nsecs_stats;
> it is only written when a requested counter matches a SW_TASK_CLOCK. In
> our case, since we have only requested HW_CPU_CYCLES,
> runtime_nsecs_stats is unavailable. So, omit printing the comment
> altogether.
>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> tools/perf/builtin-stat.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
Acked-by: Ingo Molnar <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:perf/core] perf stat: Don't print bogus data on -e cycles
2013-09-30 11:13 ` [PATCH 2/2] perf tool: don't print bogus data on -e cycles Ramkumar Ramachandra
2013-10-01 8:48 ` Ingo Molnar
@ 2013-10-15 5:29 ` tip-bot for Ramkumar Ramachandra
1 sibling, 0 replies; 5+ messages in thread
From: tip-bot for Ramkumar Ramachandra @ 2013-10-15 5:29 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra, tglx, artagnon
Commit-ID: c458fe62ca31496664c1211a7906d261220b18f9
Gitweb: http://git.kernel.org/tip/c458fe62ca31496664c1211a7906d261220b18f9
Author: Ramkumar Ramachandra <artagnon@gmail.com>
AuthorDate: Mon, 30 Sep 2013 16:43:05 +0530
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 11 Oct 2013 12:17:33 -0300
perf stat: Don't print bogus data on -e cycles
When only the cycles event is requested:
$ perf stat -e cycles dd if=/dev/zero of=/dev/null count=1000000
1000000+0 records in
1000000+0 records out
512000000 bytes (512 MB) copied, 0.26123 s, 2.0 GB/s
Performance counter stats for 'dd if=/dev/zero of=/dev/null count=1000000':
911,626,453 cycles # 0.000 GHz
0.262113350 seconds time elapsed
The 0.000 GHz comment in the output is totally bogus and misleading. It
happens because update_shadow_stats() doesn't touch runtime_nsecs_stats;
it is only written when a requested counter matches a SW_TASK_CLOCK. In
our case, since we have only requested HW_CPU_CYCLES,
runtime_nsecs_stats is unavailable. So, omit printing the comment
altogether.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1380539585-23859-3-git-send-email-artagnon@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 700b478..ce2266c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -997,10 +997,10 @@ static void abs_printout(int cpu, int nr, struct perf_evsel *evsel, double avg)
} else if (perf_evsel__match(evsel, HARDWARE, HW_CPU_CYCLES)) {
total = avg_stats(&runtime_nsecs_stats[cpu]);
- if (total)
- ratio = 1.0 * avg / total;
-
- fprintf(output, " # %8.3f GHz ", ratio);
+ if (total) {
+ ratio = avg / total;
+ fprintf(output, " # %8.3f GHz ", ratio);
+ }
} else if (transaction_run &&
perf_evsel__cmp(evsel, nth_evsel(T_CYCLES_IN_TX))) {
total = avg_stats(&runtime_cycles_stats[cpu]);
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-15 5:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-30 11:13 [PATCH 0/2] Toying with the perf tool Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 1/2] perf tool: simplify ARCH code in Makefile Ramkumar Ramachandra
2013-09-30 11:13 ` [PATCH 2/2] perf tool: don't print bogus data on -e cycles Ramkumar Ramachandra
2013-10-01 8:48 ` Ingo Molnar
2013-10-15 5:29 ` [tip:perf/core] perf stat: Don't " tip-bot for Ramkumar Ramachandra
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).