Em Wed, Sep 02, 2015 at 04:00:08PM +0300, Adrian Hunter escreveu:
> 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.hun...@intel.com>
>
> I think it would be worth picking this one up for 4.3
I guess I had this picked up, is there anything else from you that is
outstanding?
- Arnaldo
> > ---
> > 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) {
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
