lkml.org 
[lkml]   [2015]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] perf tools: Fix use of wrong event when processing exit events
From
Date
On 18/08/15 12:07, Adrian Hunter wrote:
> 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>

I think it would be worth picking this one up for 4.3

> ---
> 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 105332e950a9..17030c6c64b6 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -739,8 +739,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 dce99cfb1309..09d142b362fa 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 2a4a4120473b..ef2fb13f1dfa 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -1464,7 +1464,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) {
>



\
 
 \ /
  Last update: 2015-09-02 15:21    [W:0.089 / U:0.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site