public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCH] perf tools: Fix use of wrong event when processing exit events
Date: Thu, 3 Sep 2015 15:13:44 +0300	[thread overview]
Message-ID: <55E83978.4050609@intel.com> (raw)
In-Reply-To: <20150902195501.GB11521@kernel.org>

On 02/09/15 22:55, Arnaldo Carvalho de Melo wrote:
> 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.hunter@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?

Only "perf tools: Add support for PERF_RECORD_SWITCH to Intel PT" but that
is a feature not a bug.


  reply	other threads:[~2015-09-03 12:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-18  9:07 [PATCH] perf tools: Fix use of wrong event when processing exit events Adrian Hunter
2015-09-02 13:00 ` Adrian Hunter
2015-09-02 19:55   ` Arnaldo Carvalho de Melo
2015-09-03 12:13     ` Adrian Hunter [this message]
2015-09-05 14:01 ` [tip:perf/urgent] " tip-bot for Adrian Hunter

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=55E83978.4050609@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=jolsa@redhat.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox