* [GIT PULL] perf tools fixes
@ 2011-05-23 11:06 Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Frederic Weisbecker @ 2011-05-23 11:06 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Peter Zijlstra,
Arnaldo Carvalho de Melo, Stephane Eranian
Ingo,
Please pull the perf/urgent branch that can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
perf/urgent
Thanks,
Frederic
---
Frederic Weisbecker (2):
perf tools: Fix sample size bit operations
perf tools: Fix ommitted mmap data update on remap
tools/perf/util/event.c | 2 +-
tools/perf/util/session.c | 39 ++++++++++++++++++++++++++-------------
2 files changed, 27 insertions(+), 14 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] perf tools: Fix sample size bit operations
2011-05-23 11:06 [GIT PULL] perf tools fixes Frederic Weisbecker
@ 2011-05-23 11:06 ` Frederic Weisbecker
2011-05-23 11:28 ` Ingo Molnar
2011-05-23 11:31 ` [tip:perf/urgent] " tip-bot for Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 2/2] perf tools: Fix ommitted mmap data update on remap Frederic Weisbecker
2011-05-23 11:13 ` [GIT PULL] perf tools fixes Ingo Molnar
2 siblings, 2 replies; 9+ messages in thread
From: Frederic Weisbecker @ 2011-05-23 11:06 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Stephane Eranian
What we want is to count the number of bits in the mask,
not some other random operation written in the middle
of the night.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
tools/perf/util/event.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17c1c3c..d3fa7e4 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -42,7 +42,7 @@ int perf_sample_size(u64 sample_type)
int i;
for (i = 0; i < 64; i++) {
- if ((mask << i) & 1)
+ if (mask & (1 << i))
size++;
}
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] perf tools: Fix ommitted mmap data update on remap
2011-05-23 11:06 [GIT PULL] perf tools fixes Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
@ 2011-05-23 11:06 ` Frederic Weisbecker
2011-05-23 11:30 ` [tip:perf/urgent] " tip-bot for Frederic Weisbecker
2011-05-23 11:13 ` [GIT PULL] perf tools fixes Ingo Molnar
2 siblings, 1 reply; 9+ messages in thread
From: Frederic Weisbecker @ 2011-05-23 11:06 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
Arnaldo Carvalho de Melo, Stephane Eranian
Commit eac9eacee16
"perf tools: Check we are able to read the event size on mmap"
brought a check to ensure we can read the size of the event
before dereferencing it, and do a remap otherwise to move
the buffer forward.
However that remap was ommitting all the necessary work to update
the new page offset, head, and to unmap previous pages, etc...
To fix this, gather all the code that fetches the event in a
seperate helper which does all the necessary checks about the
header/event size and tells us anytime a remap is needed.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
---
tools/perf/util/session.c | 39 ++++++++++++++++++++++++++-------------
1 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 948327d..64500fc 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -960,6 +960,30 @@ out_err:
return err;
}
+static union perf_event *
+fetch_mmaped_event(struct perf_session *session,
+ u64 head, size_t mmap_size, char *buf)
+{
+ union perf_event *event;
+
+ /*
+ * Ensure we have enough space remaining to read
+ * the size of the event in the headers.
+ */
+ if (head + sizeof(event->header) > mmap_size)
+ return NULL;
+
+ event = (union perf_event *)(buf + head);
+
+ if (session->header.needs_swap)
+ perf_event_header__bswap(&event->header);
+
+ if (head + event->header.size > mmap_size)
+ return NULL;
+
+ return event;
+}
+
int __perf_session__process_events(struct perf_session *session,
u64 data_offset, u64 data_size,
u64 file_size, struct perf_event_ops *ops)
@@ -1014,19 +1038,8 @@ remap:
file_pos = file_offset + head;
more:
- /*
- * Ensure we have enough space remaining to read
- * the size of the event in the headers.
- */
- if (head + sizeof(event->header) > mmap_size)
- goto remap;
-
- event = (union perf_event *)(buf + head);
-
- if (session->header.needs_swap)
- perf_event_header__bswap(&event->header);
-
- if (head + event->header.size > mmap_size) {
+ event = fetch_mmaped_event(session, head, mmap_size, buf);
+ if (!event) {
if (mmaps[map_idx]) {
munmap(mmaps[map_idx], mmap_size);
mmaps[map_idx] = NULL;
--
1.7.3.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [GIT PULL] perf tools fixes
2011-05-23 11:06 [GIT PULL] perf tools fixes Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 2/2] perf tools: Fix ommitted mmap data update on remap Frederic Weisbecker
@ 2011-05-23 11:13 ` Ingo Molnar
2011-05-23 11:18 ` Ingo Molnar
2 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2011-05-23 11:13 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> Ingo,
>
> Please pull the perf/urgent branch that can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> perf/urgent
>
> Thanks,
> Frederic
> ---
>
> Frederic Weisbecker (2):
> perf tools: Fix sample size bit operations
> perf tools: Fix ommitted mmap data update on remap
>
>
> tools/perf/util/event.c | 2 +-
> tools/perf/util/session.c | 39 ++++++++++++++++++++++++++-------------
> 2 files changed, 27 insertions(+), 14 deletions(-)
Pulled, thanks a lot Frederic!
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GIT PULL] perf tools fixes
2011-05-23 11:13 ` [GIT PULL] perf tools fixes Ingo Molnar
@ 2011-05-23 11:18 ` Ingo Molnar
0 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2011-05-23 11:18 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian
* Ingo Molnar <mingo@elte.hu> wrote:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> > Ingo,
> >
> > Please pull the perf/urgent branch that can be found at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> > perf/urgent
> >
> > Thanks,
> > Frederic
> > ---
> >
> > Frederic Weisbecker (2):
> > perf tools: Fix sample size bit operations
> > perf tools: Fix ommitted mmap data update on remap
> >
> >
> > tools/perf/util/event.c | 2 +-
> > tools/perf/util/session.c | 39 ++++++++++++++++++++++++++-------------
> > 2 files changed, 27 insertions(+), 14 deletions(-)
Hm, you broke perf top:
PerfTop: 0 irqs/sec kernel:-nan% exact: -nan% [1000Hz cycles], (all, 16 CPUs)
-------------------------------------------------------------------------------------------------------
samples pcnt DSO
_______ _____
Can't parse sample, err = -14
Can't parse sample, err = -14
Can't parse sample, err = -14
Can't parse sample, err = -14
Can't parse sample, err = -14
Can't parse sample, err = -14
and perf record+report as well:
aldebaran:~/linux/linux> perf record -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.119 MB perf.data (~5201 samples) ]
aldebaran:~/linux/linux> perf report --stdio
The perf.data file has no samples!
so i'm unpulling these bits.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf tools: Fix sample size bit operations
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
@ 2011-05-23 11:28 ` Ingo Molnar
2011-05-23 12:39 ` Frederic Weisbecker
2011-05-23 11:31 ` [tip:perf/urgent] " tip-bot for Frederic Weisbecker
1 sibling, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2011-05-23 11:28 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian
* Frederic Weisbecker <fweisbec@gmail.com> wrote:
> What we want is to count the number of bits in the mask,
> not some other random operation written in the middle
> of the night.
>
> Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Stephane Eranian <eranian@google.com>
> ---
> tools/perf/util/event.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 17c1c3c..d3fa7e4 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -42,7 +42,7 @@ int perf_sample_size(u64 sample_type)
> int i;
>
> for (i = 0; i < 64; i++) {
> - if ((mask << i) & 1)
> + if (mask & (1 << i))
> size++;
> }
I fixed this to be 1UL and applied your fixes out of email - perf top and perf
report works fine now, so this was a 64-bitness bug (you probably used a 32-bit
system for testing?).
Btw., shouldnt this use hweight() or such?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [tip:perf/urgent] perf tools: Fix ommitted mmap data update on remap
2011-05-23 11:06 ` [PATCH 2/2] perf tools: Fix ommitted mmap data update on remap Frederic Weisbecker
@ 2011-05-23 11:30 ` tip-bot for Frederic Weisbecker
0 siblings, 0 replies; 9+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2011-05-23 11:30 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, eranian, acme, hpa, mingo, a.p.zijlstra, fweisbec,
tglx, mingo
Commit-ID: 998bedc8c56c6869de457c845cbd328592e5e82e
Gitweb: http://git.kernel.org/tip/998bedc8c56c6869de457c845cbd328592e5e82e
Author: Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 23 May 2011 13:06:28 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 23 May 2011 13:22:57 +0200
perf tools: Fix ommitted mmap data update on remap
Commit eac9eacee16 "perf tools: Check we are able to read the event
size on mmap" brought a check to ensure we can read the size of the
event before dereferencing it, and do a remap otherwise to move the
buffer forward.
However that remap was ommitting all the necessary work to
update the new page offset, head, and to unmap previous pages,
etc...
To fix this, gather all the code that fetches the event in a
seperate helper which does all the necessary checks about the
header/event size and tells us anytime a remap is needed.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1306148788-6179-3-git-send-email-fweisbec@gmail.com
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/util/session.c | 39 ++++++++++++++++++++++++++-------------
1 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 948327d..64500fc 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -960,6 +960,30 @@ out_err:
return err;
}
+static union perf_event *
+fetch_mmaped_event(struct perf_session *session,
+ u64 head, size_t mmap_size, char *buf)
+{
+ union perf_event *event;
+
+ /*
+ * Ensure we have enough space remaining to read
+ * the size of the event in the headers.
+ */
+ if (head + sizeof(event->header) > mmap_size)
+ return NULL;
+
+ event = (union perf_event *)(buf + head);
+
+ if (session->header.needs_swap)
+ perf_event_header__bswap(&event->header);
+
+ if (head + event->header.size > mmap_size)
+ return NULL;
+
+ return event;
+}
+
int __perf_session__process_events(struct perf_session *session,
u64 data_offset, u64 data_size,
u64 file_size, struct perf_event_ops *ops)
@@ -1014,19 +1038,8 @@ remap:
file_pos = file_offset + head;
more:
- /*
- * Ensure we have enough space remaining to read
- * the size of the event in the headers.
- */
- if (head + sizeof(event->header) > mmap_size)
- goto remap;
-
- event = (union perf_event *)(buf + head);
-
- if (session->header.needs_swap)
- perf_event_header__bswap(&event->header);
-
- if (head + event->header.size > mmap_size) {
+ event = fetch_mmaped_event(session, head, mmap_size, buf);
+ if (!event) {
if (mmaps[map_idx]) {
munmap(mmaps[map_idx], mmap_size);
mmaps[map_idx] = NULL;
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [tip:perf/urgent] perf tools: Fix sample size bit operations
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
2011-05-23 11:28 ` Ingo Molnar
@ 2011-05-23 11:31 ` tip-bot for Frederic Weisbecker
1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Frederic Weisbecker @ 2011-05-23 11:31 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, eranian, acme, hpa, mingo, a.p.zijlstra, fweisbec,
tglx, mingo
Commit-ID: 3cb6d1540880e767d911b79eb49578de2190f428
Gitweb: http://git.kernel.org/tip/3cb6d1540880e767d911b79eb49578de2190f428
Author: Frederic Weisbecker <fweisbec@gmail.com>
AuthorDate: Mon, 23 May 2011 13:06:27 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 23 May 2011 13:26:36 +0200
perf tools: Fix sample size bit operations
What we want is to count the number of bits in the mask,
not some other random operation written in the middle
of the night.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1306148788-6179-2-git-send-email-fweisbec@gmail.com
[ Fixed perf_event__names[] alignment which was nearby and hurting my eyes ... ]
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/util/event.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17c1c3c..252b72a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -9,21 +9,21 @@
#include "thread_map.h"
static const char *perf_event__names[] = {
- [0] = "TOTAL",
- [PERF_RECORD_MMAP] = "MMAP",
- [PERF_RECORD_LOST] = "LOST",
- [PERF_RECORD_COMM] = "COMM",
- [PERF_RECORD_EXIT] = "EXIT",
- [PERF_RECORD_THROTTLE] = "THROTTLE",
- [PERF_RECORD_UNTHROTTLE] = "UNTHROTTLE",
- [PERF_RECORD_FORK] = "FORK",
- [PERF_RECORD_READ] = "READ",
- [PERF_RECORD_SAMPLE] = "SAMPLE",
- [PERF_RECORD_HEADER_ATTR] = "ATTR",
- [PERF_RECORD_HEADER_EVENT_TYPE] = "EVENT_TYPE",
- [PERF_RECORD_HEADER_TRACING_DATA] = "TRACING_DATA",
- [PERF_RECORD_HEADER_BUILD_ID] = "BUILD_ID",
- [PERF_RECORD_FINISHED_ROUND] = "FINISHED_ROUND",
+ [0] = "TOTAL",
+ [PERF_RECORD_MMAP] = "MMAP",
+ [PERF_RECORD_LOST] = "LOST",
+ [PERF_RECORD_COMM] = "COMM",
+ [PERF_RECORD_EXIT] = "EXIT",
+ [PERF_RECORD_THROTTLE] = "THROTTLE",
+ [PERF_RECORD_UNTHROTTLE] = "UNTHROTTLE",
+ [PERF_RECORD_FORK] = "FORK",
+ [PERF_RECORD_READ] = "READ",
+ [PERF_RECORD_SAMPLE] = "SAMPLE",
+ [PERF_RECORD_HEADER_ATTR] = "ATTR",
+ [PERF_RECORD_HEADER_EVENT_TYPE] = "EVENT_TYPE",
+ [PERF_RECORD_HEADER_TRACING_DATA] = "TRACING_DATA",
+ [PERF_RECORD_HEADER_BUILD_ID] = "BUILD_ID",
+ [PERF_RECORD_FINISHED_ROUND] = "FINISHED_ROUND",
};
const char *perf_event__name(unsigned int id)
@@ -42,7 +42,7 @@ int perf_sample_size(u64 sample_type)
int i;
for (i = 0; i < 64; i++) {
- if ((mask << i) & 1)
+ if (mask & (1UL << i))
size++;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] perf tools: Fix sample size bit operations
2011-05-23 11:28 ` Ingo Molnar
@ 2011-05-23 12:39 ` Frederic Weisbecker
0 siblings, 0 replies; 9+ messages in thread
From: Frederic Weisbecker @ 2011-05-23 12:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Peter Zijlstra, Arnaldo Carvalho de Melo, Stephane Eranian
On Mon, May 23, 2011 at 01:28:15PM +0200, Ingo Molnar wrote:
>
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> > What we want is to count the number of bits in the mask,
> > not some other random operation written in the middle
> > of the night.
> >
> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Stephane Eranian <eranian@google.com>
> > ---
> > tools/perf/util/event.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> > index 17c1c3c..d3fa7e4 100644
> > --- a/tools/perf/util/event.c
> > +++ b/tools/perf/util/event.c
> > @@ -42,7 +42,7 @@ int perf_sample_size(u64 sample_type)
> > int i;
> >
> > for (i = 0; i < 64; i++) {
> > - if ((mask << i) & 1)
> > + if (mask & (1 << i))
> > size++;
> > }
>
> I fixed this to be 1UL and applied your fixes out of email - perf top and perf
> report works fine now, so this was a 64-bitness bug (you probably used a 32-bit
> system for testing?).
Nope I was using a 64 machine for testing, but I only tested sched_switch
events with callchains on perf report and perf script.
Perhaps that bug triggered only on perf top or with default cpu-cycle events.
>
> Btw., shouldnt this use hweight() or such?
A real proper way would better have that yeah :)
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-05-23 12:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-23 11:06 [GIT PULL] perf tools fixes Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 1/2] perf tools: Fix sample size bit operations Frederic Weisbecker
2011-05-23 11:28 ` Ingo Molnar
2011-05-23 12:39 ` Frederic Weisbecker
2011-05-23 11:31 ` [tip:perf/urgent] " tip-bot for Frederic Weisbecker
2011-05-23 11:06 ` [PATCH 2/2] perf tools: Fix ommitted mmap data update on remap Frederic Weisbecker
2011-05-23 11:30 ` [tip:perf/urgent] " tip-bot for Frederic Weisbecker
2011-05-23 11:13 ` [GIT PULL] perf tools fixes Ingo Molnar
2011-05-23 11:18 ` Ingo Molnar
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.