Skip end-of-sequence and non-statement lines while walking
through lines list.
The "end-of-sequence" line information means
"the current address is that of the first byte after the
end of a sequence of target machine instructions."
(DWARF version 4 spec 6.2.2)
This actually means out of scope and we can not probe on it.
On the other hand, the statement lines (is_stmt) means
"the current instruction is a recommended breakpoint location.
A recommended breakpoint location is intended to “represent”
a line, a statement and/or a semantically distinct subpart
of a statement."
(DWARF version 4 spec 6.2.2)
So, non-statement line info also should be skipped.
These can reduce unneeded probe points and also avoid an error.
E.g. without this patch,
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new events:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_2 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_3 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_4 (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask_4 -aR sleep 1
#
This puts 5 probes on one line, but acutally it's not inlined
function. This is because there are many non statement instructions
at the function prologue.
With this patch,
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new event:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1
#
Now perf-probe skips unneeded addresses.
Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/util/dwarf-aux.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index ac82fd937e4b..f31001d13bfb 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -782,6 +782,7 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
int decl = 0, inl;
Dwarf_Die die_mem, *cu_die;
size_t nlines, i;
+ bool flag;
/* Get the CU die */
if (dwarf_tag(rt_die) != DW_TAG_compile_unit) {
@@ -812,6 +813,12 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
"Possible error in debuginfo.\n");
continue;
}
+ /* Skip end-of-sequence */
+ if (dwarf_lineendsequence(line, &flag) != 0 || flag)
+ continue;
+ /* Skip Non statement line-info */
+ if (dwarf_linebeginstatement(line, &flag) != 0 || !flag)
+ continue;
/* Filter lines based on address */
if (rt_die != cu_die) {
/*
Em Wed, Oct 30, 2019 at 04:09:21PM +0900, Masami Hiramatsu escreveu:
> Skip end-of-sequence and non-statement lines while walking
> through lines list.
>
> The "end-of-sequence" line information means
> "the current address is that of the first byte after the
> end of a sequence of target machine instructions."
> (DWARF version 4 spec 6.2.2)
> This actually means out of scope and we can not probe on it.
Thanks, tested before and after and applied,
- Arnaldo
The following commit has been merged into the perf/core branch of tip:
Commit-ID: f4d99bdfd124823a81878b44b5e8750b97f73902
Gitweb: https://git.kernel.org/tip/f4d99bdfd124823a81878b44b5e8750b97f73902
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 30 Oct 2019 16:09:21 +09:00
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitterDate: Thu, 07 Nov 2019 08:30:18 -03:00
perf probe: Skip end-of-sequence and non statement lines
Skip end-of-sequence and non-statement lines while walking through lines
list.
The "end-of-sequence" line information means:
"the current address is that of the first byte after the
end of a sequence of target machine instructions."
(DWARF version 4 spec 6.2.2)
This actually means out of scope and we can not probe on it.
On the other hand, the statement lines (is_stmt) means:
"the current instruction is a recommended breakpoint location.
A recommended breakpoint location is intended to “represent”
a line, a statement and/or a semantically distinct subpart
of a statement."
(DWARF version 4 spec 6.2.2)
So, non-statement line info also should be skipped.
These can reduce unneeded probe points and also avoid an error.
E.g. without this patch:
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new events:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_2 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_3 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_4 (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask_4 -aR sleep 1
#
This puts 5 probes on one line, but acutally it's not inlined function.
This is because there are many non statement instructions at the
function prologue.
With this patch:
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new event:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1
#
Now perf-probe skips unneeded addresses.
Committer testing:
Slightly different results, but similar:
Before:
# uname -a
Linux quaco 5.3.8-200.fc30.x86_64 #1 SMP Tue Oct 29 14:46:22 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
#
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new events:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_1 (on clear_tasks_mm_cpumask:1)
probe:clear_tasks_mm_cpumask_2 (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask_2 -aR sleep 1
#
After:
# perf probe -a "clear_tasks_mm_cpumask:1"
Added new event:
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask:1)
You can now use it in all perf tools, such as:
perf record -e probe:clear_tasks_mm_cpumask -aR sleep 1
# perf probe -l
probe:clear_tasks_mm_cpumask (on clear_tasks_mm_cpumask@kernel/cpu.c)
#
Fixes: 4cc9cec636e7 ("perf probe: Introduce lines walker interface")
Signed-off-by: Masami Hiramatsu <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lore.kernel.org/lkml/157241936090.32002.12156347518596111660.stgit@devnote2
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/dwarf-aux.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index ac82fd9..f31001d 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -782,6 +782,7 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
int decl = 0, inl;
Dwarf_Die die_mem, *cu_die;
size_t nlines, i;
+ bool flag;
/* Get the CU die */
if (dwarf_tag(rt_die) != DW_TAG_compile_unit) {
@@ -812,6 +813,12 @@ int die_walk_lines(Dwarf_Die *rt_die, line_walk_callback_t callback, void *data)
"Possible error in debuginfo.\n");
continue;
}
+ /* Skip end-of-sequence */
+ if (dwarf_lineendsequence(line, &flag) != 0 || flag)
+ continue;
+ /* Skip Non statement line-info */
+ if (dwarf_linebeginstatement(line, &flag) != 0 || !flag)
+ continue;
/* Filter lines based on address */
if (rt_die != cu_die) {
/*
On Wed, 2019-11-06 at 17:04 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 30, 2019 at 04:09:21PM +0900, Masami Hiramatsu escreveu:
> > Skip end-of-sequence and non-statement lines while walking
> > through lines list.
> >
> > The "end-of-sequence" line information means
> > "the current address is that of the first byte after the
> > end of a sequence of target machine instructions."
> > (DWARF version 4 spec 6.2.2)
> > This actually means out of scope and we can not probe on it.
>
> Thanks, tested before and after and applied,
>
> - Arnaldo
Hello,
I'm getting perf test failures after this patch hit mainline and LTS
kernels.
With v5.5-rc5 (x86_64 gcc-9.2.1) I get:
# perf test
...
69: Add vfs_getname probe to get syscall args filenames : FAILED!
71: Use vfs_getname probe to get syscall args filenames : FAILED!
73: Check open filename arg using perf trace + vfs_getname: FAILED!
Reason is that it's no longer possible to add probe in getname_flags()
line "result->uptr = filename;", and this is causing the perf tests to
fail:
# perf probe -L getname_flags
<getname_flags@/root/linux-core2/fs/namei.c:0>
0 getname_flags(const char __user *filename, int flags, int *empty)
{
...
61 result->refcnt = 1;
/* The empty path is special. */
63 if (unlikely(!len)) {
64 if (empty)
65 *empty = 1;
66 if (!(flags & LOOKUP_EMPTY)) {
67 putname(result);
68 return ERR_PTR(-ENOENT);
}
}
result->uptr = filename;
73 result->aname = NULL;
audit_getname(result);
return result;
}
I can reproduce it locally with a kernel config change (with attached KVM
guest config): when kernel is built with CONFIG_MCORE2=y the issue
reproduces. When switching to CONFIG_GENERIC_CPU=y it's again possible to
add the probe to the source line and the tests pass.
Diffing the disassembly of fs/namei.o between CONFIG_MCORE2=y and
CONFIG_GENERIC_CPU=y, all instructions in getname_flags() are the same,
but there are some changes in the instruction ordering.
Any ideas what's going wrong here...?
Something wrong in the dwarf data?
In 4.19.y it's enough to revert this patch ("perf probe: Skip end-of-
sequence and non statement lines") to fix this (assuming it's bug...), but
in v5.5-rc5 it's not enough (switching to v5.4 perf fixed it).
-Tommi
Hi Tommi,
Thank you for reporting!
On Fri, 10 Jan 2020 09:29:22 +0000
"Rantala, Tommi T. (Nokia - FI/Espoo)" <[email protected]> wrote:
>
> Hello,
>
> I'm getting perf test failures after this patch hit mainline and LTS
> kernels.
>
> With v5.5-rc5 (x86_64 gcc-9.2.1) I get:
>
> # perf test
> ...
> 69: Add vfs_getname probe to get syscall args filenames : FAILED!
> 71: Use vfs_getname probe to get syscall args filenames : FAILED!
> 73: Check open filename arg using perf trace + vfs_getname: FAILED!
>
> Reason is that it's no longer possible to add probe in getname_flags()
> line "result->uptr = filename;", and this is causing the perf tests to
> fail:
>
> # perf probe -L getname_flags
> <getname_flags@/root/linux-core2/fs/namei.c:0>
> 0 getname_flags(const char __user *filename, int flags, int *empty)
> {
> ...
> 61 result->refcnt = 1;
> /* The empty path is special. */
> 63 if (unlikely(!len)) {
> 64 if (empty)
> 65 *empty = 1;
> 66 if (!(flags & LOOKUP_EMPTY)) {
> 67 putname(result);
> 68 return ERR_PTR(-ENOENT);
> }
> }
>
> result->uptr = filename;
> 73 result->aname = NULL;
> audit_getname(result);
> return result;
> }
>
>
> I can reproduce it locally with a kernel config change (with attached KVM
> guest config): when kernel is built with CONFIG_MCORE2=y the issue
> reproduces. When switching to CONFIG_GENERIC_CPU=y it's again possible to
> add the probe to the source line and the tests pass.
>
> Diffing the disassembly of fs/namei.o between CONFIG_MCORE2=y and
> CONFIG_GENERIC_CPU=y, all instructions in getname_flags() are the same,
> but there are some changes in the instruction ordering.
>
> Any ideas what's going wrong here...?
> Something wrong in the dwarf data?
Hmm, I guess gcc's dwarf generator gets some effects from the optimizer.
Maybe since the optimizer reordering the insturction, the dwarf generator
might lose the line statement information. For example, if instructions
from different lines are mixed, it is hard to say "this is the begining
of a line".
>
> In 4.19.y it's enough to revert this patch ("perf probe: Skip end-of-
> sequence and non statement lines") to fix this (assuming it's bug...), but
> in v5.5-rc5 it's not enough (switching to v5.4 perf fixed it).
I think perf test should be fixed to use more stable lines.
(But I'm not sure why it uses this line...)
Thank you,
--
Masami Hiramatsu <[email protected]>