perf tools: Some fixes and updates
cover perf tools: Some fixes and updates
Enclosed some changes made while working with the code. Mostly trivial but worth to add.
-Robert
Robert Richter (4):
perf tools: Fix include header files in util/parse-events.h perf tools: Fix thread map that is type pid_t
perf tools: Declare some references static in builtin-stat.c perf tools: Use sw counter only if hw pmu is not detected
tools/perf/builtin-record.c | 2 +- tools/perf/builtin-stat.c | 26 +++++++++++++------------- tools/perf/util/parse-events.h | 2 ++ tools/perf/util/thread_map.h | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-)
Signature · 1 line
1.7.8.4
PATCH 1/4 perf tools: Fix include header files in util/parse-events.h
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’
... and types.h:
util/parse-events.h:28: error: expected ‘)’ before ‘config’ util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’ util/parse-events.h:45: error: expected ‘)’ before ‘type’
This happens if now other include files are included before util/parse-events.h.
--- tools/perf/util/parse-events.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
tools/perf/util/parse-events.h +2 -0
@@ -4,7 +4,9 @@* Parse symbolic events/counts passed in as options: */ +#include <stdbool.h> #include "../../../include/linux/perf_event.h" +#include "types.h" struct list_head ;struct perf_evsel ;
Signature · 1 line
1.7.8.4
Re: [PATCH 1/4] perf tools: Fix include header files in util/parse-events.h
Quoted 8 lines · Robert Richter
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’... and
types.h:
Quoted 8 lines
util/parse-events.h:28: error: expected ‘)’ before ‘config’util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’util/parse-events.h:45: error: expected ‘)’ before ‘type’This happens if now other include files are included before
util/parse-events.h.Signed-off-by Robert Richter <[email protected]>
Arnaldo,
is it your intention that you didn't apply this patch?
Thanks,
-Robert
Quoted 21 lines
--- tools/perf/util/parse-events.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)tools/perf/util/parse-events.h +2
-0@@ -4,7 +4,9 @@* Parse symbolic events/counts passed in as options: */ +#include <stdbool.h> #include "../../../include/linux/perf_event.h" +#include "types.h" struct list_head ;struct perf_evsel ;Signature · 1 line
1.7.8.4
Signature · 2 lines
Advanced Micro Devices, Inc. Operating System Research Center
Re: [PATCH 1/4] perf tools: Fix include header files in util/parse-events.h
Em Tue, Apr 24, 2012 at 06:53:44PM +0200, Robert Richter escreveu:
Quoted 13 lines · Robert Richter
Quoted 12 lines
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’... and
types.h:
util/parse-events.h:28: error: expected ‘)’ before ‘config’util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’util/parse-events.h:45: error: expected ‘)’ before ‘type’
Quoted 8 lines
Quoted 4 lines
This happens if now other include files are included before
util/parse-events.h.Signed-off-by Robert Richter <[email protected]>Arnaldo,
is it your intention that you didn't apply this patch?
Is this on perf/urgent or perf/core? In what system does this happen?
- Arnaldo
Quoted 29 lines
Thanks,
-Robert
Quoted 21 lines
--- tools/perf/util/parse-events.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)tools/perf/util/parse-events.h +2
-0@@ -4,7 +4,9 @@* Parse symbolic events/counts passed in as options: */ +#include <stdbool.h> #include "../../../include/linux/perf_event.h" +#include "types.h" struct list_head ;struct perf_evsel ;Signature · 1 line
1.7.8.4Signature · 2 lines
Advanced Micro Devices, Inc. Operating System Research Center
Re: [PATCH 1/4] perf tools: Fix include header files in util/parse-events.h
Quoted 17 lines · Arnaldo Carvalho de Melo
Em Tue, Apr 24, 2012 at 06:53:44PM +0200, Robert Richter escreveu:
Quoted 16 lines · Robert Richter
Quoted 15 lines
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’... and
types.h:
util/parse-events.h:28: error: expected ‘)’ before ‘config’util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’util/parse-events.h:45: error: expected ‘)’ before ‘type’This happens if now other include files are included before
util/parse-events.h.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Robert Richter <[email protected]>Arnaldo,
is it your intention that you didn't apply this patch?
Is this on perf/urgent or perf/core? In what system does this happen?
Neither, I stumbled over this while working on a modified version of perf tools. The interface definition of util/parse-events.h contains type bool and u64, but both are not defined. Thus, including parse-events.h independently of other header files will raise this compile errors. It took me some time to figure out what happened so I think it is worth fixing it.
-Robert
Signature · 2 lines
Advanced Micro Devices, Inc. Operating System Research Center
Re: [PATCH 1/4] perf tools: Fix include header files in util/parse-events.h
Em Tue, Apr 24, 2012 at 07:58:04PM +0200, Robert Richter escreveu:
Quoted 24 lines · Arnaldo Carvalho de Melo
Quoted 23 lines
Em Tue, Apr 24, 2012 at 06:53:44PM +0200, Robert Richter escreveu:
Quoted 21 lines · Robert Richter
Quoted 8 lines
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’... and
types.h:Quoted 8 lines
util/parse-events.h:28: error: expected ‘)’ before ‘config’util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’util/parse-events.h:45: error: expected ‘)’ before ‘type’This happens if now other include files are included before
util/parse-events.h.Signed-off-by Robert Richter <[email protected]>Arnaldo,
is it your intention that you didn't apply this patch?
Quoted 8 lines
Quoted 1 line
Is this on perf/urgent or perf/core? In what system does this happen?
Neither, I stumbled over this while working on a modified version of perf tools. The interface definition of
util/parse-events.hcontains type bool and u64, but both are not defined. Thus, includingparse-events.hindependently of other header files will raise this compile errors. It took me some time to figure out what happened so I think it is worth fixing it.
Cool, that explains why I have not stumbled on it, thanks for explaining, will apply.
- Arnaldo
[tip:perf/core] perf tools: Fix include header files in util/ parse-events.h
In reply to Robert RichterCommit-ID: c651214e90e5c150015f7524a6bfc298ad61435f Gitweb: http://git.kernel.org/tip/c651214e90e5c150015f7524a6bfc298ad61435f Author: Robert Richter <[email protected]> AuthorDate: Thu, 5 Apr 2012 18:26:25 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 2 May 2012 15:14:35 -0300
perf tools: Fix include header files in util/parse-events.h
Include header fixes for
... bool:
util/parse-events.h:31: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘have_tracepoints’
... and types.h:
util/parse-events.h:28: error: expected ‘)’ before ‘config’ util/parse-events.h:34: error: expected declaration specifiers or ‘...’ before ‘u64’ util/parse-events.h:45: error: expected ‘)’ before ‘type’
This happens if now other include files are included before util/parse-events.h.
--- tools/perf/util/parse-events.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
tools/perf/util/parse-events.h +2 -0
@@ -4,7 +4,9 @@* Parse symbolic events/counts passed in as options: */ +#include <stdbool.h> #include "../../../include/linux/perf_event.h" +#include "types.h" struct list_head ;struct perf_evsel ;
PATCH 2/4 perf tools: Fix thread map that is type pid_t
In reply to Robert RichterThread map is actually type pid_t and not int.
--- tools/perf/util/thread_map.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
tools/perf/util/thread_map.h +1 -1
@@ -6,7 +6,7 @@struct thread_map {int nr; -int map[]; +pid_t map[]; };struct thread_map *thread_map__new_by_pid (pid_t pid);
Signature · 1 line
1.7.8.4
[tip:perf/core] perf tools: Fix thread map that is type pid_t
Commit-ID: 61d5bf5b01a0baf0934a4bd3d15bc14e4bbf202c Gitweb: http://git.kernel.org/tip/61d5bf5b01a0baf0934a4bd3d15bc14e4bbf202c Author: Robert Richter <[email protected]> AuthorDate: Thu, 5 Apr 2012 18:26:26 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 11 Apr 2012 17:38:50 -0300
perf tools: Fix thread map that is type pid_t
Thread map is actually type pid_t and not int.
--- tools/perf/util/thread_map.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
tools/perf/util/thread_map.h +1 -1
@@ -6,7 +6,7 @@struct thread_map {int nr; -int map[]; +pid_t map[]; };struct thread_map *thread_map__new_by_pid (pid_t pid);
PATCH 3/4 perf tools: Declare some references static in builtin-stat.c
In reply to Robert RichterThis references are not exported, use static declaration.
--- tools/perf/builtin-stat.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
tools/perf/builtin-stat.c +13 -13
@@ -173,7 +173,7 @@ static struct perf_event_attr very_very_detailed_attrs[] = { -struct perf_evlist *evsel_list; +static struct perf_evlist *evsel_list;static bool system_wide = false;static int run_idx =0 ;
@@ -265,18 +265,18 @@ static double stddev_stats(struct stats *stats)return sqrt (variance_mean); } -struct stats runtime_nsecs_stats[MAX_NR_CPUS ]; -struct stats runtime_cycles_stats[MAX_NR_CPUS ]; -struct stats runtime_stalled_cycles_front_stats[MAX_NR_CPUS ]; -struct stats runtime_stalled_cycles_back_stats[MAX_NR_CPUS ]; -struct stats runtime_branches_stats[MAX_NR_CPUS ]; -struct stats runtime_cacherefs_stats[MAX_NR_CPUS ]; -struct stats runtime_l1_dcache_stats[MAX_NR_CPUS ]; -struct stats runtime_l1_icache_stats[MAX_NR_CPUS ]; -struct stats runtime_ll_cache_stats[MAX_NR_CPUS ]; -struct stats runtime_itlb_cache_stats[MAX_NR_CPUS ]; -struct stats runtime_dtlb_cache_stats[MAX_NR_CPUS ]; -struct stats walltime_nsecs_stats; +static struct stats runtime_nsecs_stats[MAX_NR_CPUS ]; +static struct stats runtime_cycles_stats[MAX_NR_CPUS ]; +static struct stats runtime_stalled_cycles_front_stats[MAX_NR_CPUS ]; +static struct stats runtime_stalled_cycles_back_stats[MAX_NR_CPUS ]; +static struct stats runtime_branches_stats[MAX_NR_CPUS ]; +static struct stats runtime_cacherefs_stats[MAX_NR_CPUS ]; +static struct stats runtime_l1_dcache_stats[MAX_NR_CPUS ]; +static struct stats runtime_l1_icache_stats[MAX_NR_CPUS ]; +static struct stats runtime_ll_cache_stats[MAX_NR_CPUS ]; +static struct stats runtime_itlb_cache_stats[MAX_NR_CPUS ]; +static struct stats runtime_dtlb_cache_stats[MAX_NR_CPUS ]; +static struct stats walltime_nsecs_stats;static int create_perf_stat_counter (struct perf_evsel *evsel,struct perf_evsel *first)
Signature · 1 line
1.7.8.4
[tip:perf/core] perf stat: Declare some references static
Commit-ID: 666e6d48c57921874008d97aac13f6ee3e24fd55 Gitweb: http://git.kernel.org/tip/666e6d48c57921874008d97aac13f6ee3e24fd55 Author: Robert Richter <[email protected]> AuthorDate: Thu, 5 Apr 2012 18:26:27 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 11 Apr 2012 17:37:16 -0300
perf stat: Declare some references static
This references are not exported, use static declaration.
--- tools/perf/builtin-stat.c | 26 +++++++++++++------------- 1 files changed, 13 insertions(+), 13 deletions(-)
tools/perf/builtin-stat.c +13 -13
@@ -173,7 +173,7 @@ static struct perf_event_attr very_very_detailed_attrs[] = { -struct perf_evlist *evsel_list; +static struct perf_evlist *evsel_list;static bool system_wide = false;static int run_idx =0 ;
@@ -265,18 +265,18 @@ static double stddev_stats(struct stats *stats)return sqrt (variance_mean); } -struct stats runtime_nsecs_stats[MAX_NR_CPUS ]; -struct stats runtime_cycles_stats[MAX_NR_CPUS ]; -struct stats runtime_stalled_cycles_front_stats[MAX_NR_CPUS ]; -struct stats runtime_stalled_cycles_back_stats[MAX_NR_CPUS ]; -struct stats runtime_branches_stats[MAX_NR_CPUS ]; -struct stats runtime_cacherefs_stats[MAX_NR_CPUS ]; -struct stats runtime_l1_dcache_stats[MAX_NR_CPUS ]; -struct stats runtime_l1_icache_stats[MAX_NR_CPUS ]; -struct stats runtime_ll_cache_stats[MAX_NR_CPUS ]; -struct stats runtime_itlb_cache_stats[MAX_NR_CPUS ]; -struct stats runtime_dtlb_cache_stats[MAX_NR_CPUS ]; -struct stats walltime_nsecs_stats; +static struct stats runtime_nsecs_stats[MAX_NR_CPUS ]; +static struct stats runtime_cycles_stats[MAX_NR_CPUS ]; +static struct stats runtime_stalled_cycles_front_stats[MAX_NR_CPUS ]; +static struct stats runtime_stalled_cycles_back_stats[MAX_NR_CPUS ]; +static struct stats runtime_branches_stats[MAX_NR_CPUS ]; +static struct stats runtime_cacherefs_stats[MAX_NR_CPUS ]; +static struct stats runtime_l1_dcache_stats[MAX_NR_CPUS ]; +static struct stats runtime_l1_icache_stats[MAX_NR_CPUS ]; +static struct stats runtime_ll_cache_stats[MAX_NR_CPUS ]; +static struct stats runtime_itlb_cache_stats[MAX_NR_CPUS ]; +static struct stats runtime_dtlb_cache_stats[MAX_NR_CPUS ]; +static struct stats walltime_nsecs_stats;static int create_perf_stat_counter (struct perf_evsel *evsel,struct perf_evsel *first)
PATCH 4/4 perf tools: Use sw counter only if hw pmu is not detected
In reply to Robert RichterUse cpu-clock-tick sw counter for cpu-cycles only if there is no hw pmu available. This is the case if the syscall reports ENOENT. In other cases (e.g. invalid attributes) we don't want the sw counter to be used.
--- tools/perf/builtin-record.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
tools/perf/builtin-record.c +1 -1
@@ -245,7 +245,7 @@ try_again:* based cpu-clock-tick sw counter, which * is always available even if no PMU support: */ -if (attr->type ==PERF_TYPE_HARDWARE +if (err ==ENOENT && attr->type ==PERF_TYPE_HARDWARE && attr->config ==PERF_COUNT_HW_CPU_CYCLES ) {if (verbose)
Signature · 1 line
1.7.8.4
Re: [PATCH 4/4] perf tools: Use sw counter only if hw pmu is not detected
Quoted 4 lines · Robert Richter
Use cpu-clock-tick sw counter for cpu-cycles only if there is no hw pmu available. This is the case if the syscall reports ENOENT. In other cases (e.g. invalid attributes) we don't want the sw counter to be used.
Isn't the invalid attributes case handled by the
'} else if (err == EINVAL) {'
case before this switch is done?
David
Quoted 19 lines
Signed-off-by Robert Richter<[email protected]>--- tools/perf/builtin-record.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)tools/perf/builtin-record.c +1
-1@@ -245,7 +245,7 @@ try_again:* based cpu-clock-tick sw counter, which * is always available even if no PMU support: */ -if (attr->type ==PERF_TYPE_HARDWARE +if (err ==ENOENT && attr->type ==PERF_TYPE_HARDWARE && attr->config ==PERF_COUNT_HW_CPU_CYCLES ) {if (verbose)
Re: [PATCH 4/4] perf tools: Use sw counter only if hw pmu is not detected
Quoted 9 lines · David Ahern
Quoted 4 lines · Robert Richter
Use cpu-clock-tick sw counter for cpu-cycles only if there is no hw pmu available. This is the case if the syscall reports ENOENT. In other cases (e.g. invalid attributes) we don't want the sw counter to be used.
Isn't the invalid attributes case handled by the
'} else if (err == EINVAL) {'
case before this switch is done?
Yes, but it does not jump out the code path. It is only doing some diagnostics and retries the syscall with some modified attributes or so.
I had have the case what a syscall failed due to invalid attributes and then the sw counter was setup. I got odd results and I needed some time debugging this to realize what was going on.
-Robert
Signature · 2 lines
Advanced Micro Devices, Inc. Operating System Research Center
[tip:perf/core] perf record: Use sw counter only if hw pmu is not detected
In reply to Robert RichterCommit-ID: 5a7ed29c7572d00a75e8c4529e30c5ac2ef82271 Gitweb: http://git.kernel.org/tip/5a7ed29c7572d00a75e8c4529e30c5ac2ef82271 Author: Robert Richter <[email protected]> AuthorDate: Thu, 5 Apr 2012 18:26:28 +0200 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Wed, 11 Apr 2012 17:39:19 -0300
perf record: Use sw counter only if hw pmu is not detected
Use cpu-clock-tick sw counter for cpu-cycles only if there is no hw pmu available. This is the case if the syscall reports ENOENT. In other cases (e.g. invalid attributes) we don't want the sw counter to be used.
--- tools/perf/builtin-record.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
tools/perf/builtin-record.c +1 -1
@@ -245,7 +245,7 @@ try_again:* based cpu-clock-tick sw counter, which * is always available even if no PMU support: */ -if (attr->type ==PERF_TYPE_HARDWARE +if (err ==ENOENT && attr->type ==PERF_TYPE_HARDWARE && attr->config ==PERF_COUNT_HW_CPU_CYCLES ) {if (verbose)