* [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms
@ 2009-05-26 22:21 Arnaldo Carvalho de Melo
2009-05-27 6:10 ` Ingo Molnar
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-05-26 22:21 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Paul Mackerras, Mike Galbraith, Thomas Gleixner,
Steven Rostedt, Linux Kernel Mailing List
>From 7508bc84d541ac245be633803d393244b6860af6 Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Tue, 26 May 2009 19:14:47 -0300
Subject: [PATCH] perf: report should only load text symbols from kallsyms
Just like we do for userspace when reading the symtab, reducing the
number of entries we insert on the symbols rbtree.
Before:
[acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
Performance counter stats for 'perf':
218.138382 task clock ticks (msecs)
4 context switches (events)
8 CPU migrations (events)
2136 pagefaults (events)
32746212 CPU cycles (events) (scaled from 67.04%)
11961102 instructions (events) (scaled from 66.19%)
49841 cache references (events) (scaled from 21.96%)
13777 cache misses (events) (scaled from 21.98%)
Wall-clock time elapsed: 218.702477 msecs
[acme@emilia ~]$ perf report -i perf_report.perf | head
11.06 perf [.] 0x00000000000057cb /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
9.15 perf [.] 0x00000000000056a0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
8.72 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
8.51 perf [.] 0x0000000000006672 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
3.83 perf [k] 0xffffffff811cfc5a vsnprintf
3.40 perf [.] 0x0000000000005e33 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
3.40 perf [.] 0x0000000000005ec7 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
3.19 perf [k] 0xffffffff811ce1c1 number
2.77 perf [.] 0x0000000000006869 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
2.77 perf [.] 0x000000000000fde3 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: rb_insert_color
[acme@emilia ~]$
After:
acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
Performance counter stats for 'perf':
190.228511 task clock ticks (msecs)
4 context switches (events)
7 CPU migrations (events)
1625 pagefaults (events)
29578745 CPU cycles (events) (scaled from 66.92%)
10516914 instructions (events) (scaled from 66.47%)
44015 cache references (events) (scaled from 22.04%)
8248 cache misses (events) (scaled from 22.07%)
Wall-clock time elapsed: 190.816096 msecs
[acme@emilia ~]$ perf report -i perf_report.perf | head
15.99 perf [.] 0x00000000000057a9 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
10.87 perf [.] 0x000000000000674d /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
8.74 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
5.54 perf [.] 0x0000000000005e42 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
4.48 perf [.] 0x0000000000005ebe /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
4.48 perf [k] 0xffffffff811cfba0 vsnprintf
3.84 perf [.] 0x00000000000056b4 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
3.62 perf [.] 0x00000000000068d0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
3.20 perf [k] 0xffffffff811ce0b3 number
2.56 perf [.] 0x0000000000006d78 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: __cmd_report
[acme@emilia ~]$
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
Documentation/perf_counter/builtin-report.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/perf_counter/builtin-report.c b/Documentation/perf_counter/builtin-report.c
index c517483..a55f15d 100644
--- a/Documentation/perf_counter/builtin-report.c
+++ b/Documentation/perf_counter/builtin-report.c
@@ -3,6 +3,7 @@
#include <libelf.h>
#include <gelf.h>
#include <elf.h>
+#include <ctype.h>
#include "util/list.h"
#include "util/rbtree.h"
@@ -408,13 +409,20 @@ static int load_kallsyms(void)
int len = hex2long(line, &start);
- len += 3; /* ' t ' */
- if (len >= line_len)
+ len++;
+ if (len + 2 >= line_len)
+ continue;
+
+ char symbol_type = line[len];
+ /*
+ * We're interested only in code ('T'ext)
+ */
+ if (toupper(symbol_type) != 'T')
continue;
/*
* Well fix up the end later, when we have all sorted.
*/
- struct symbol *sym = symbol__new(start, 0xdead, line + len);
+ struct symbol *sym = symbol__new(start, 0xdead, line + len + 2);
if (sym == NULL)
goto out_delete_dso;
--
1.6.0.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms
2009-05-26 22:21 [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms Arnaldo Carvalho de Melo
@ 2009-05-27 6:10 ` Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only " tip-bot for Arnaldo Carvalho de Melo
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2009-05-27 6:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Mike Galbraith, Thomas Gleixner,
Steven Rostedt, Linux Kernel Mailing List
* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> >From 7508bc84d541ac245be633803d393244b6860af6 Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Tue, 26 May 2009 19:14:47 -0300
> Subject: [PATCH] perf: report should only load text symbols from kallsyms
>
> Just like we do for userspace when reading the symtab, reducing the
> number of entries we insert on the symbols rbtree.
>
> Before:
>
> [acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
>
> Performance counter stats for 'perf':
>
> 218.138382 task clock ticks (msecs)
> 4 context switches (events)
> 8 CPU migrations (events)
> 2136 pagefaults (events)
> 32746212 CPU cycles (events) (scaled from 67.04%)
> 11961102 instructions (events) (scaled from 66.19%)
> 49841 cache references (events) (scaled from 21.96%)
> 13777 cache misses (events) (scaled from 21.98%)
>
> Wall-clock time elapsed: 218.702477 msecs
>
> [acme@emilia ~]$ perf report -i perf_report.perf | head
> 11.06 perf [.] 0x00000000000057cb /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
> 9.15 perf [.] 0x00000000000056a0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
> 8.72 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
> 8.51 perf [.] 0x0000000000006672 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
> 3.83 perf [k] 0xffffffff811cfc5a vsnprintf
> 3.40 perf [.] 0x0000000000005e33 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
> 3.40 perf [.] 0x0000000000005ec7 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
> 3.19 perf [k] 0xffffffff811ce1c1 number
> 2.77 perf [.] 0x0000000000006869 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
> 2.77 perf [.] 0x000000000000fde3 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: rb_insert_color
> [acme@emilia ~]$
>
> After:
>
> acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
>
> Performance counter stats for 'perf':
>
> 190.228511 task clock ticks (msecs)
> 4 context switches (events)
> 7 CPU migrations (events)
> 1625 pagefaults (events)
> 29578745 CPU cycles (events) (scaled from 66.92%)
> 10516914 instructions (events) (scaled from 66.47%)
> 44015 cache references (events) (scaled from 22.04%)
> 8248 cache misses (events) (scaled from 22.07%)
>
> Wall-clock time elapsed: 190.816096 msecs
nice!
> [acme@emilia ~]$ perf report -i perf_report.perf | head
> 15.99 perf [.] 0x00000000000057a9 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
> 10.87 perf [.] 0x000000000000674d /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
> 8.74 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
> 5.54 perf [.] 0x0000000000005e42 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
> 4.48 perf [.] 0x0000000000005ebe /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
> 4.48 perf [k] 0xffffffff811cfba0 vsnprintf
> 3.84 perf [.] 0x00000000000056b4 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
> 3.62 perf [.] 0x00000000000068d0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
> 3.20 perf [k] 0xffffffff811ce0b3 number
> 2.56 perf [.] 0x0000000000006d78 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: __cmd_report
> [acme@emilia ~]$
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> Documentation/perf_counter/builtin-report.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/perf_counter/builtin-report.c b/Documentation/perf_counter/builtin-report.c
> index c517483..a55f15d 100644
> --- a/Documentation/perf_counter/builtin-report.c
> +++ b/Documentation/perf_counter/builtin-report.c
> @@ -3,6 +3,7 @@
> #include <libelf.h>
> #include <gelf.h>
> #include <elf.h>
> +#include <ctype.h>
>
> #include "util/list.h"
> #include "util/rbtree.h"
> @@ -408,13 +409,20 @@ static int load_kallsyms(void)
>
> int len = hex2long(line, &start);
>
> - len += 3; /* ' t ' */
> - if (len >= line_len)
> + len++;
> + if (len + 2 >= line_len)
> + continue;
> +
> + char symbol_type = line[len];
> + /*
> + * We're interested only in code ('T'ext)
> + */
> + if (toupper(symbol_type) != 'T')
> continue;
We would also include ' W ' symbols, right?
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:perfcounters/core] perf report: Only load text symbols from kallsyms
2009-05-26 22:21 [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms Arnaldo Carvalho de Melo
2009-05-27 6:10 ` Ingo Molnar
@ 2009-05-27 7:16 ` tip-bot for Arnaldo Carvalho de Melo
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only load text symbols from kallsyms, fix tip-bot for Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking tip-bot for Ingo Molnar
3 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Arnaldo Carvalho de Melo @ 2009-05-27 7:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, acme, hpa, mingo, jkacur, a.p.zijlstra,
efault, rostedt, mtosatti, tglx, cjashfor, mingo
Commit-ID: 03f6316d32738ec5eda2e6f628c12d1c01e61a87
Gitweb: http://git.kernel.org/tip/03f6316d32738ec5eda2e6f628c12d1c01e61a87
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Tue, 26 May 2009 19:21:55 -0300
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 27 May 2009 09:10:36 +0200
perf report: Only load text symbols from kallsyms
Just like we do for userspace when reading the symtab, reducing the
number of entries we insert on the symbols rbtree.
Before:
[acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
Performance counter stats for 'perf':
218.138382 task clock ticks (msecs)
4 context switches (events)
8 CPU migrations (events)
2136 pagefaults (events)
32746212 CPU cycles (events) (scaled from 67.04%)
11961102 instructions (events) (scaled from 66.19%)
49841 cache references (events) (scaled from 21.96%)
13777 cache misses (events) (scaled from 21.98%)
Wall-clock time elapsed: 218.702477 msecs
[acme@emilia ~]$ perf report -i perf_report.perf | head
11.06 perf [.] 0x00000000000057cb /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
9.15 perf [.] 0x00000000000056a0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
8.72 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
8.51 perf [.] 0x0000000000006672 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
3.83 perf [k] 0xffffffff811cfc5a vsnprintf
3.40 perf [.] 0x0000000000005e33 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
3.40 perf [.] 0x0000000000005ec7 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
3.19 perf [k] 0xffffffff811ce1c1 number
2.77 perf [.] 0x0000000000006869 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
2.77 perf [.] 0x000000000000fde3 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: rb_insert_color
[acme@emilia ~]$
After:
acme@emilia ~]$ rm -f perf_report.perf ; perf record -o perf_report.perf perf stat perf report > /dev/null
Performance counter stats for 'perf':
190.228511 task clock ticks (msecs)
4 context switches (events)
7 CPU migrations (events)
1625 pagefaults (events)
29578745 CPU cycles (events) (scaled from 66.92%)
10516914 instructions (events) (scaled from 66.47%)
44015 cache references (events) (scaled from 22.04%)
8248 cache misses (events) (scaled from 22.07%)
Wall-clock time elapsed: 190.816096 msecs
[acme@emilia ~]$ perf report -i perf_report.perf | head
15.99 perf [.] 0x00000000000057a9 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__find_symbol
10.87 perf [.] 0x000000000000674d /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: thread__symbol_incnew
8.74 perf [k] 0xffffffff8101b1d2 intel_pmu_enable_all
5.54 perf [.] 0x0000000000005e42 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex
4.48 perf [.] 0x0000000000005ebe /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: hex2long
4.48 perf [k] 0xffffffff811cfba0 vsnprintf
3.84 perf [.] 0x00000000000056b4 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: dso__insert_symbol
3.62 perf [.] 0x00000000000068d0 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: threads__findnew
3.20 perf [k] 0xffffffff811ce0b3 number
2.56 perf [.] 0x0000000000006d78 /home/acme/git/linux-2.6-tip/Documentation/perf_counter/perf: __cmd_report
[acme@emilia ~]$
[ Impact: optimization ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20090526222155.GJ4424@ghostprotocols.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
Documentation/perf_counter/builtin-report.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/perf_counter/builtin-report.c b/Documentation/perf_counter/builtin-report.c
index c517483..a55f15d 100644
--- a/Documentation/perf_counter/builtin-report.c
+++ b/Documentation/perf_counter/builtin-report.c
@@ -3,6 +3,7 @@
#include <libelf.h>
#include <gelf.h>
#include <elf.h>
+#include <ctype.h>
#include "util/list.h"
#include "util/rbtree.h"
@@ -408,13 +409,20 @@ static int load_kallsyms(void)
int len = hex2long(line, &start);
- len += 3; /* ' t ' */
- if (len >= line_len)
+ len++;
+ if (len + 2 >= line_len)
+ continue;
+
+ char symbol_type = line[len];
+ /*
+ * We're interested only in code ('T'ext)
+ */
+ if (toupper(symbol_type) != 'T')
continue;
/*
* Well fix up the end later, when we have all sorted.
*/
- struct symbol *sym = symbol__new(start, 0xdead, line + len);
+ struct symbol *sym = symbol__new(start, 0xdead, line + len + 2);
if (sym == NULL)
goto out_delete_dso;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perfcounters/core] perf report: Only load text symbols from kallsyms, fix
2009-05-26 22:21 [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms Arnaldo Carvalho de Melo
2009-05-27 6:10 ` Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only " tip-bot for Arnaldo Carvalho de Melo
@ 2009-05-27 7:16 ` tip-bot for Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking tip-bot for Ingo Molnar
3 siblings, 0 replies; 7+ messages in thread
From: tip-bot for Ingo Molnar @ 2009-05-27 7:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, acme, hpa, mingo, jkacur, a.p.zijlstra,
efault, rostedt, mtosatti, tglx, cjashfor, mingo
Commit-ID: af83632f98aefd1ae4d8ca3c7c285ccf6a7d3956
Gitweb: http://git.kernel.org/tip/af83632f98aefd1ae4d8ca3c7c285ccf6a7d3956
Author: Ingo Molnar <mingo@elte.hu>
AuthorDate: Wed, 27 May 2009 08:38:48 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 27 May 2009 09:10:37 +0200
perf report: Only load text symbols from kallsyms, fix
- allow 'W' symbols too
- Convert initializations to C99 style
- whitespace cleanups
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20090526222155.GJ4424@ghostprotocols.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
Documentation/perf_counter/builtin-report.c | 30 ++++++++++++++++----------
1 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/Documentation/perf_counter/builtin-report.c b/Documentation/perf_counter/builtin-report.c
index a55f15d..ed3da9d 100644
--- a/Documentation/perf_counter/builtin-report.c
+++ b/Documentation/perf_counter/builtin-report.c
@@ -384,21 +384,26 @@ int hex2long(char *ptr, unsigned long *long_val)
static int load_kallsyms(void)
{
+ struct rb_node *nd, *prevnd;
+ char *line = NULL;
+ FILE *file;
+ size_t n;
+
kernel_dso = dso__new("[kernel]");
if (kernel_dso == NULL)
return -1;
- FILE *file = fopen("/proc/kallsyms", "r");
-
+ file = fopen("/proc/kallsyms", "r");
if (file == NULL)
goto out_delete_dso;
- char *line = NULL;
- size_t n;
-
while (!feof(file)) {
unsigned long start;
- int line_len = getline(&line, &n, file);
+ struct symbol *sym;
+ int line_len, len;
+ char symbol_type;
+
+ line_len = getline(&line, &n, file);
if (line_len < 0)
break;
@@ -407,22 +412,22 @@ static int load_kallsyms(void)
line[--line_len] = '\0'; /* \n */
- int len = hex2long(line, &start);
-
+ len = hex2long(line, &start);
+
len++;
if (len + 2 >= line_len)
continue;
- char symbol_type = line[len];
+ symbol_type = toupper(line[len]);
/*
* We're interested only in code ('T'ext)
*/
- if (toupper(symbol_type) != 'T')
+ if (symbol_type != 'T' && symbol_type != 'W')
continue;
/*
* Well fix up the end later, when we have all sorted.
*/
- struct symbol *sym = symbol__new(start, 0xdead, line + len + 2);
+ sym = symbol__new(start, 0xdead, line + len + 2);
if (sym == NULL)
goto out_delete_dso;
@@ -434,7 +439,7 @@ static int load_kallsyms(void)
* Now that we have all sorted out, just set the ->end of all
* symbols
*/
- struct rb_node *nd, *prevnd = rb_first(&kernel_dso->syms);
+ prevnd = rb_first(&kernel_dso->syms);
if (prevnd == NULL)
goto out_delete_line;
@@ -450,6 +455,7 @@ static int load_kallsyms(void)
dsos__add(kernel_dso);
free(line);
fclose(file);
+
return 0;
out_delete_line:
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking
2009-05-26 22:21 [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only load text symbols from kallsyms, fix tip-bot for Ingo Molnar
@ 2009-05-27 7:16 ` tip-bot for Ingo Molnar
2009-05-28 3:56 ` Paul Mackerras
3 siblings, 1 reply; 7+ messages in thread
From: tip-bot for Ingo Molnar @ 2009-05-27 7:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, paulus, acme, hpa, mingo, jkacur, a.p.zijlstra,
efault, rostedt, mtosatti, tglx, cjashfor, mingo
Commit-ID: 16f762a2ac5ecf8a11f6f0332e46cc3459220da5
Gitweb: http://git.kernel.org/tip/16f762a2ac5ecf8a11f6f0332e46cc3459220da5
Author: Ingo Molnar <mingo@elte.hu>
AuthorDate: Wed, 27 May 2009 09:10:38 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 27 May 2009 08:10:35 +0200
perf_counter tools: Introduce stricter C code checking
Tighten up our C code requirements:
- disallow warnings
- disallow declarations-mixed-with-statements
- require proper prototypes
- require C99 (with gcc extensions)
Fix up a ton of problems these measures unearth:
- unused functions
- needlessly global functions
- missing prototypes
- code mixed with declarations
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: John Kacur <jkacur@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
LKML-Reference: <20090526222155.GJ4424@ghostprotocols.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
Documentation/perf_counter/Makefile | 2 +-
Documentation/perf_counter/builtin-help.c | 2 +-
Documentation/perf_counter/builtin-record.c | 38 ++++++-----
Documentation/perf_counter/builtin-report.c | 103 +++++++++++++--------------
Documentation/perf_counter/builtin-stat.c | 3 +-
Documentation/perf_counter/builtin-top.c | 2 +-
Documentation/perf_counter/util/abspath.c | 2 +-
Documentation/perf_counter/util/cache.h | 2 +
Documentation/perf_counter/util/util.h | 2 +
9 files changed, 82 insertions(+), 74 deletions(-)
diff --git a/Documentation/perf_counter/Makefile b/Documentation/perf_counter/Makefile
index 10c13a6..efb0589 100644
--- a/Documentation/perf_counter/Makefile
+++ b/Documentation/perf_counter/Makefile
@@ -159,7 +159,7 @@ uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
# CFLAGS and LDFLAGS are for the users to override from the command line.
-CFLAGS = -ggdb3 -Wall
+CFLAGS = -ggdb3 -Wall -Werror -Wstrict-prototypes -Wmissing-declarations -Wmissing-prototypes -std=gnu99 -Wdeclaration-after-statement
LDFLAGS = -lpthread -lrt -lelf
ALL_CFLAGS = $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
diff --git a/Documentation/perf_counter/builtin-help.c b/Documentation/perf_counter/builtin-help.c
index 6616de0..d2bd317 100644
--- a/Documentation/perf_counter/builtin-help.c
+++ b/Documentation/perf_counter/builtin-help.c
@@ -399,7 +399,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
* HTML.
*/
#ifndef open_html
-void open_html(const char *path)
+static void open_html(const char *path)
{
execl_perf_cmd("web--browse", "-c", "help.browser", path, NULL);
}
diff --git a/Documentation/perf_counter/builtin-record.c b/Documentation/perf_counter/builtin-record.c
index ec2b787..68abfdf 100644
--- a/Documentation/perf_counter/builtin-record.c
+++ b/Documentation/perf_counter/builtin-record.c
@@ -1,6 +1,7 @@
#include "perf.h"
+#include "builtin.h"
#include "util/util.h"
#include "util/parse-options.h"
#include "util/parse-events.h"
@@ -144,26 +145,32 @@ static int nr_poll;
static int nr_cpu;
struct mmap_event {
- struct perf_event_header header;
- __u32 pid, tid;
- __u64 start;
- __u64 len;
- __u64 pgoff;
- char filename[PATH_MAX];
+ struct perf_event_header header;
+ __u32 pid;
+ __u32 tid;
+ __u64 start;
+ __u64 len;
+ __u64 pgoff;
+ char filename[PATH_MAX];
};
+
struct comm_event {
- struct perf_event_header header;
- __u32 pid,tid;
- char comm[16];
+ struct perf_event_header header;
+ __u32 pid;
+ __u32 tid;
+ char comm[16];
};
static pid_t pid_synthesize_comm_event(pid_t pid)
{
+ struct comm_event comm_ev;
char filename[PATH_MAX];
+ pid_t spid, ppid;
char bf[BUFSIZ];
- struct comm_event comm_ev;
+ int fd, nr, ret;
+ char comm[18];
size_t size;
- int fd;
+ char state;
snprintf(filename, sizeof(filename), "/proc/%d/stat", pid);
@@ -178,12 +185,8 @@ static pid_t pid_synthesize_comm_event(pid_t pid)
}
close(fd);
- pid_t spid, ppid;
- char state;
- char comm[18];
-
memset(&comm_ev, 0, sizeof(comm_ev));
- int nr = sscanf(bf, "%d %s %c %d %d ",
+ nr = sscanf(bf, "%d %s %c %d %d ",
&spid, comm, &state, &ppid, &comm_ev.pid);
if (nr != 5) {
fprintf(stderr, "couldn't get COMM and pgid, malformed %s\n",
@@ -198,7 +201,8 @@ static pid_t pid_synthesize_comm_event(pid_t pid)
memcpy(comm_ev.comm, comm + 1, size);
size = ALIGN(size, sizeof(uint64_t));
comm_ev.header.size = sizeof(comm_ev) - (sizeof(comm_ev.comm) - size);
- int ret = write(output, &comm_ev, comm_ev.header.size);
+
+ ret = write(output, &comm_ev, comm_ev.header.size);
if (ret < 0) {
perror("failed to write");
exit(-1);
diff --git a/Documentation/perf_counter/builtin-report.c b/Documentation/perf_counter/builtin-report.c
index 2d65d9c..7f1255d 100644
--- a/Documentation/perf_counter/builtin-report.c
+++ b/Documentation/perf_counter/builtin-report.c
@@ -1,4 +1,5 @@
#include "util/util.h"
+#include "builtin.h"
#include <libelf.h>
#include <gelf.h>
@@ -22,7 +23,7 @@ static int input;
static int show_mask = SHOW_KERNEL | SHOW_USER | SHOW_HV;
static int dump_trace = 0;
-static int verbose;
+static int verbose;
static unsigned long page_size;
static unsigned long mmap_window = 32;
@@ -60,10 +61,10 @@ typedef union event_union {
} event_t;
struct symbol {
- struct rb_node rb_node;
- uint64_t start;
- uint64_t end;
- char name[0];
+ struct rb_node rb_node;
+ __u64 start;
+ __u64 end;
+ char name[0];
};
static struct symbol *symbol__new(uint64_t start, uint64_t len, const char *name)
@@ -86,7 +87,7 @@ static void symbol__delete(struct symbol *self)
static size_t symbol__fprintf(struct symbol *self, FILE *fp)
{
- return fprintf(fp, " %lx-%lx %s\n",
+ return fprintf(fp, " %llx-%llx %s\n",
self->start, self->end, self->name);
}
@@ -147,10 +148,12 @@ static void dso__insert_symbol(struct dso *self, struct symbol *sym)
static struct symbol *dso__find_symbol(struct dso *self, uint64_t ip)
{
+ struct rb_node *n;
+
if (self == NULL)
return NULL;
- struct rb_node *n = self->syms.rb_node;
+ n = self->syms.rb_node;
while (n) {
struct symbol *s = rb_entry(n, struct symbol, rb_node);
@@ -221,33 +224,42 @@ static Elf_Scn *elf_section_by_name(Elf *elf, GElf_Ehdr *ep,
static int dso__load(struct dso *self)
{
- int fd = open(self->name, O_RDONLY), err = -1;
+ Elf_Data *symstrs;
+ uint32_t nr_syms;
+ int fd, err = -1;
+ uint32_t index;
+ GElf_Ehdr ehdr;
+ GElf_Shdr shdr;
+ Elf_Data *syms;
+ GElf_Sym sym;
+ Elf_Scn *sec;
+ Elf *elf;
+
+ fd = open(self->name, O_RDONLY);
if (fd == -1)
return -1;
- Elf *elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
+ elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
if (elf == NULL) {
fprintf(stderr, "%s: cannot read %s ELF file.\n",
__func__, self->name);
goto out_close;
}
- GElf_Ehdr ehdr;
if (gelf_getehdr(elf, &ehdr) == NULL) {
fprintf(stderr, "%s: cannot get elf header.\n", __func__);
goto out_elf_end;
}
- GElf_Shdr shdr;
- Elf_Scn *sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
+ sec = elf_section_by_name(elf, &ehdr, &shdr, ".symtab", NULL);
if (sec == NULL)
sec = elf_section_by_name(elf, &ehdr, &shdr, ".dynsym", NULL);
if (sec == NULL)
goto out_elf_end;
- Elf_Data *syms = elf_getdata(sec, NULL);
+ syms = elf_getdata(sec, NULL);
if (syms == NULL)
goto out_elf_end;
@@ -255,14 +267,12 @@ static int dso__load(struct dso *self)
if (sec == NULL)
goto out_elf_end;
- Elf_Data *symstrs = elf_getdata(sec, NULL);
+ symstrs = elf_getdata(sec, NULL);
if (symstrs == NULL)
goto out_elf_end;
- const uint32_t nr_syms = shdr.sh_size / shdr.sh_entsize;
+ nr_syms = shdr.sh_size / shdr.sh_entsize;
- GElf_Sym sym;
- uint32_t index;
elf_symtab__for_each_symbol(syms, nr_syms, index, sym) {
struct symbol *f;
@@ -342,7 +352,7 @@ out_delete_dso:
return NULL;
}
-void dsos__fprintf(FILE *fp)
+static void dsos__fprintf(FILE *fp)
{
struct dso *pos;
@@ -365,7 +375,7 @@ static int hex(char ch)
* While we find nice hex chars, build a long_val.
* Return number of chars processed.
*/
-int hex2long(char *ptr, unsigned long *long_val)
+static int hex2long(char *ptr, unsigned long *long_val)
{
const char *p = ptr;
*long_val = 0;
@@ -493,12 +503,6 @@ out_delete:
return NULL;
}
-static size_t map__fprintf(struct map *self, FILE *fp)
-{
- return fprintf(fp, " %lx-%lx %lx %s\n",
- self->start, self->end, self->pgoff, self->dso->name);
-}
-
struct thread;
static const char *thread__name(struct thread *self, char *bf, size_t size);
@@ -531,11 +535,6 @@ static struct symhist *symhist__new(struct symbol *sym, uint64_t ip,
return self;
}
-void symhist__delete(struct symhist *self)
-{
- free(self);
-}
-
static void symhist__inc(struct symhist *self)
{
++self->count;
@@ -608,6 +607,8 @@ static int thread__symbol_incnew(struct thread *self, struct symbol *sym,
struct symhist *sh;
while (*p != NULL) {
+ uint64_t start;
+
parent = *p;
sh = rb_entry(parent, struct symhist, rb_node);
@@ -617,7 +618,7 @@ static int thread__symbol_incnew(struct thread *self, struct symbol *sym,
}
/* Handle unresolved symbols too */
- const uint64_t start = !sh->sym ? sh->ip : sh->sym->start;
+ start = !sh->sym ? sh->ip : sh->sym->start;
if (ip < start)
p = &(*p)->rb_left;
@@ -639,17 +640,6 @@ static int thread__set_comm(struct thread *self, const char *comm)
return self->comm ? 0 : -ENOMEM;
}
-size_t thread__maps_fprintf(struct thread *self, FILE *fp)
-{
- struct map *pos;
- size_t ret = 0;
-
- list_for_each_entry(pos, &self->maps, node)
- ret += map__fprintf(pos, fp);
-
- return ret;
-}
-
static size_t thread__fprintf(struct thread *self, FILE *fp)
{
int ret = fprintf(fp, "thread: %d %s\n", self->pid, self->comm);
@@ -657,13 +647,14 @@ static size_t thread__fprintf(struct thread *self, FILE *fp)
for (nd = rb_first(&self->symhists); nd; nd = rb_next(nd)) {
struct symhist *pos = rb_entry(nd, struct symhist, rb_node);
+
ret += symhist__fprintf(pos, 0, fp);
}
return ret;
}
-static struct rb_root threads = RB_ROOT;
+static struct rb_root threads;
static struct thread *threads__findnew(pid_t pid)
{
@@ -699,11 +690,11 @@ static void thread__insert_map(struct thread *self, struct map *map)
static struct map *thread__find_map(struct thread *self, uint64_t ip)
{
+ struct map *pos;
+
if (self == NULL)
return NULL;
- struct map *pos;
-
list_for_each_entry(pos, &self->maps, node)
if (ip >= pos->start && ip <= pos->end)
return pos;
@@ -711,7 +702,7 @@ static struct map *thread__find_map(struct thread *self, uint64_t ip)
return NULL;
}
-void threads__fprintf(FILE *fp)
+static void threads__fprintf(FILE *fp)
{
struct rb_node *nd;
for (nd = rb_first(&threads); nd; nd = rb_next(nd)) {
@@ -720,7 +711,7 @@ void threads__fprintf(FILE *fp)
}
}
-static struct rb_root global_symhists = RB_ROOT;
+static struct rb_root global_symhists;
static void threads__insert_symhist(struct symhist *sh)
{
@@ -852,7 +843,7 @@ more:
(void *)(long)(event->header.size),
event->header.misc,
event->ip.pid,
- (void *)event->ip.ip);
+ (void *)(long)ip);
}
if (thread == NULL) {
@@ -866,9 +857,12 @@ more:
level = 'k';
dso = kernel_dso;
} else if (event->header.misc & PERF_EVENT_MISC_USER) {
+ struct map *map;
+
show = SHOW_USER;
level = '.';
- struct map *map = thread__find_map(thread, ip);
+
+ map = thread__find_map(thread, ip);
if (map != NULL) {
dso = map->dso;
ip -= map->start + map->pgoff;
@@ -896,9 +890,9 @@ more:
fprintf(stderr, "%p [%p]: PERF_EVENT_MMAP: [%p(%p) @ %p]: %s\n",
(void *)(offset + head),
(void *)(long)(event->header.size),
- (void *)event->mmap.start,
- (void *)event->mmap.len,
- (void *)event->mmap.pgoff,
+ (void *)(long)event->mmap.start,
+ (void *)(long)event->mmap.len,
+ (void *)(long)event->mmap.pgoff,
event->mmap.filename);
}
if (thread == NULL || map == NULL) {
@@ -964,6 +958,11 @@ done:
return 0;
}
+ if (verbose >= 2) {
+ dsos__fprintf(stdout);
+ threads__fprintf(stdout);
+ }
+
threads__sort_symhists();
threads__symhists_fprintf(total, stdout);
diff --git a/Documentation/perf_counter/builtin-stat.c b/Documentation/perf_counter/builtin-stat.c
index e7cb941..ce661e2 100644
--- a/Documentation/perf_counter/builtin-stat.c
+++ b/Documentation/perf_counter/builtin-stat.c
@@ -30,6 +30,7 @@
*/
#include "perf.h"
+#include "builtin.h"
#include "util/util.h"
#include "util/parse-options.h"
#include "util/parse-events.h"
@@ -108,7 +109,7 @@ static void create_perfstat_counter(int counter)
}
}
-int do_perfstat(int argc, const char **argv)
+static int do_perfstat(int argc, const char **argv)
{
unsigned long long t0, t1;
int counter;
diff --git a/Documentation/perf_counter/builtin-top.c b/Documentation/perf_counter/builtin-top.c
index 6b1c66f..a890872 100644
--- a/Documentation/perf_counter/builtin-top.c
+++ b/Documentation/perf_counter/builtin-top.c
@@ -42,8 +42,8 @@
* Released under the GPL v2. (and only v2, not any later version)
*/
-
#include "perf.h"
+#include "builtin.h"
#include "util/util.h"
#include "util/util.h"
#include "util/parse-options.h"
diff --git a/Documentation/perf_counter/util/abspath.c b/Documentation/perf_counter/util/abspath.c
index 649f34f..61d33b8 100644
--- a/Documentation/perf_counter/util/abspath.c
+++ b/Documentation/perf_counter/util/abspath.c
@@ -5,7 +5,7 @@
* symlink to a directory, we do not want to say it is a directory when
* dealing with tracked content in the working tree.
*/
-int is_directory(const char *path)
+static int is_directory(const char *path)
{
struct stat st;
return (!stat(path, &st) && S_ISDIR(st.st_mode));
diff --git a/Documentation/perf_counter/util/cache.h b/Documentation/perf_counter/util/cache.h
index 7108051..393d614 100644
--- a/Documentation/perf_counter/util/cache.h
+++ b/Documentation/perf_counter/util/cache.h
@@ -104,6 +104,8 @@ char *strip_path_suffix(const char *path, const char *suffix);
extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
extern char *perf_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
+/* perf_mkstemp() - create tmp file honoring TMPDIR variable */
+extern int perf_mkstemp(char *path, size_t len, const char *template);
extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
diff --git a/Documentation/perf_counter/util/util.h b/Documentation/perf_counter/util/util.h
index 36e40c3..76590a1 100644
--- a/Documentation/perf_counter/util/util.h
+++ b/Documentation/perf_counter/util/util.h
@@ -309,6 +309,8 @@ extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern int xdup(int fd);
extern FILE *xfdopen(int fd, const char *mode);
+extern int xmkstemp(char *template);
+
static inline size_t xsize_t(off_t len)
{
return (size_t)len;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking
2009-05-27 7:16 ` [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking tip-bot for Ingo Molnar
@ 2009-05-28 3:56 ` Paul Mackerras
2009-05-28 8:35 ` Ingo Molnar
0 siblings, 1 reply; 7+ messages in thread
From: Paul Mackerras @ 2009-05-28 3:56 UTC (permalink / raw)
To: mingo, hpa, acme, linux-kernel, jkacur, a.p.zijlstra, efault,
rostedt, mtosatti, tglx, cjashfor, mingo
Cc: linux-tip-commits
tip-bot for Ingo Molnar writes:
> perf_counter tools: Introduce stricter C code checking
>
> Tighten up our C code requirements:
>
> - disallow warnings
This causes failures when I compile it as a 64-bit executable on
powerpc:
CC builtin-record.o
builtin-record.c: In function 'pid_synthesize_mmap_events':
builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 3 has type '__u64 *'
builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 4 has type '__u64 *'
builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 9 has type '__u64 *'
This is because u64 is an unsigned long in userspace for a 64-bit
build, not unsigned long long. I'm not sure how best to solve this
problem.
If I compile it as a 32-bit executable, it doesn't generate warnings,
but when I try to run "perf top" (this is on a 64-bit kernel, of
course, since 32-bit powerpc kernels don't currently support
perf_counters), I get:
# perf top
left: 0000000000000000
ip: 00000000000891a4
right: 00000000ffffffff
KernelTop refresh period: 2 seconds
perf: builtin-top.c:453: record_ip: Assertion `left <= ip && ip <= right' failed.
Aborted
Paul.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking
2009-05-28 3:56 ` Paul Mackerras
@ 2009-05-28 8:35 ` Ingo Molnar
0 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2009-05-28 8:35 UTC (permalink / raw)
To: Paul Mackerras
Cc: mingo, hpa, acme, linux-kernel, jkacur, a.p.zijlstra, efault,
rostedt, mtosatti, tglx, cjashfor, linux-tip-commits
* Paul Mackerras <paulus@samba.org> wrote:
> tip-bot for Ingo Molnar writes:
>
> > perf_counter tools: Introduce stricter C code checking
> >
> > Tighten up our C code requirements:
> >
> > - disallow warnings
>
> This causes failures when I compile it as a 64-bit executable on
> powerpc:
>
> CC builtin-record.o
> builtin-record.c: In function 'pid_synthesize_mmap_events':
> builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 3 has type '__u64 *'
> builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 4 has type '__u64 *'
> builtin-record.c:241: warning: format '%llx' expects type 'long long unsigned int *', but argument 9 has type '__u64 *'
>
> This is because u64 is an unsigned long in userspace for a 64-bit
> build, not unsigned long long. I'm not sure how best to solve
> this problem.
We could perhaps use __u64 consistently? (can we?)
> If I compile it as a 32-bit executable, it doesn't generate warnings,
> but when I try to run "perf top" (this is on a 64-bit kernel, of
> course, since 32-bit powerpc kernels don't currently support
> perf_counters), I get:
>
> # perf top
> left: 0000000000000000
> ip: 00000000000891a4
> right: 00000000ffffffff
> KernelTop refresh period: 2 seconds
> perf: builtin-top.c:453: record_ip: Assertion `left <= ip && ip <= right' failed.
> Aborted
mind trying a 'git bisect run' session - which commit broke things
for you? Or is this related to the type problems?
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-05-28 8:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-26 22:21 [PATCH 2/2 tip] perf: report should only load text symbols from kallsyms Arnaldo Carvalho de Melo
2009-05-27 6:10 ` Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only " tip-bot for Arnaldo Carvalho de Melo
2009-05-27 7:16 ` [tip:perfcounters/core] perf report: Only load text symbols from kallsyms, fix tip-bot for Ingo Molnar
2009-05-27 7:16 ` [tip:perfcounters/core] perf_counter tools: Introduce stricter C code checking tip-bot for Ingo Molnar
2009-05-28 3:56 ` Paul Mackerras
2009-05-28 8:35 ` Ingo Molnar
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).