All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@gmail.com>
To: acme@kernel.org
Cc: linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>
Subject: [PATCH] perf trace: Handle legacy syscalls
Date: Thu, 19 Mar 2015 12:36:21 -0600	[thread overview]
Message-ID: <1426790181-19118-1-git-send-email-dsahern@gmail.com> (raw)

Currently the code skips the first field with the expectation that it is
'nr'. But older kernels do not have the 'nr' field:

    field:int nr;   offset:8;   size:4; signed:1;

Change perf-trace to drop the field if it exists after parsing the format
file.

This fixes the off-by-one problem with older kernels (e.g., RHEL6). e.g,
perf-trace shows this for write:

    1.515 ( 0.006 ms): dd/4245 write(buf: 2</dev/pts/0>, count: 140733837536224                      ) = 26

where 2 is really the fd, the huge number is really the buf address, etc.
With this patch you get the more appropriate:

    1.813 ( 0.003 ms): dd/6330 write(fd: 2</dev/pts/0>, buf: 0x7fff22fc81f0, count: 25               ) = 25

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/lib/traceevent/event-parse.c | 11 ++++++++---
 tools/lib/traceevent/event-parse.h |  1 +
 tools/perf/builtin-trace.c         | 16 +++++++++++++---
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index afe20ed9fac8..e8a29e730dfb 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -6228,15 +6228,20 @@ void pevent_ref(struct pevent *pevent)
 	pevent->ref_count++;
 }
 
+void free_format_field(struct format_field *field)
+{
+	free(field->type);
+	free(field->name);
+	free(field);
+}
+
 static void free_format_fields(struct format_field *field)
 {
 	struct format_field *next;
 
 	while (field) {
 		next = field->next;
-		free(field->type);
-		free(field->name);
-		free(field);
+		free_format_field(field);
 		field = next;
 	}
 }
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h
index 5b4efc062320..a548fac646f6 100644
--- a/tools/lib/traceevent/event-parse.h
+++ b/tools/lib/traceevent/event-parse.h
@@ -619,6 +619,7 @@ enum pevent_errno pevent_parse_format(struct pevent *pevent,
 				      const char *buf,
 				      unsigned long size, const char *sys);
 void pevent_free_format(struct event_format *event);
+void free_format_field(struct format_field *field);
 
 void *pevent_get_field_raw(struct trace_seq *s, struct event_format *event,
 			   const char *name, struct pevent_record *record,
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index dcd950ef2fd7..5caeefeda48a 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1442,14 +1442,24 @@ static int syscall__set_arg_fmts(struct syscall *sc)
 	struct format_field *field;
 	int idx = 0;
 
-	sc->arg_scnprintf = calloc(sc->tp_format->format.nr_fields - 1, sizeof(void *));
+	field = sc->tp_format->format.fields;
+	/* drop nr field - not relevant here; does not exist on older kernels */
+	if (field && strcmp(field->name, "nr") == 0) {
+		struct format_field *next = field->next;
+
+		free_format_field(field);
+		sc->tp_format->format.fields = next;
+		sc->tp_format->format.nr_fields--;
+	}
+
+	sc->arg_scnprintf = calloc(sc->tp_format->format.nr_fields, sizeof(void *));
 	if (sc->arg_scnprintf == NULL)
 		return -1;
 
 	if (sc->fmt)
 		sc->arg_parm = sc->fmt->arg_parm;
 
-	for (field = sc->tp_format->format.fields->next; field; field = field->next) {
+	for (field = sc->tp_format->format.fields; field; field = field->next) {
 		if (sc->fmt && sc->fmt->arg_scnprintf[idx])
 			sc->arg_scnprintf[idx] = sc->fmt->arg_scnprintf[idx];
 		else if (field->flags & FIELD_IS_POINTER)
@@ -1547,7 +1557,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
 			.thread = thread,
 		};
 
-		for (field = sc->tp_format->format.fields->next; field;
+		for (field = sc->tp_format->format.fields; field;
 		     field = field->next, ++arg.idx, bit <<= 1) {
 			if (arg.mask & bit)
 				continue;
-- 
2.2.1


             reply	other threads:[~2015-03-19 18:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19 18:36 David Ahern [this message]
2015-03-20 15:32 ` [PATCH] perf trace: Handle legacy syscalls Arnaldo Carvalho de Melo
2015-03-20 15:40   ` Steven Rostedt
2015-03-20 15:44     ` Arnaldo Carvalho de Melo
2015-03-20 15:48       ` David Ahern
2015-03-20 15:48   ` David Ahern
2015-03-20 16:06     ` Arnaldo Carvalho de Melo
2015-03-20 16:12       ` David Ahern
2015-03-20 16:26         ` Arnaldo Carvalho de Melo
2015-03-20 16:30           ` David Ahern
2015-03-22 10:17 ` [tip:perf/core] tools lib traceevent: Add destructor for format_field tip-bot for David Ahern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1426790181-19118-1-git-send-email-dsahern@gmail.com \
    --to=dsahern@gmail.com \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.