[PATCH 3/7] Revert "perf: Fix warning while reading ring buffer headers"

From: Ian Munsie
Date: Thu May 13 2010 - 02:05:52 EST


From: Ian Munsie <imunsie@xxxxxxxxxxx>

This reverts commit d00a47cce569a3e660a8c9de5d57af28d6a9f0f7.
"perf: Fix warning while reading ring buffer headers"

The reverted patch removed the processing of the header_page, skipping
over it instead on the assumption that perf was not using any of the
data from that header. The patch neglected to remove the header_page_
variables which were initialised in the removed code, nor did it fix any
code that was using those variables.

In particular, long_size was set based on one of those variables
(header_page_size_size) to learn the size of a long from the kernel,
which is necessary to correctly print out some of the trace information
in some circumstances. For instance, the size of a long in a 64 bit
kernel would differ from the size of a long in perf if it was compiled
for a 32 bit userspace. Perf trace needs to know the size of a long in
the kernel so that it can print out the correct value without
truncation.

Signed-off-by: Ian Munsie <imunsie@xxxxxxxxxxx>
---
tools/perf/util/trace-event-parse.c | 89 +++++++++++++++++++++++++++++++++++
tools/perf/util/trace-event-read.c | 12 ++---
tools/perf/util/trace-event.h | 1 +
3 files changed, 95 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c
index f360f75..9717f7a 100644
--- a/tools/perf/util/trace-event-parse.c
+++ b/tools/perf/util/trace-event-parse.c
@@ -691,6 +691,11 @@ static int __read_expected(enum event_type expect, const char *str,
return ret;
}

+static int read_expected_warn(enum event_type expect, const char *str, bool warn)
+{
+ return __read_expected(expect, str, 1, warn);
+}
+
static int read_expected(enum event_type expect, const char *str)
{
return __read_expected(expect, str, 1, true);
@@ -3107,6 +3112,90 @@ static void print_args(struct print_arg *args)
}
}

+static void parse_header_field(const char *field,
+ int *offset, int *size, bool warn)
+{
+ char *token;
+ int type;
+
+ if (read_expected(EVENT_ITEM, "field") < 0)
+ return;
+ if (read_expected(EVENT_OP, ":") < 0)
+ return;
+
+ /* type */
+ if (read_expect_type(EVENT_ITEM, &token) < 0)
+ goto fail;
+ free_token(token);
+
+ if (read_expected_warn(EVENT_ITEM, field, warn) < 0)
+ return;
+ if (read_expected(EVENT_OP, ";") < 0)
+ return;
+ if (read_expected(EVENT_ITEM, "offset") < 0)
+ return;
+ if (read_expected(EVENT_OP, ":") < 0)
+ return;
+ if (read_expect_type(EVENT_ITEM, &token) < 0)
+ goto fail;
+ *offset = atoi(token);
+ free_token(token);
+ if (read_expected(EVENT_OP, ";") < 0)
+ return;
+ if (read_expected(EVENT_ITEM, "size") < 0)
+ return;
+ if (read_expected(EVENT_OP, ":") < 0)
+ return;
+ if (read_expect_type(EVENT_ITEM, &token) < 0)
+ goto fail;
+ *size = atoi(token);
+ free_token(token);
+ if (read_expected(EVENT_OP, ";") < 0)
+ return;
+ type = read_token(&token);
+ if (type != EVENT_NEWLINE) {
+ /* newer versions of the kernel have a "signed" type */
+ if (type != EVENT_ITEM)
+ goto fail;
+
+ if (strcmp(token, "signed") != 0)
+ goto fail;
+
+ free_token(token);
+
+ if (read_expected(EVENT_OP, ":") < 0)
+ return;
+
+ if (read_expect_type(EVENT_ITEM, &token))
+ goto fail;
+
+ free_token(token);
+ if (read_expected(EVENT_OP, ";") < 0)
+ return;
+
+ if (read_expect_type(EVENT_NEWLINE, &token))
+ goto fail;
+ }
+ fail:
+ free_token(token);
+}
+
+int parse_header_page(char *buf, unsigned long size)
+{
+ init_input_buf(buf, size);
+
+ parse_header_field("timestamp", &header_page_ts_offset,
+ &header_page_ts_size, true);
+ parse_header_field("commit", &header_page_size_offset,
+ &header_page_size_size, true);
+ parse_header_field("overwrite", &header_page_overwrite_offset,
+ &header_page_overwrite_size, false);
+ parse_header_field("data", &header_page_data_offset,
+ &header_page_data_size, true);
+
+ return 0;
+}
+
int parse_ftrace_file(char *buf, unsigned long size)
{
struct format_field *field;
diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c
index cb54cd0..43f19c1 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;
@@ -190,6 +184,7 @@ static void read_ftrace_printk(void)
static void read_header_files(void)
{
unsigned long long size;
+ char *header_page;
char *header_event;
char buf[BUFSIZ];

@@ -199,7 +194,10 @@ static void read_header_files(void)
die("did not read header page");

size = read8();
- skip(size);
+ header_page = malloc_or_die(size);
+ read_or_die(header_page, size);
+ parse_header_page(header_page, size);
+ free(header_page);

/*
* The size field in the page is of type long,
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index cc58a19..5fa3c5a 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -246,6 +246,7 @@ extern int header_page_data_size;

extern bool latency_format;

+int parse_header_page(char *buf, unsigned long size);
int trace_parse_common_type(void *data);
int trace_parse_common_pid(void *data);
int parse_common_pc(void *data);
--
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/