LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Leo Yan <leo.yan@linaro.org>
To: Mike Leach <mike.leach@linaro.org>
Cc: Robert Walker <robert.walker@arm.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Kim Phillips <kim.phillips@arm.co>, Tor Jeremiassen <tor@ti.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, coresight@lists.linaro.org
Subject: Re: [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets
Date: Wed, 30 May 2018 23:39:00 +0800	[thread overview]
Message-ID: <20180530153900.GA10925@leoy-ThinkPad-X240s> (raw)
In-Reply-To: <CAJ9a7VhC7EHJ1DtTEH1uERGiBpCJgseboaH0cqgpsU3ZMcgjgQ@mail.gmail.com>

Hi Mike,

On Wed, May 30, 2018 at 04:04:34PM +0100, Mike Leach wrote:

[...]

> >>> +               /* Generate sample for exception packet */
> >>> +               if (etmq->prev_packet->exc == true)
> >>> +                       generate_sample = true;
> >>
> >>
> >> Please don't do that.  Exception packets have a type of their own and can
> >> be
> >> added to the decoder packet queue the same way INST_RANGE and TRACE_ON
> >> packets
> >> are.  Moreover exception packet containt an address that, if I'm reading
> >> the
> >> documenation properly, can be used to keep track of instructions that were
> >> executed between the last address of the previous range packet and the
> >> address
> >> executed just before the exception occurred.  Mike and Rob will have to
> >> confirm
> >> this as the decoder may be doing all that hard work for us.
> >>
> 
> clarification on the exception packets....
> 
> The Opencsd output exception packet gives you the exception number,
> and optionally the preferred return address. If this address is
> present does depend a lot on the underlying protocol - will normally
> be there with ETMv4.
> Exceptions are marked differently in the underlying protocol - the
> OCSD packets abstract away these differences.
> 
> consider the code:
> 
> 0x1000: <some instructions>
> 0x1100: BR 0x2000
> ....
> 0x2000: <some instructions>
> 0x2020  BZ r4
> 
> Without an exception this would result in the packets
> 
> OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken)   // recall that
> range packets have start addr inclusive, end addr exclusive.
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
> 
> Now consider an exception occurring before the BR 0x2000
> 
> this will result in:-
> OCSD_RANGE(0x1000, 0x1100, Last instr type=Other)
> OCSD_EXECEPTION(IRQ, ret-addr 0x1100)
> OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken)    //
> this is more likely to have multiple ranges / branches before any
> return, but simplified here.
> OCSD_EXCEPTION_RETURN()  // present if exception returns are
> explicitly marked in underlying trace - may not always be depending on
> circumstances.
> OCSD_RANGE(0x1100,0x1104, Last=BR, taken) // continue on with short
> range - just the branch
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
> 
> Now consider the exception occurring after the BR, but before any
> other instructions are executed.
> 
> OCSD_RANGE(0x1000,0x1104, Last instr type=Br, taken)   // recall that
> range packets have start addr inclusive, end addr exclusive.
> OCSD_EXECEPTION(IRQ, ret-addr 0x2000)     // here the preferred return
> address is actually the target of the branch.
> OCSD_RANGE(IRQ_START, IRQ_END+4, Last instr type = BR, taken)    //
> this is more likely to have multiple ranges / branches before any
> return, but simplified here.
> OCSD_RANGE(0x2000,0x2024, Last instr type=Br, <taken / not taken -
> depends on condition>
> 
> So in general it is possible to arrive in the IRQ_START range with the
> previous packet having been either a taken branch, a not taken branch,
> or not a branch.
> Care must be taken - whether AutoFDO or normal trace disassembly not
> to assume that having the last range packet as a taken branch means
> that the next range packet is the target, if there is an intervening
> exception packet.

Thanks a lot for detailed explaination.

IIUC, AutoFDO will not have such issue due every range packet will be
handled for it.  On the other hand, as you remind, the branch samples
(and its consumer trace disassembler) is very dependent on the flag
'last_instr_taken_branch'.

According to your explaination, I think we consider the branch is
taken for below situations:

- The new coming packet is exception packet (both for exception entry
  and exit packets);
- The previous packet is expcetion packet;
- The previous packet is normal range packet with
  'last_instr_taken_branch' = true;

So I'd like to use below function to demonstrate my understanding for
exception packets handling.  I also will send out one new patch for
support exception packet for reviewing.

If you have concern or I miss anything, please let me know.

static bool cs_etm__is_taken_branch(struct cs_etm_packet *prev_packet,
                                    struct cs_etm_packet *packet,)
{
        /* The branch is taken for normal range packet with taken branch flag */
        if (prev_packet->sample_type == CS_ETM_RANGE &&
            prev_packet->last_instr_taken_branch)
                return true;

        /* The branch is taken if previous packet is exception packet */
        if (prev_packet->sample_type == CS_ETM_EXCEPTION ||
            prev_packet->sample_type == CS_ETM_EXCEPTION_RET)
                return true;

        /* The branch is taken for an intervening exception packet */
        if (packet->sample_type == CS_ETM_EXCEPTION ||
            packet->sample_type == CS_ETM_EXCEPTION_RET)
                return true;

        return false;
}

[...]

Thanks,
Leo Yan

  reply	other threads:[~2018-05-30 15:39 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-28  8:44 [RFT v3 0/4] Perf script: Add python script for CoreSight trace disassembler Leo Yan
2018-05-28  8:45 ` [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets Leo Yan
2018-05-28 22:13   ` Mathieu Poirier
2018-05-29  0:25     ` Leo Yan
2018-05-29 16:04       ` Mathieu Poirier
2018-05-30  0:28         ` Leo Yan
2018-05-30 14:45           ` Mathieu Poirier
2018-05-30 14:49             ` Leo Yan
2018-05-30  9:45     ` Robert Walker
2018-05-30 15:04       ` Mike Leach
2018-05-30 15:39         ` Leo Yan [this message]
2018-05-30 15:49           ` Leo Yan
2018-05-28  8:45 ` [RFT v3 2/4] perf script python: Add addr into perf sample dict Leo Yan
2018-05-31 10:45   ` [tip:perf/urgent] " tip-bot for Leo Yan
2018-05-28  8:45 ` [RFT v3 3/4] perf script python: Add script for CoreSight trace disassembler Leo Yan
2018-05-28  8:45 ` [RFT v3 4/4] coresight: Document " Leo Yan
2018-05-28 20:03 ` [RFT v3 0/4] Perf script: Add python script " Arnaldo Carvalho de Melo
2018-05-28 21:53   ` Mathieu Poirier
2018-05-29 13:32     ` Arnaldo Carvalho de Melo

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=20180530153900.GA10925@leoy-ThinkPad-X240s \
    --to=leo.yan@linaro.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=coresight@lists.linaro.org \
    --cc=jolsa@redhat.com \
    --cc=kim.phillips@arm.co \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mike.leach@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=robert.walker@arm.com \
    --cc=tor@ti.com \
    --subject='Re: [RFT v3 1/4] perf cs-etm: Generate branch sample for missed packets' \
    /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

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