From: "Steinar H. Gunderson" <sesse@google.com>
To: Adrian Hunter <adrian.hunter@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
He Kuang <hekuang@huawei.com>
Subject: Re: [PATCH] perf intel-pt: Synthesize cycle events
Date: Tue, 29 Mar 2022 14:31:14 +0200 [thread overview]
Message-ID: <YkL8EpWaeZ1US8t2@google.com> (raw)
In-Reply-To: <Yjm5stBpRC0g4G8s@google.com>
On Tue, Mar 22, 2022 at 12:57:38PM +0100, Steinar H. Gunderson wrote:
> and this is evidently fatal. So for whatever reason, the sample address
> is attributed to some symbol, and that symbol is assumed to have a
> single range (is this even necessarily true?), we refuse the event,
> and then we fail the entire report. (It doesn't happen with --stdio,
> though!)
I think I traced this. The basic issue is this routine, which compares
two symbols (to see if they should be combined for the sake of perf
report):
int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
{
if (!sym_l || !sym_r)
return cmp_null(sym_l, sym_r);
if (sym_l == sym_r)
return 0;
if (sym_l->inlined || sym_r->inlined) {
int ret = strcmp(sym_l->name, sym_r->name);
if (ret)
return ret;
if ((sym_l->start <= sym_r->end) && (sym_l->end >= sym_r->start))
return 0;
}
if (sym_l->start != sym_r->start)
return (int64_t)(sym_r->start - sym_l->start);
return (int64_t)(sym_r->end - sym_l->end);
}
The problem is that part comparing sym_l->start and sym_r->end. I'm not
entirely sure what the logic here is, but it seems to me that the
intention is that if sym_l is a subset of sym_r, it collapses them.
However, it seems to assume that [start,end] is inclusive, whereas it is
generally [start,end) in later code. This means that if you have e.g.
a symbol [0x1000,0x1010) which is then inlined as the very first thing
in the adjacent function [0x1010,0x1030) (say, sym_r is the inlined
symbol [0x1010,0x1020)), _sort__sym_cmp will indeed collapse them.
This causes the ERANGE problem later, as you can have 0x1005 being
mapped to a hist_entry correpsonding to the symbol [0x1010,0x1030).
It seems this logic was indeed added to fix ERANGE problems in 2019
(see 7346195e8), but it is seemingly incomplete.
If I change <= to < and >= to >, it stops erroring out; however, I'm
still not sure whether this is actually right. Doesn't the collapsing
then depend on what order the entries happen to be added in, ie.,
whether the larger symbol comes first or last?
/* Steinar */
next prev parent reply other threads:[~2022-03-29 12:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-10 9:38 [PATCH] perf intel-pt: Synthesize cycle events Steinar H. Gunderson
2022-03-11 9:10 ` Adrian Hunter
2022-03-11 17:42 ` Steinar H. Gunderson
2022-03-14 16:24 ` Adrian Hunter
2022-03-15 10:16 ` Steinar H. Gunderson
2022-03-15 11:32 ` Adrian Hunter
2022-03-15 18:00 ` Steinar H. Gunderson
2022-03-15 20:11 ` Adrian Hunter
2022-03-16 8:19 ` Steinar H. Gunderson
2022-03-16 11:19 ` Adrian Hunter
2022-03-16 12:59 ` Steinar H. Gunderson
2022-03-21 9:16 ` Adrian Hunter
2022-03-21 10:33 ` Steinar H. Gunderson
2022-03-21 13:09 ` Adrian Hunter
2022-03-21 16:58 ` Steinar H. Gunderson
2022-03-21 17:40 ` Adrian Hunter
2022-03-22 11:57 ` Steinar H. Gunderson
2022-03-29 12:31 ` Steinar H. Gunderson [this message]
2022-03-29 14:16 ` Steinar H. Gunderson
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=YkL8EpWaeZ1US8t2@google.com \
--to=sesse@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=hekuang@huawei.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.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;
as well as URLs for NNTP newsgroup(s).