* [PATCH 13/21] perf test: fix a build error on builtin-test
2012-11-09 21:42 [GIT PULL 00/21] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2012-11-09 21:43 ` Arnaldo Carvalho de Melo
2012-11-12 2:10 ` [GIT PULL 00/21] perf/core improvements and fixes Namhyung Kim
2012-11-13 18:11 ` Ingo Molnar
2 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-11-09 21:43 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Zheng Liu, Vinson Lee, Zheng Liu, David Ahern,
Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, stable,
Arnaldo Carvalho de Melo
From: Zheng Liu <gnehzuil.liu@gmail.com>
Recently I build perf and get a build error on builtin-test.c. The error is as
following:
$ make
CC perf.o
CC builtin-test.o
cc1: warnings being treated as errors
builtin-test.c: In function ‘sched__get_first_possible_cpu’:
builtin-test.c:977: warning: implicit declaration of function ‘CPU_ALLOC’
builtin-test.c:977: warning: nested extern declaration of ‘CPU_ALLOC’
builtin-test.c:977: warning: assignment makes pointer from integer without a cast
builtin-test.c:978: warning: implicit declaration of function ‘CPU_ALLOC_SIZE’
builtin-test.c:978: warning: nested extern declaration of ‘CPU_ALLOC_SIZE’
builtin-test.c:979: warning: implicit declaration of function ‘CPU_ZERO_S’
builtin-test.c:979: warning: nested extern declaration of ‘CPU_ZERO_S’
builtin-test.c:982: warning: implicit declaration of function ‘CPU_FREE’
builtin-test.c:982: warning: nested extern declaration of ‘CPU_FREE’
builtin-test.c:992: warning: implicit declaration of function ‘CPU_ISSET_S’
builtin-test.c:992: warning: nested extern declaration of ‘CPU_ISSET_S’
builtin-test.c:998: warning: implicit declaration of function ‘CPU_CLR_S’
builtin-test.c:998: warning: nested extern declaration of ‘CPU_CLR_S’
make: *** [builtin-test.o] Error 1
This problem is introduced in 3e7c439a. CPU_ALLOC and related macros are
missing in sched__get_first_possible_cpu function. In 54489c18, commiter
mentioned that CPU_ALLOC has been removed. So CPU_ALLOC calls in this
function are removed to let perf to be built.
Signed-off-by: Vinson Lee <vlee@twitter.com>
Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Vinson Lee <vlee@twitter.com>
Cc: Zheng Liu <wenqing.lz@taobao.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/1352422726-31114-1-git-send-email-vlee@twitter.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/builtin-test.c | 38 ++++++++++++--------------------------
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index b5a544d..5d4354e 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -605,19 +605,13 @@ out_free_threads:
#undef nsyscalls
}
-static int sched__get_first_possible_cpu(pid_t pid, cpu_set_t **maskp,
- size_t *sizep)
+static int sched__get_first_possible_cpu(pid_t pid, cpu_set_t *maskp)
{
- cpu_set_t *mask;
- size_t size;
int i, cpu = -1, nrcpus = 1024;
realloc:
- mask = CPU_ALLOC(nrcpus);
- size = CPU_ALLOC_SIZE(nrcpus);
- CPU_ZERO_S(size, mask);
+ CPU_ZERO(maskp);
- if (sched_getaffinity(pid, size, mask) == -1) {
- CPU_FREE(mask);
+ if (sched_getaffinity(pid, sizeof(*maskp), maskp) == -1) {
if (errno == EINVAL && nrcpus < (1024 << 8)) {
nrcpus = nrcpus << 2;
goto realloc;
@@ -627,19 +621,14 @@ realloc:
}
for (i = 0; i < nrcpus; i++) {
- if (CPU_ISSET_S(i, size, mask)) {
- if (cpu == -1) {
+ if (CPU_ISSET(i, maskp)) {
+ if (cpu == -1)
cpu = i;
- *maskp = mask;
- *sizep = size;
- } else
- CPU_CLR_S(i, size, mask);
+ else
+ CPU_CLR(i, maskp);
}
}
- if (cpu == -1)
- CPU_FREE(mask);
-
return cpu;
}
@@ -654,8 +643,8 @@ static int test__PERF_RECORD(void)
.freq = 10,
.mmap_pages = 256,
};
- cpu_set_t *cpu_mask = NULL;
- size_t cpu_mask_size = 0;
+ cpu_set_t cpu_mask;
+ size_t cpu_mask_size = sizeof(cpu_mask);
struct perf_evlist *evlist = perf_evlist__new(NULL, NULL);
struct perf_evsel *evsel;
struct perf_sample sample;
@@ -719,8 +708,7 @@ static int test__PERF_RECORD(void)
evsel->attr.sample_type |= PERF_SAMPLE_TIME;
perf_evlist__config_attrs(evlist, &opts);
- err = sched__get_first_possible_cpu(evlist->workload.pid, &cpu_mask,
- &cpu_mask_size);
+ err = sched__get_first_possible_cpu(evlist->workload.pid, &cpu_mask);
if (err < 0) {
pr_debug("sched__get_first_possible_cpu: %s\n", strerror(errno));
goto out_delete_evlist;
@@ -731,9 +719,9 @@ static int test__PERF_RECORD(void)
/*
* So that we can check perf_sample.cpu on all the samples.
*/
- if (sched_setaffinity(evlist->workload.pid, cpu_mask_size, cpu_mask) < 0) {
+ if (sched_setaffinity(evlist->workload.pid, cpu_mask_size, &cpu_mask) < 0) {
pr_debug("sched_setaffinity: %s\n", strerror(errno));
- goto out_free_cpu_mask;
+ goto out_delete_evlist;
}
/*
@@ -917,8 +905,6 @@ found_exit:
}
out_err:
perf_evlist__munmap(evlist);
-out_free_cpu_mask:
- CPU_FREE(cpu_mask);
out_delete_evlist:
perf_evlist__delete(evlist);
out:
--
1.7.9.2.358.g22243
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [GIT PULL 00/21] perf/core improvements and fixes
2012-11-09 21:42 [GIT PULL 00/21] perf/core improvements and fixes Arnaldo Carvalho de Melo
2012-11-09 21:43 ` [PATCH 13/21] perf test: fix a build error on builtin-test Arnaldo Carvalho de Melo
@ 2012-11-12 2:10 ` Namhyung Kim
2012-11-12 13:55 ` Jiri Olsa
2012-11-13 18:11 ` Ingo Molnar
2 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2012-11-12 2:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, linux-kernel, Andi Kleen, Corey Ashford, David Ahern,
Frederic Weisbecker, Irina Tirdea, Jiri Olsa, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, stable, Stephane Eranian,
Steven Rostedt, Vinson Lee, Zheng Liu, acme,
Arnaldo Carvalho de Melo
Hi Arnaldo,
On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> Please consider pulling.
>
> - Arnaldo
>
> The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
>
> perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
>
> for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
>
> tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> . Add a 'link' method for hists, so that we can have the leader with
> buckets for all the entries in all the hists. This new method
> is now used in the default 'diff' output, making the sum of the 'baseline'
> column be 100%, eliminating blind spots. Now we need to use this
> for 'diff' with > 2 perf.data files and for multi event 'report' and
> 'annotate'.
I'm not sure it can be used for group report at least in its current
form. IIUC it connects multiple hist entries using a list head and
create a dummy entry in the leader if need be. But it didn't handle
non-leader entries so it's hard to tell which is which if less entries
are present only. For example consider following case:
leader member1 member2
A A A
B
C
D
where leader, member1 and member2 are evsel/hists and A, B, C and D are
hist entries. After 'linking' the entries the leader will have
following linkage:
leader
A -> A -> A
B
C (dummy) -> C
D (dummy) -> D
In this case, for entry A the leader can determine which entry came from
which hists by looking its order in the list. For entry B the leader
can use zero value for them since the list is empty. However for
entries C and D, it cannot know which one is the right hists unless it
records a hist index or creates dummy entry and insert it in a correct
order (looks far from an optimal solution). Am I missing something?
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL 00/21] perf/core improvements and fixes
2012-11-12 2:10 ` [GIT PULL 00/21] perf/core improvements and fixes Namhyung Kim
@ 2012-11-12 13:55 ` Jiri Olsa
2012-11-12 16:01 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2012-11-12 13:55 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel, Andi Kleen,
Corey Ashford, David Ahern, Frederic Weisbecker, Irina Tirdea,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, stable,
Stephane Eranian, Steven Rostedt, Vinson Lee, Zheng Liu, acme,
Arnaldo Carvalho de Melo
On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
> Hi Arnaldo,
>
> On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> > Hi Ingo,
> >
> > Please consider pulling.
> >
> > - Arnaldo
> >
> > The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
> >
> > perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
> >
> > for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
> >
> > tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
> >
> > ----------------------------------------------------------------
> > perf/core improvements and fixes:
> >
> > . Add a 'link' method for hists, so that we can have the leader with
> > buckets for all the entries in all the hists. This new method
> > is now used in the default 'diff' output, making the sum of the 'baseline'
> > column be 100%, eliminating blind spots. Now we need to use this
> > for 'diff' with > 2 perf.data files and for multi event 'report' and
> > 'annotate'.
>
> I'm not sure it can be used for group report at least in its current
> form. IIUC it connects multiple hist entries using a list head and
> create a dummy entry in the leader if need be. But it didn't handle
> non-leader entries so it's hard to tell which is which if less entries
> are present only. For example consider following case:
>
> leader member1 member2
> A A A
> B
> C
> D
>
> where leader, member1 and member2 are evsel/hists and A, B, C and D are
> hist entries. After 'linking' the entries the leader will have
> following linkage:
>
> leader
> A -> A -> A
> B
> C (dummy) -> C
> D (dummy) -> D
>
> In this case, for entry A the leader can determine which entry came from
> which hists by looking its order in the list. For entry B the leader
> can use zero value for them since the list is empty. However for
> entries C and D, it cannot know which one is the right hists unless it
> records a hist index or creates dummy entry and insert it in a correct
> order (looks far from an optimal solution). Am I missing something?
there's hists pointer in hist_entry if that's what you look for
jirka
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL 00/21] perf/core improvements and fixes
2012-11-12 13:55 ` Jiri Olsa
@ 2012-11-12 16:01 ` Arnaldo Carvalho de Melo
2012-11-13 1:20 ` Namhyung Kim
0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-11-12 16:01 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, Ingo Molnar, linux-kernel, Andi Kleen,
Corey Ashford, David Ahern, Frederic Weisbecker, Irina Tirdea,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, stable,
Stephane Eranian, Steven Rostedt, Vinson Lee, Zheng Liu
Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu:
> On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
> > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> > > . Add a 'link' method for hists, so that we can have the leader with
> > > buckets for all the entries in all the hists. This new method
> > > is now used in the default 'diff' output, making the sum of the 'baseline'
> > > column be 100%, eliminating blind spots. Now we need to use this
> > > for 'diff' with > 2 perf.data files and for multi event 'report' and
> > > 'annotate'.
> > I'm not sure it can be used for group report at least in its current
> > form. IIUC it connects multiple hist entries using a list head and
> > create a dummy entry in the leader if need be. But it didn't handle
> > non-leader entries so it's hard to tell which is which if less entries
> > are present only. For example consider following case:
> > leader member1 member2
> > A A A
> > B
> > C
> > D
> > where leader, member1 and member2 are evsel/hists and A, B, C and D are
> > hist entries. After 'linking' the entries the leader will have
> > following linkage:
> > leader
> > A -> A -> A
> > B
> > C (dummy) -> C
> > D (dummy) -> D
> > In this case, for entry A the leader can determine which entry came from
> > which hists by looking its order in the list. For entry B the leader
> > can use zero value for them since the list is empty. However for
> > entries C and D, it cannot know which one is the right hists unless it
> > records a hist index or creates dummy entry and insert it in a correct
> > order (looks far from an optimal solution). Am I missing something?
> there's hists pointer in hist_entry if that's what you look for
And from there to evsel->idx. In your patchset you even introduce
hists_2_evsel(), right?
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL 00/21] perf/core improvements and fixes
2012-11-12 16:01 ` Arnaldo Carvalho de Melo
@ 2012-11-13 1:20 ` Namhyung Kim
0 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2012-11-13 1:20 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Ingo Molnar, linux-kernel, Andi Kleen, Corey Ashford,
David Ahern, Frederic Weisbecker, Irina Tirdea, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, stable, Stephane Eranian,
Steven Rostedt, Vinson Lee, Zheng Liu
On Mon, 12 Nov 2012 13:01:39 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu:
>> On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
>> > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
>> > > . Add a 'link' method for hists, so that we can have the leader with
>> > > buckets for all the entries in all the hists. This new method
>> > > is now used in the default 'diff' output, making the sum of the 'baseline'
>> > > column be 100%, eliminating blind spots. Now we need to use this
>> > > for 'diff' with > 2 perf.data files and for multi event 'report' and
>> > > 'annotate'.
>
>> > I'm not sure it can be used for group report at least in its current
>> > form. IIUC it connects multiple hist entries using a list head and
>> > create a dummy entry in the leader if need be. But it didn't handle
>> > non-leader entries so it's hard to tell which is which if less entries
>> > are present only. For example consider following case:
>
>> > leader member1 member2
>> > A A A
>> > B
>> > C
>> > D
>
>> > where leader, member1 and member2 are evsel/hists and A, B, C and D are
>> > hist entries. After 'linking' the entries the leader will have
>> > following linkage:
>
>> > leader
>> > A -> A -> A
>> > B
>> > C (dummy) -> C
>> > D (dummy) -> D
>
>> > In this case, for entry A the leader can determine which entry came from
>> > which hists by looking its order in the list. For entry B the leader
>> > can use zero value for them since the list is empty. However for
>> > entries C and D, it cannot know which one is the right hists unless it
>> > records a hist index or creates dummy entry and insert it in a correct
>> > order (looks far from an optimal solution). Am I missing something?
>
>> there's hists pointer in hist_entry if that's what you look for
>
> And from there to evsel->idx. In your patchset you even introduce
> hists_2_evsel(), right?
Ah, okay. I worried about a possiblity of non-consecutive event groups
for some reason, but that's not gonna happen in the future?
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL 00/21] perf/core improvements and fixes
2012-11-09 21:42 [GIT PULL 00/21] perf/core improvements and fixes Arnaldo Carvalho de Melo
2012-11-09 21:43 ` [PATCH 13/21] perf test: fix a build error on builtin-test Arnaldo Carvalho de Melo
2012-11-12 2:10 ` [GIT PULL 00/21] perf/core improvements and fixes Namhyung Kim
@ 2012-11-13 18:11 ` Ingo Molnar
2 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2012-11-13 18:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Andi Kleen, Corey Ashford, David Ahern,
Frederic Weisbecker, Irina Tirdea, Jiri Olsa, Mike Galbraith,
Namhyung Kim, Paul Mackerras, Peter Zijlstra, stable,
Stephane Eranian, Steven Rostedt, Vinson Lee, Zheng Liu, acme,
Arnaldo Carvalho de Melo
* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> Hi Ingo,
>
> Please consider pulling.
>
> - Arnaldo
>
> The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
>
> perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
>
> for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
>
> tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> . Add a 'link' method for hists, so that we can have the leader with
> buckets for all the entries in all the hists. This new method
> is now used in the default 'diff' output, making the sum of the 'baseline'
> column be 100%, eliminating blind spots. Now we need to use this
> for 'diff' with > 2 perf.data files and for multi event 'report' and
> 'annotate'.
>
> . libtraceevent fixes for compiler warnings trying to make perf it build
> on some distros, like fedora 14, 32-bit, some of the warnings really
> pointed to real bugs.
>
> . Remove temp dir on failure in 'perf test', fix from Jiri Olsa.
>
> . Fixes for handling data, stack mmaps, from Namhyung Kim.
>
> . Fix live annotation bug related to recent objdump lookup patches, from
> Namhyung Kim
>
> . Don't try to follow jump target on PLT symbols in the annotation browser,
> fix from Namhyung Kim.
>
> . Fix leak on hist_entry delete, from Namhyung Kim.
>
> . Fix a CPU_ALLOC related build error on builtin-test, from Zheng Liu.
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> Andi Kleen (1):
> perf tools: Add arbitary aliases and support names with -
>
> Arnaldo Carvalho de Melo (10):
> perf diff: Start moving to support matching more than two hists
> perf diff: Move hists__match to the hists lib
> perf hists: Introduce hists__link
> perf diff: Use hists__link when not pairing just with baseline
> perf machine: Move more methods to machine.[ch]
> tools lib traceevent: Add __maybe_unused to unused parameters
> tools lib traceevent: Avoid comparisions between signed/unsigned
> tools lib traceevent: No need to check for < 0 on an unsigned enum
> tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror
> tools lib traceevent: Use 'const' in variables pointing to const strings
>
> Jiri Olsa (2):
> perf tests: Move attr.py temp dir cleanup into finally section
> perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw
>
> Namhyung Kim (7):
> perf machine: Set kernel data mapping length
> perf tools: Fix detection of stack area
> perf hists: Free branch_info when freeing hist_entry
> perf tools: Don't try to lookup objdump for live mode
> perf annotate: Whitespace fixups
> perf annotate: Don't try to follow jump target on PLT symbols
> perf annotate: Merge same lines in summary view
>
> Zheng Liu (1):
> perf test: fix a build error on builtin-test
>
> tools/lib/traceevent/event-parse.c | 22 ++--
> tools/perf/Makefile | 12 ++-
> tools/perf/arch/common.c | 7 ++
> tools/perf/builtin-diff.c | 48 ++-------
> tools/perf/tests/attr.py | 30 +++---
> tools/perf/tests/builtin-test.c | 39 +++----
> tools/perf/tests/dso-data.c | 1 +
> tools/perf/ui/browsers/annotate.c | 12 +++
> tools/perf/ui/hist.c | 10 +-
> tools/perf/util/annotate.c | 69 ++++++++++--
> tools/perf/util/annotate.h | 1 +
> tools/perf/util/dso.c | 1 +
> tools/perf/util/hist.c | 100 ++++++++++++++++++
> tools/perf/util/hist.h | 3 +
> tools/perf/util/machine.c | 205 ++++++++++++++++++++++++++++++++++--
> tools/perf/util/machine.h | 131 ++++++++++++++++++++++-
> tools/perf/util/map.c | 181 +------------------------------
> tools/perf/util/map.h | 93 ----------------
> tools/perf/util/parse-events.l | 2 +
> tools/perf/util/session.h | 5 +-
> tools/perf/util/sort.h | 27 ++++-
> tools/perf/util/symbol.c | 1 +
> tools/perf/util/symbol.h | 20 ----
> 23 files changed, 604 insertions(+), 416 deletions(-)
Pulled, thanks Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread