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