* [PATCH 14/29] perf annotate: Support jump instruction with target as second operand
2016-12-20 17:03 [GIT PULL 00/29] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2016-12-20 17:03 ` Arnaldo Carvalho de Melo
2016-12-20 17:03 ` [PATCH 15/29] perf annotate: Fix jump target outside of function address range Arnaldo Carvalho de Melo
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-12-20 17:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Ravi Bangoria, Alexander Shishkin, Chris Riyder,
Kim Phillips, Markus Trippelsdorf, Masami Hiramatsu,
Naveen N . Rao, Peter Zijlstra, Taeung Song, linuxppc-dev,
Arnaldo Carvalho de Melo
From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Architectures like PowerPC have jump instructions that includes a target
address as a second operand. For example, 'bne cr7,0xc0000000000f6154'.
Add support for such instruction in perf annotate.
objdump o/p:
c0000000000f6140: ld r9,1032(r31)
c0000000000f6144: cmpdi cr7,r9,0
c0000000000f6148: bne cr7,0xc0000000000f6154
c0000000000f614c: ld r9,2312(r30)
c0000000000f6150: std r9,1032(r31)
c0000000000f6154: ld r9,88(r31)
Corresponding perf annotate o/p:
Before patch:
ld r9,1032(r31)
cmpdi cr7,r9,0
v bne 3ffffffffff09f2c
ld r9,2312(r30)
std r9,1032(r31)
74: ld r9,88(r31)
After patch:
ld r9,1032(r31)
cmpdi cr7,r9,0
v bne 74
ld r9,2312(r30)
std r9,1032(r31)
74: ld r9,88(r31)
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Riyder <chris.ryder@arm.com>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Taeung Song <treeze.taeung@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/1480953407-7605-2-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/annotate.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ea7e0de4b9c1..590244e5781e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -223,8 +223,12 @@ bool ins__is_call(const struct ins *ins)
static int jump__parse(struct arch *arch __maybe_unused, struct ins_operands *ops, struct map *map __maybe_unused)
{
const char *s = strchr(ops->raw, '+');
+ const char *c = strchr(ops->raw, ',');
- ops->target.addr = strtoull(ops->raw, NULL, 16);
+ if (c++ != NULL)
+ ops->target.addr = strtoull(c, NULL, 16);
+ else
+ ops->target.addr = strtoull(ops->raw, NULL, 16);
if (s++ != NULL)
ops->target.offset = strtoull(s, NULL, 16);
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 15/29] perf annotate: Fix jump target outside of function address range
2016-12-20 17:03 [GIT PULL 00/29] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-12-20 17:03 ` [PATCH 14/29] perf annotate: Support jump instruction with target as second operand Arnaldo Carvalho de Melo
@ 2016-12-20 17:03 ` Arnaldo Carvalho de Melo
2016-12-20 17:03 ` [PATCH 23/29] perf annotate: Don't throw error for zero length symbols Arnaldo Carvalho de Melo
2016-12-20 19:15 ` [GIT PULL 00/29] perf/core improvements and fixes Ingo Molnar
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-12-20 17:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Ravi Bangoria, Alexander Shishkin, Chris Riyder,
Kim Phillips, Markus Trippelsdorf, Masami Hiramatsu,
Naveen N . Rao, Peter Zijlstra, Taeung Song, linuxppc-dev,
Arnaldo Carvalho de Melo
From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
If jump target is outside of function range, perf is not handling it
correctly. Especially when target address is lesser than function start
address, target offset will be negative. But, target address declared to
be unsigned, converts negative number into 2's complement. See below
example. Here target of 'jumpq' instruction at 34cf8 is 34ac0 which is
lesser than function start address(34cf0).
34ac0 - 34cf0 = -0x230 = 0xfffffffffffffdd0
Objdump output:
0000000000034cf0 <__sigaction>:
__GI___sigaction():
34cf0: lea -0x20(%rdi),%eax
34cf3: cmp -bashx1,%eax
34cf6: jbe 34d00 <__sigaction+0x10>
34cf8: jmpq 34ac0 <__GI___libc_sigaction>
34cfd: nopl (%rax)
34d00: mov 0x386161(%rip),%rax # 3bae68 <_DYNAMIC+0x2e8>
34d07: movl -bashx16,%fs:(%rax)
34d0e: mov -bashxffffffff,%eax
34d13: retq
perf annotate before applying patch:
__GI___sigaction /usr/lib64/libc-2.22.so
lea -0x20(%rdi),%eax
cmp -bashx1,%eax
v jbe 10
v jmpq fffffffffffffdd0
nop
10: mov _DYNAMIC+0x2e8,%rax
movl -bashx16,%fs:(%rax)
mov -bashxffffffff,%eax
retq
perf annotate after applying patch:
__GI___sigaction /usr/lib64/libc-2.22.so
lea -0x20(%rdi),%eax
cmp -bashx1,%eax
v jbe 10
^ jmpq 34ac0 <__GI___libc_sigaction>
nop
10: mov _DYNAMIC+0x2e8,%rax
movl -bashx16,%fs:(%rax)
mov -bashxffffffff,%eax
retq
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Chris Riyder <chris.ryder@arm.com>
Cc: Kim Phillips <kim.phillips@arm.com>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Taeung Song <treeze.taeung@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/1480953407-7605-3-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/annotate.c | 5 +++--
tools/perf/util/annotate.c | 14 +++++++++-----
tools/perf/util/annotate.h | 5 +++--
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ec7a30fad149..ba36aac340bc 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -215,7 +215,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
ui_browser__set_color(browser, color);
if (dl->ins.ops && dl->ins.ops->scnprintf) {
if (ins__is_jump(&dl->ins)) {
- bool fwd = dl->ops.target.offset > (u64)dl->offset;
+ bool fwd = dl->ops.target.offset > dl->offset;
ui_browser__write_graph(browser, fwd ? SLSMG_DARROW_CHAR :
SLSMG_UARROW_CHAR);
@@ -245,7 +245,8 @@ static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sy
{
if (!dl || !dl->ins.ops || !ins__is_jump(&dl->ins)
|| !disasm_line__has_offset(dl)
- || dl->ops.target.offset >= symbol__size(sym))
+ || dl->ops.target.offset < 0
+ || dl->ops.target.offset >= (s64)symbol__size(sym))
return false;
return true;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 590244e5781e..c81a3950a7fe 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -230,10 +230,12 @@ static int jump__parse(struct arch *arch __maybe_unused, struct ins_operands *op
else
ops->target.addr = strtoull(ops->raw, NULL, 16);
- if (s++ != NULL)
+ if (s++ != NULL) {
ops->target.offset = strtoull(s, NULL, 16);
- else
- ops->target.offset = UINT64_MAX;
+ ops->target.offset_avail = true;
+ } else {
+ ops->target.offset_avail = false;
+ }
return 0;
}
@@ -241,7 +243,7 @@ static int jump__parse(struct arch *arch __maybe_unused, struct ins_operands *op
static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
struct ins_operands *ops)
{
- if (!ops->target.addr)
+ if (!ops->target.addr || ops->target.offset < 0)
return ins__raw_scnprintf(ins, bf, size, ops);
return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset);
@@ -1209,9 +1211,11 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
if (dl == NULL)
return -1;
- if (dl->ops.target.offset == UINT64_MAX)
+ if (!disasm_line__has_offset(dl)) {
dl->ops.target.offset = dl->ops.target.addr -
map__rip_2objdump(map, sym->start);
+ dl->ops.target.offset_avail = true;
+ }
/* kcore has no symbols, so add the call target name */
if (dl->ins.ops && ins__is_call(&dl->ins) && !dl->ops.target.name) {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 87e4cadc5d27..09776b5af991 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -24,7 +24,8 @@ struct ins_operands {
char *raw;
char *name;
u64 addr;
- u64 offset;
+ s64 offset;
+ bool offset_avail;
} target;
union {
struct {
@@ -68,7 +69,7 @@ struct disasm_line {
static inline bool disasm_line__has_offset(const struct disasm_line *dl)
{
- return dl->ops.target.offset != UINT64_MAX;
+ return dl->ops.target.offset_avail;
}
void disasm_line__free(struct disasm_line *dl);
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 23/29] perf annotate: Don't throw error for zero length symbols
2016-12-20 17:03 [GIT PULL 00/29] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-12-20 17:03 ` [PATCH 14/29] perf annotate: Support jump instruction with target as second operand Arnaldo Carvalho de Melo
2016-12-20 17:03 ` [PATCH 15/29] perf annotate: Fix jump target outside of function address range Arnaldo Carvalho de Melo
@ 2016-12-20 17:03 ` Arnaldo Carvalho de Melo
2016-12-20 19:15 ` [GIT PULL 00/29] perf/core improvements and fixes Ingo Molnar
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-12-20 17:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Ravi Bangoria, Alexander Shishkin,
Benjamin Herrenschmidt, Chris Riyder, linuxppc-dev,
Masami Hiramatsu, Michael Ellerman, Nicholas Piggin,
Paul Mackerras, Peter Zijlstra, stable, #, v4.9+,
Arnaldo Carvalho de Melo
From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
'perf report --tui' exits with error when it finds a sample of zero
length symbol (i.e. addr == sym->start == sym->end). Actually these are
valid samples. Don't exit TUI and show report with such symbols.
Reported-and-Tested-by: Anton Blanchard <anton@samba.org>
Link: https://lkml.org/lkml/2016/10/8/189
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Chris Riyder <chris.ryder@arm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@kernel.org # v4.9+
Link: http://lkml.kernel.org/r/1479804050-5028-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/annotate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index c81a3950a7fe..06cc04e5806a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -647,7 +647,8 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map, addr));
- if (addr < sym->start || addr >= sym->end) {
+ if ((addr < sym->start || addr >= sym->end) &&
+ (addr != sym->end || sym->start != sym->end)) {
pr_debug("%s(%d): ERANGE! sym->name=%s, start=%#" PRIx64 ", addr=%#" PRIx64 ", end=%#" PRIx64 "\n",
__func__, __LINE__, sym->name, sym->start, addr, sym->end);
return -ERANGE;
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [GIT PULL 00/29] perf/core improvements and fixes
2016-12-20 17:03 [GIT PULL 00/29] perf/core improvements and fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2016-12-20 17:03 ` [PATCH 23/29] perf annotate: Don't throw error for zero length symbols Arnaldo Carvalho de Melo
@ 2016-12-20 19:15 ` Ingo Molnar
3 siblings, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2016-12-20 19:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Adrian Hunter, Alexander Shishkin,
Alexei Starovoitov, Andi Kleen, Benjamin Herrenschmidt,
Chris Riyder, Daniel Borkmann, David Ahern, Davidlohr Bueso,
Jiri Olsa, Joe Stringer, Kan Liang, Kim Phillips, Kyle McMartin,
linuxppc-dev, Markus Trippelsdorf, Masami Hiramatsu,
Michael Ellerman, Minchan Kim, Namhyung Kim, Naveen N . Rao,
netdev, Nicholas Piggin, Paul Mackerras, Peter Zijlstra,
Ravi Bangoria, Taeung Song, Wang Nan, Arnaldo Carvalho de Melo
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Hi Ingo,
>
> Please consider pulling, I had most of this queued before your first
> pull req to Linus for 4.10, most are fixes, with 'perf sched timehist --idle'
> as a followup new feature to the 'perf sched timehist' command introduced in
> this window.
>
> One other thing that delayed this was the samples/bpf/ switch to
> tools/lib/bpf/ that involved fixing up merge clashes with net.git and also
> to properly test it, after more rounds than antecipated, but all seems ok
> now and would be good to get this merge issues past us ASAP.
>
> - Arnaldo
>
> Test results at the end of this message, as usual.
>
> The following changes since commit e7aa8c2eb11ba69b1b69099c3c7bd6be3087b0ba:
>
> Merge tag 'docs-4.10' of git://git.lwn.net/linux (2016-12-12 21:58:13 -0800)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo-20161220
>
> for you to fetch changes up to 9899694a7f67714216665b87318eb367e2c5c901:
>
> samples/bpf: Move open_raw_sock to separate header (2016-12-20 12:00:40 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> New features:
>
> - Introduce 'perf sched timehist --idle', to analyse processes
> going to/from idle state (Namhyung Kim)
>
> Fixes:
>
> - Allow 'perf record -u user' to continue when facing races with threads
> going away after having scanned them via /proc (Jiri Olsa)
>
> - Fix 'perf mem' --all-user/--all-kernel options (Jiri Olsa)
>
> - Support jumps with multiple arguments (Ravi Bangoria)
>
> - Fix jumps to before the function where they are located (Ravi
> Bangoria)
>
> - Fix lock-pi help string (Davidlohr Bueso)
>
> - Fix build of 'perf trace' in odd systems such as a RHEL PPC one (Jiri Olsa)
>
> - Do not overwrite valid build id in 'perf diff' (Kan Liang)
>
> - Don't throw error for zero length symbols, allowing the use of the TUI
> in PowerPC, where such symbols became more common recently (Ravi Bangoria)
>
> Infrastructure:
>
> - Switch of samples/bpf/ to use tools/lib/bpf, removing libbpf
> duplication (Joe Stringer)
>
> - Move headers check into bash script (Jiri Olsa)
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (3):
> perf tools: Remove some needless __maybe_unused
> samples/bpf: Make perf_event_read() static
> samples/bpf: Be consistent with bpf_load_program bpf_insn parameter
>
> Davidlohr Bueso (1):
> perf bench futex: Fix lock-pi help string
>
> Jiri Olsa (7):
> perf tools: Move headers check into bash script
> perf mem: Fix --all-user/--all-kernel options
> perf evsel: Use variable instead of repeating lengthy FD macro
> perf thread_map: Add thread_map__remove function
> perf evsel: Allow to ignore missing pid
> perf record: Force ignore_missing_thread for uid option
> perf trace: Check if MAP_32BIT is defined (again)
>
> Joe Stringer (8):
> tools lib bpf: Sync {tools,}/include/uapi/linux/bpf.h
> tools lib bpf: use __u32 from linux/types.h
> tools lib bpf: Add flags to bpf_create_map()
> samples/bpf: Make samples more libbpf-centric
> samples/bpf: Switch over to libbpf
> tools lib bpf: Add bpf_prog_{attach,detach}
> samples/bpf: Remove perf_event_open() declaration
> samples/bpf: Move open_raw_sock to separate header
>
> Kan Liang (1):
> perf diff: Do not overwrite valid build id
>
> Namhyung Kim (6):
> perf sched timehist: Split is_idle_sample()
> perf sched timehist: Introduce struct idle_time_data
> perf sched timehist: Save callchain when entering idle
> perf sched timehist: Skip non-idle events when necessary
> perf sched timehist: Add -I/--idle-hist option
> perf sched timehist: Show callchains for idle stat
>
> Ravi Bangoria (3):
> perf annotate: Support jump instruction with target as second operand
> perf annotate: Fix jump target outside of function address range
> perf annotate: Don't throw error for zero length symbols
>
> samples/bpf/Makefile | 70 +--
> samples/bpf/README.rst | 4 +-
> samples/bpf/bpf_load.c | 21 +-
> samples/bpf/bpf_load.h | 3 +
> samples/bpf/fds_example.c | 13 +-
> samples/bpf/lathist_user.c | 2 +-
> samples/bpf/libbpf.c | 176 -------
> samples/bpf/libbpf.h | 28 +-
> samples/bpf/lwt_len_hist_user.c | 6 +-
> samples/bpf/offwaketime_user.c | 8 +-
> samples/bpf/sampleip_user.c | 7 +-
> samples/bpf/sock_example.c | 14 +-
> samples/bpf/sock_example.h | 35 ++
> samples/bpf/sockex1_user.c | 7 +-
> samples/bpf/sockex2_user.c | 5 +-
> samples/bpf/sockex3_user.c | 5 +-
> samples/bpf/spintest_user.c | 8 +-
> samples/bpf/tc_l2_redirect_user.c | 4 +-
> samples/bpf/test_cgrp2_array_pin.c | 4 +-
> samples/bpf/test_cgrp2_attach.c | 12 +-
> samples/bpf/test_cgrp2_attach2.c | 8 +-
> samples/bpf/test_cgrp2_sock.c | 7 +-
> samples/bpf/test_current_task_under_cgroup_user.c | 8 +-
> samples/bpf/test_lru_dist.c | 32 +-
> samples/bpf/test_probe_write_user_user.c | 2 +-
> samples/bpf/trace_event_user.c | 23 +-
> samples/bpf/trace_output_user.c | 7 +-
> samples/bpf/tracex2_user.c | 10 +-
> samples/bpf/tracex3_user.c | 4 +-
> samples/bpf/tracex4_user.c | 4 +-
> samples/bpf/tracex6_user.c | 5 +-
> samples/bpf/xdp1_user.c | 2 +-
> samples/bpf/xdp_tx_iptunnel_user.c | 6 +-
> tools/include/uapi/linux/bpf.h | 593 +++++++++++++---------
> tools/lib/bpf/bpf.c | 30 +-
> tools/lib/bpf/bpf.h | 9 +-
> tools/lib/bpf/libbpf.c | 3 +-
> tools/perf/Documentation/perf-sched.txt | 4 +
> tools/perf/Makefile.perf | 94 +---
> tools/perf/bench/futex-lock-pi.c | 2 +-
> tools/perf/builtin-c2c.c | 13 +-
> tools/perf/builtin-mem.c | 4 +-
> tools/perf/builtin-record.c | 3 +
> tools/perf/builtin-report.c | 2 +-
> tools/perf/builtin-sched.c | 261 ++++++++--
> tools/perf/builtin-stat.c | 6 +-
> tools/perf/check-headers.sh | 59 +++
> tools/perf/perf.h | 1 +
> tools/perf/tests/builtin-test.c | 4 +
> tools/perf/tests/tests.h | 1 +
> tools/perf/tests/thread-map.c | 44 ++
> tools/perf/trace/beauty/mmap.c | 2 +
> tools/perf/ui/browsers/annotate.c | 5 +-
> tools/perf/util/annotate.c | 23 +-
> tools/perf/util/annotate.h | 5 +-
> tools/perf/util/evsel.c | 61 ++-
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/symbol.c | 3 +-
> tools/perf/util/thread_map.c | 22 +
> tools/perf/util/thread_map.h | 1 +
> 60 files changed, 1075 insertions(+), 731 deletions(-)
> delete mode 100644 samples/bpf/libbpf.c
> create mode 100644 samples/bpf/sock_example.h
> create mode 100755 tools/perf/check-headers.sh
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread