From: Jiri Olsa <jolsa@redhat.com>
To: Milian Wolff <milian.wolff@kdab.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
acme@kernel.org, jolsa@kernel.org,
Jin Yao <yao.jin@linux.intel.com>,
Linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
kernel-team@lge.com
Subject: Re: [PATCH v5 16/16] perf util: use correct IP mapping to find srcline for hist entry
Date: Fri, 13 Oct 2017 13:03:20 +0200 [thread overview]
Message-ID: <20171013110320.GA28151@krava> (raw)
In-Reply-To: <20171012185218.GA20959@krava>
On Thu, Oct 12, 2017 at 08:52:18PM +0200, Jiri Olsa wrote:
> On Thu, Oct 12, 2017 at 08:22:58PM +0200, Milian Wolff wrote:
>
> SNIP
>
> > > > return SRCLINE_UNKNOWN;
> > > >
> > > > - return get_srcline(map->dso, map__rip_2objdump(map, he->ip),
> > > > + return get_srcline(map->dso, map__objdump_2mem(map, he->ip),
> > > >
> > > > he->ms.sym, true, true);
> > >
> > > I'm not sure this is right. IIRC hist_entry->ip is a relative IP so
> > > it needs to be changed for objdump use. Maybe there's some bug on
> > > translating the address but it seems that the original code is
> > > correct. And this change breaks srcline for my test program (which is
> > > a PIE).
> >
> > Odd, I'll have to look at that. But this is imo unrelated to the rest of the
> > patch series. So maybe we skip this one and apply the others. Assuming those
> > other patches are OK by now?
> >
> > Jiri, you also reviewed it before, is there anything else missing?
>
> I plan to check on it this week.. sry for delay
I think Namhyung is right with his comment, the rest look ok,
so for patches 1 - 15:
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
thanks,
jirka
next prev parent reply other threads:[~2017-10-13 11:03 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-09 20:32 [PATCH v5 00/16] generate full callchain cursor entries for inlined frames Milian Wolff
2017-10-09 20:32 ` [PATCH v5 01/16] perf report: remove code to handle inline frames from browsers Milian Wolff
2017-10-09 20:32 ` [PATCH v5 02/16] perf util: store srcline in callchain_cursor_node Milian Wolff
2017-10-09 20:32 ` [PATCH v5 03/16] perf util: refactor inline_list to operate on symbols Milian Wolff
2017-10-09 20:32 ` [PATCH v5 04/16] perf util: refactor inline_list to store srcline string directly Milian Wolff
2017-10-09 20:32 ` [PATCH v5 05/16] perf report: create real callchain entries for inlined frames Milian Wolff
2017-10-09 20:33 ` [PATCH v5 06/16] perf report: fall-back to function name comparison for -g srcline Milian Wolff
2017-10-09 20:33 ` [PATCH v5 07/16] perf report: mark inlined frames in output by " (inlined)" suffix Milian Wolff
2017-10-09 20:33 ` [PATCH v5 08/16] perf script: mark inlined frames and do not print DSO for them Milian Wolff
2017-10-09 20:33 ` [PATCH v5 09/16] perf report: compare symbol name for inlined frames when matching Milian Wolff
2017-10-13 13:28 ` Arnaldo Carvalho de Melo
2017-10-09 20:33 ` [PATCH v5 10/16] perf report: compare symbol name for inlined frames when sorting Milian Wolff
2017-10-09 20:33 ` [PATCH v5 11/16] perf report: properly handle branch count in match_chain Milian Wolff
2017-10-13 13:39 ` Arnaldo Carvalho de Melo
2017-10-13 14:08 ` Arnaldo Carvalho de Melo
2017-10-14 19:30 ` Milian Wolff
2017-10-16 14:17 ` Arnaldo Carvalho de Melo
2017-10-16 4:18 ` ravi
2017-10-16 8:27 ` Milian Wolff
2017-10-16 14:19 ` Arnaldo Carvalho de Melo
2017-10-09 20:33 ` [PATCH v5 12/16] perf report: cache failed lookups of inlined frames Milian Wolff
2017-10-09 20:33 ` [PATCH v5 13/16] perf report: cache srclines for callchain nodes Milian Wolff
2017-10-09 20:33 ` [PATCH v5 14/16] perf report: use srcline from callchain for hist entries Milian Wolff
2017-10-09 20:33 ` [PATCH v5 15/16] perf util: enable handling of inlined frames by default Milian Wolff
2017-10-09 20:33 ` [PATCH v5 16/16] perf util: use correct IP mapping to find srcline for hist entry Milian Wolff
2017-10-10 4:49 ` Namhyung Kim
2017-10-12 18:22 ` Milian Wolff
2017-10-12 18:52 ` Jiri Olsa
2017-10-13 11:03 ` Jiri Olsa [this message]
2017-10-13 1:19 ` Namhyung Kim
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=20171013110320.GA28151@krava \
--to=jolsa@redhat.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=jolsa@kernel.org \
--cc=kernel-team@lge.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=milian.wolff@kdab.com \
--cc=namhyung@kernel.org \
--cc=yao.jin@linux.intel.com \
/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).