[PATCH] perf/trace: use read() instead of lseek() in trace_event_read.c:skip()

[PATCH] perf/trace: use read() instead of lseek() in trace_event_read.c:skip()

[Tom Zanussi]

Hi,

Here's a small fix for a problem affecting live-mode, introduced in the
past day or so:

root@tropicana:~# perf trace rwtop
perf trace started with Perl
script /root/libexec/perf-core/scripts/perl/rwtop.pl

  Fatal: did not read header event

commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
function to skip over e.g. header_page, but this doesn't work for live
mode.  This patch re-implements skip() to use read() instead of
lseek() to fix that.

Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
---
 tools/perf/util/trace-event-read.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index cb54cd0..f55cc3a 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -53,12 +53,6 @@ static unsigned long	page_size;
 static ssize_t calc_data_size;
 static bool repipe;
 
-/* If it fails, the next read will report it */
-static void skip(int size)
-{
-	lseek(input_fd, size, SEEK_CUR);
-}
-
 static int do_read(int fd, void *buf, int size)
 {
 	int rsize = size;
@@ -98,6 +92,19 @@ static int read_or_die(void *data, int size)
 	return r;
 }
 
+/* If it fails, the next read will report it */
+static void skip(int size)
+{
+	char buf[BUFSIZ];
+	int r;
+
+	while (size) {
+		r = size > BUFSIZ ? BUFSIZ : size;
+		read_or_die(buf, r);
+		size -= r;
+	};
+}
+
 static unsigned int read4(void)
 {
 	unsigned int data;
-- 
1.6.4.GIT

Re: [PATCH] perf/trace: use read() instead of lseek() in trace_event_read.c:skip()

[Borislav Petkov]

From: Tom Zanussi <tzanussi@gmail.com>
Date: Tue, May 04, 2010 at 11:02:10PM -0500

Hi,

> Here's a small fix for a problem affecting live-mode, introduced in the
> past day or so:
> 
> root@tropicana:~# perf trace rwtop
> perf trace started with Perl
> script /root/libexec/perf-core/scripts/perl/rwtop.pl
> 
>   Fatal: did not read header event
> 
> commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
> function to skip over e.g. header_page, but this doesn't work for live
> mode.  This patch re-implements skip() to use read() instead of
> lseek() to fix that.
> 
> Signed-off-by: Tom Zanussi <tzanussi@gmail.com>

has this one landed in any tree in the meantime?

tip/perf/core doesn't have it.

-- 
Regards/Gruss,
Boris.

Operating Systems Research Center
Advanced Micro Devices, Inc.

Re: [PATCH] perf/trace: use read() instead of lseek() in trace_event_read.c:skip()

[Frederic Weisbecker]

On Fri, May 14, 2010 at 06:12:55PM +0200, Borislav Petkov wrote:
> From: Tom Zanussi <tzanussi@gmail.com>
> Date: Tue, May 04, 2010 at 11:02:10PM -0500
> 
> Hi,
> 
> > Here's a small fix for a problem affecting live-mode, introduced in the
> > past day or so:
> > 
> > root@tropicana:~# perf trace rwtop
> > perf trace started with Perl
> > script /root/libexec/perf-core/scripts/perl/rwtop.pl
> > 
> >   Fatal: did not read header event
> > 
> > commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
> > function to skip over e.g. header_page, but this doesn't work for live
> > mode.  This patch re-implements skip() to use read() instead of
> > lseek() to fix that.
> > 
> > Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
> 
> has this one landed in any tree in the meantime?
> 
> tip/perf/core doesn't have it.


I forgot to queue it, my bad.

Queued, thanks.

[tip:perf/core] perf: Use read() instead of lseek() in trace_event_read.c:skip()

[tip-bot for Tom Zanussi]

Commit-ID:  cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
Gitweb:     http://git.kernel.org/tip/cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
Author:     Tom Zanussi <tzanussi@gmail.com>
AuthorDate: Tue, 4 May 2010 23:02:10 -0500
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Thu, 20 May 2010 08:37:17 +0200

perf: Use read() instead of lseek() in trace_event_read.c:skip()

This is a small fix for a problem affecting live-mode, introduced
recently:

root@tropicana:~# perf trace rwtop
perf trace started with Perl
script /root/libexec/perf-core/scripts/perl/rwtop.pl

  Fatal: did not read header event

commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
function to skip over e.g. header_page, but this doesn't work for
live mode.  This patch re-implements skip() to use read() instead of
lseek() to fix that.

Signed-off-by: Tom Zanussi <tzanussi@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: Paul Mackerras <paulus@samba.org>
LKML-Reference: <1273032130.6383.28.camel@tropicana>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
---
 tools/perf/util/trace-event-read.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index cb54cd0..f55cc3a 100644
--- a/tools/perf/util/trace-event-read.c
+++ b/tools/perf/util/trace-event-read.c
@@ -53,12 +53,6 @@ static unsigned long	page_size;
 static ssize_t calc_data_size;
 static bool repipe;
 
-/* If it fails, the next read will report it */
-static void skip(int size)
-{
-	lseek(input_fd, size, SEEK_CUR);
-}
-
 static int do_read(int fd, void *buf, int size)
 {
 	int rsize = size;
@@ -98,6 +92,19 @@ static int read_or_die(void *data, int size)
 	return r;
 }
 
+/* If it fails, the next read will report it */
+static void skip(int size)
+{
+	char buf[BUFSIZ];
+	int r;
+
+	while (size) {
+		r = size > BUFSIZ ? BUFSIZ : size;
+		read_or_die(buf, r);
+		size -= r;
+	};
+}
+
 static unsigned int read4(void)
 {
 	unsigned int data;

Re: [tip:perf/core] perf: Use read() instead of lseek() in trace_event_read.c:skip()

[Pekka Enberg]

On Thu, May 20, 2010 at 3:54 PM, tip-bot for Tom Zanussi
<tzanussi@gmail.com> wrote:
> Commit-ID:  cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
> Gitweb:     http://git.kernel.org/tip/cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
> Author:     Tom Zanussi <tzanussi@gmail.com>
> AuthorDate: Tue, 4 May 2010 23:02:10 -0500
> Committer:  Frederic Weisbecker <fweisbec@gmail.com>
> CommitDate: Thu, 20 May 2010 08:37:17 +0200
>
> perf: Use read() instead of lseek() in trace_event_read.c:skip()
>
> This is a small fix for a problem affecting live-mode, introduced
> recently:
>
> root@tropicana:~# perf trace rwtop
> perf trace started with Perl
> script /root/libexec/perf-core/scripts/perl/rwtop.pl
>
>  Fatal: did not read header event
>
> commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
> function to skip over e.g. header_page, but this doesn't work for
> live mode.  This patch re-implements skip() to use read() instead of
> lseek() to fix that.

Out of curiosity: why doesn't lseek() work and why can't we fix that?

Re: [tip:perf/core] perf: Use read() instead of lseek() in trace_event_read.c:skip()

[H. Peter Anvin]

On 05/20/2010 06:39 AM, Pekka Enberg wrote:
> On Thu, May 20, 2010 at 3:54 PM, tip-bot for Tom Zanussi
> <tzanussi@gmail.com> wrote:
>> Commit-ID:  cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
>> Gitweb:     http://git.kernel.org/tip/cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
>> Author:     Tom Zanussi <tzanussi@gmail.com>
>> AuthorDate: Tue, 4 May 2010 23:02:10 -0500
>> Committer:  Frederic Weisbecker <fweisbec@gmail.com>
>> CommitDate: Thu, 20 May 2010 08:37:17 +0200
>>
>> perf: Use read() instead of lseek() in trace_event_read.c:skip()
>>
>> This is a small fix for a problem affecting live-mode, introduced
>> recently:
>>
>> root@tropicana:~# perf trace rwtop
>> perf trace started with Perl
>> script /root/libexec/perf-core/scripts/perl/rwtop.pl
>>
>>  Fatal: did not read header event
>>
>> commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
>> function to skip over e.g. header_page, but this doesn't work for
>> live mode.  This patch re-implements skip() to use read() instead of
>> lseek() to fix that.
> 
> Out of curiosity: why doesn't lseek() work and why can't we fix that?

Presumably because we're reading from a nonseekable stream?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [tip:perf/core] perf: Use read() instead of lseek() in trace_event_read.c:skip()

[Frederic Weisbecker]

On Thu, May 20, 2010 at 04:39:37PM +0300, Pekka Enberg wrote:
> On Thu, May 20, 2010 at 3:54 PM, tip-bot for Tom Zanussi
> <tzanussi@gmail.com> wrote:
> > Commit-ID:  cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
> > Gitweb:     http://git.kernel.org/tip/cbb5cf7ff6b298beacfe23db3386335b0b9c0a2d
> > Author:     Tom Zanussi <tzanussi@gmail.com>
> > AuthorDate: Tue, 4 May 2010 23:02:10 -0500
> > Committer:  Frederic Weisbecker <fweisbec@gmail.com>
> > CommitDate: Thu, 20 May 2010 08:37:17 +0200
> >
> > perf: Use read() instead of lseek() in trace_event_read.c:skip()
> >
> > This is a small fix for a problem affecting live-mode, introduced
> > recently:
> >
> > root@tropicana:~# perf trace rwtop
> > perf trace started with Perl
> > script /root/libexec/perf-core/scripts/perl/rwtop.pl
> >
> >  Fatal: did not read header event
> >
> > commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7 added a skip()
> > function to skip over e.g. header_page, but this doesn't work for
> > live mode.  This patch re-implements skip() to use read() instead of
> > lseek() to fix that.
> 
> Out of curiosity: why doesn't lseek() work and why can't we fix that?


Because depending on the mode we are, that may or may not work.

We have the normal mode, when the input in a regular file (perf.data)
where it works. And we have the pipe mode, where the seek fails.