[GIT PULL 0/2] perf/urgent fixes

[GIT PULL 0/2] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit 5b923564ccf43f92969c9e0fd199c8c5db657039:

  Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2015-09-02 09:22:53 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 53ff6bc37be449f546158a39c528d7814dfb15a1:

  perf tools: Fix use of wrong event when processing exit events (2015-09-02 17:46:26 -0300)

----------------------------------------------------------------
perf/urgent fixes:

- In some cases where perf_event.fork.{pid,tid} should be used we were instead
  using perf_event.comm.{pid,tid}, which is not a problem for for the 'pid'
  case, that sits in the same place in these union_perf_event members, but
  comm.tid sits where fork.ppid is, oops.

  These cases were considered as (potentially) problematic:

   - 'perf script' with !sample_id_all, i.e. only non old kernels without
      perf_event_attr.sample_id_all.

   - intel_pt could be affected when decoding without timestamps, as the exit
     event is only used to flush out data which anyway gets flushed at the
     end of the session.

   - 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.

  Fix it. (Adrian Hunter)

- Due to relaxing the compiler checks for bison generated files, we missed
  updating one parse_events_add_pmu() caller when this function had its
  prototype changed, fix it. (Jiri Olsa)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Adrian Hunter (1):
      perf tools: Fix use of wrong event when processing exit events

Jiri Olsa (1):
      perf tools: Fix parse_events_add_pmu caller

 tools/perf/builtin-script.c    | 4 ++--
 tools/perf/util/intel-bts.c    | 2 +-
 tools/perf/util/intel-pt.c     | 2 +-
 tools/perf/util/parse-events.y | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

[PATCH 1/2] perf tools: Fix parse_events_add_pmu caller

[Arnaldo Carvalho de Melo]

From: Jiri Olsa <jolsa@kernel.org>

Following commit changed parse_events_add_pmu interface:
  36adec85a86f perf tools: Change parse_events_add_pmu interface

but forgot to change one caller. Because of lessen compilation rules for
the bison parser, the compiler did not warn on that.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Raphael Beamonte <raphael.beamonte@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Fixes: 36adec85a86f ("perf tools: Change parse_events_add_pmu interface")
Link: http://lkml.kernel.org/r/1441180605-24737-2-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 591905a02b92..9cd70819c795 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -255,7 +255,7 @@ PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
 	list_add_tail(&term->list, head);
 
 	ALLOC_LIST(list);
-	ABORT_ON(parse_events_add_pmu(list, &data->idx, "cpu", head));
+	ABORT_ON(parse_events_add_pmu(data, list, "cpu", head));
 	parse_events__free_terms(head);
 	$$ = list;
 }
-- 
2.1.0

[PATCH 2/2] perf tools: Fix use of wrong event when processing exit events

[Arnaldo Carvalho de Melo]

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

Re: [GIT PULL 0/2] perf/urgent fixes

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit 5b923564ccf43f92969c9e0fd199c8c5db657039:
> 
>   Merge tag 'perf-urgent-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2015-09-02 09:22:53 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 53ff6bc37be449f546158a39c528d7814dfb15a1:
> 
>   perf tools: Fix use of wrong event when processing exit events (2015-09-02 17:46:26 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> - In some cases where perf_event.fork.{pid,tid} should be used we were instead
>   using perf_event.comm.{pid,tid}, which is not a problem for for the 'pid'
>   case, that sits in the same place in these union_perf_event members, but
>   comm.tid sits where fork.ppid is, oops.
> 
>   These cases were considered as (potentially) problematic:
> 
>    - 'perf script' with !sample_id_all, i.e. only non old kernels without
>       perf_event_attr.sample_id_all.
> 
>    - intel_pt could be affected when decoding without timestamps, as the exit
>      event is only used to flush out data which anyway gets flushed at the
>      end of the session.
> 
>    - 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.
> 
>   Fix it. (Adrian Hunter)
> 
> - Due to relaxing the compiler checks for bison generated files, we missed
>   updating one parse_events_add_pmu() caller when this function had its
>   prototype changed, fix it. (Jiri Olsa)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Adrian Hunter (1):
>       perf tools: Fix use of wrong event when processing exit events
> 
> Jiri Olsa (1):
>       perf tools: Fix parse_events_add_pmu caller
> 
>  tools/perf/builtin-script.c    | 4 ++--
>  tools/perf/util/intel-bts.c    | 2 +-
>  tools/perf/util/intel-pt.c     | 2 +-
>  tools/perf/util/parse-events.y | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo