linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Adrian Hunter <adrian.hunter@intel.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 2/2] perf tools: Fix use of wrong event when processing exit events
Date: Thu,  3 Sep 2015 13:02:09 -0300	[thread overview]
Message-ID: <1441296129-31530-3-git-send-email-acme@kernel.org> (raw)
In-Reply-To: <1441296129-31530-1-git-send-email-acme@kernel.org>

From: Adrian Hunter <adrian.hunter@intel.com>

In a couple of cases the 'comm' member of 'union event' has been used
instead of the correct member ('fork') when processing exit events.

In the cases where it has been used incorrectly, only the 'pid' and
'tid' are affected.  The 'pid' value would be correct anyway because it
is in the same position in 'comm' and 'fork' events, but the 'tid' would
have been incorrectly assigned from 'ppid'.

However, for exit events, the kernel puts the current task in the 'ppid'
and 'ttid' which is the same as the exiting task.  That is 'ppid' ==
'pid' and if the task is not multi-threaded, 'pid' == 'tid' i.e. the
data goes wrong only when tracing multi-threaded programs.

It is hard to find an example of how this would produce an error in
practice.  There are 3 occurences of the fix:

1. perf script is only affected if !sample_id_all which only happens on
  old kernels.

2. intel_pt is only affected when decoding without timestamps
   and would probably still decode correctly - the exit event is
   only used to flush out data which anyway gets flushed at the
   end of the session

3. intel_bts also uses the exit event to flush data which
   would probably not cause errors as it would get flushed at
   the end of the session instead

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/1439888825-27708-1-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c | 4 ++--
 tools/perf/util/intel-bts.c | 2 +-
 tools/perf/util/intel-pt.c  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index eb51325e8ad9..284a76e04628 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -768,8 +768,8 @@ static int process_exit_event(struct perf_tool *tool,
 	if (!evsel->attr.sample_id_all) {
 		sample->cpu = 0;
 		sample->time = 0;
-		sample->tid = event->comm.tid;
-		sample->pid = event->comm.pid;
+		sample->tid = event->fork.tid;
+		sample->pid = event->fork.pid;
 	}
 	print_sample_start(sample, thread, evsel);
 	perf_event__fprintf(event, stdout);
diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
index ea768625ab5b..eb0e7f8bf515 100644
--- a/tools/perf/util/intel-bts.c
+++ b/tools/perf/util/intel-bts.c
@@ -623,7 +623,7 @@ static int intel_bts_process_event(struct perf_session *session,
 	if (err)
 		return err;
 	if (event->header.type == PERF_RECORD_EXIT) {
-		err = intel_bts_process_tid_exit(bts, event->comm.tid);
+		err = intel_bts_process_tid_exit(bts, event->fork.tid);
 		if (err)
 			return err;
 	}
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index bb41c20e6005..535d86f8e4d1 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -1494,7 +1494,7 @@ static int intel_pt_process_event(struct perf_session *session,
 	if (pt->timeless_decoding) {
 		if (event->header.type == PERF_RECORD_EXIT) {
 			err = intel_pt_process_timeless_queues(pt,
-							       event->comm.tid,
+							       event->fork.tid,
 							       sample->time);
 		}
 	} else if (timestamp) {
-- 
2.1.0


  parent reply	other threads:[~2015-09-03 16:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-03 16:02 [GIT PULL 0/2] perf/urgent fixes Arnaldo Carvalho de Melo
2015-09-03 16:02 ` [PATCH 1/2] perf tools: Fix parse_events_add_pmu caller Arnaldo Carvalho de Melo
2015-09-03 16:02 ` Arnaldo Carvalho de Melo [this message]
2015-09-04  9:01 ` [GIT PULL 0/2] perf/urgent fixes Ingo Molnar

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=1441296129-31530-3-git-send-email-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).